block_disk_not_default_options: creates new test case #4209
Conversation
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
2 times, most recently
from
66f362b to
8442610
Compare
|
Tests results |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
8442610 to
9817d33
Compare
|
@qingwangrh could you review this PR? Thanks ! |
| remove_image_stg0 = yes | ||
| force_create_image_stg0 = yes | ||
| dd_cmd = "dd if=/dev/urandom of=test.txt bs=1M count=10" | ||
| virtio_blk: |
There was a problem hiding this comment.
The file is the non-default option comment test, it will evolve to cover other option test. so I suggest considering current test as one variant: example - with_packed_and_ppv
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
2 times, most recently
from
2166cd8 to
c955535
Compare
| session.cmd(params.get("dd_cmd")) | ||
|
|
||
| possible_types = {"virtio-blk-device", "virtio-scsi-device"} | ||
| verify_non_default_options("packed", possible_types) |
There was a problem hiding this comment.
it can comes from cfg instead of hard code
qtree_check={k1:v1}
There was a problem hiding this comment.
qtree verification implemented in cfg side, so the user can configure it to verify any option of a desired device
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
c955535 to
1400868
Compare
| dd_cmd = "dd if=/dev/urandom of=test.txt bs=1M count=10" | ||
| variants: | ||
| - with_packed_and_ppv: | ||
| qtree_check = '{"packed" : ["virtio-blk-device", "virtio-scsi-device"], "page-per-vq" : ["virtio-blk-pci", "virtio-scsi-pci"]}' |
There was a problem hiding this comment.
- it can be evolved to support different image ,like as
qtree_check_images=" stg0 stg1...."
qtree_check_image_stg0='{"packed" : ["virtio-blk-device", "virtio-scsi-device"], "page-per-vq" : ["virtio-... - where is the value definition ?example, I want to check the key1 value is "ABC"?
| if qtree_check: | ||
| for key in qtree_check: | ||
| verify_non_default_options(key, qtree_check[key]) | ||
| else: |
There was a problem hiding this comment.
The qtree_check is not mandatory, so I suggest changing it to warn message, otherwise, the case will be looked as failed case.
There was a problem hiding this comment.
@qingwangrh yeah I thought about that and changed it yesterday :) I will push the new changes asap
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
2 times, most recently
from
564547b to
dea631d
Compare
|
@qingwangrh now the qtree check should be fully implemented, still working on the dd cmd improvement |
|
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
dea631d to
cd1213e
Compare
|
@mcasquer Please don't forget to add -S when pushing the patch. (I.E. commit signature) |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
cd1213e to
5496417
Compare
Sorry for that, it's been a one-time oversight 😅 |
|
Once the dependent patch #4219 gets merged, CI should be green after rebase |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
2 times, most recently
from
c6db2da to
e194141
Compare
|
After applying the dependent patch: Win10 guest with virtio_scsi Win2025 guest with virtio_blk |
|
This is a kindly reminder, @qingwangrh please could you review again this PR? Thanks ! |
|
Added @XueqiangWei |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
e194141 to
bd82563
Compare
|
With last update on the provider: |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
bd82563 to
405f0f7
Compare
|
After applying the dependent patch with last changes both test cases passed |
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
405f0f7 to
93f8ce5
Compare
| vm.verify_alive() | ||
| session = vm.wait_for_login(timeout=360) | ||
|
|
||
| qtree_check_images = params.get("qtree_check_images", "stg0").split() |
There was a problem hiding this comment.
| qtree_check_images = params.get("qtree_check_images", "stg0").split() | |
| qtree_check_images = params.get_list("qtree_check_images", "stg0") |
| ) | ||
|
|
||
| for image in qtree_check_images: | ||
| qtree_check_image = eval(params.get(f"qtree_check_{image}", "{}")) |
There was a problem hiding this comment.
eval is not a good choice, reading your cfg, you may want convert json to dict?
There was a problem hiding this comment.
@PaulYuuu
Yes, I think so, it's been a long time since I push this code... 😅
There was a problem hiding this comment.
No worries, to be honest, I am not a block maintainer, so just review to be best from my knowledge, any maybe params.get_dict can also what you want.
There was a problem hiding this comment.
@PaulYuuu got it, thanks for the review I am gonna give a try !
There was a problem hiding this comment.
No, it seems it's not a JSON, it's like a properties file, after applying get_dict:
ValueError: failed to find "=" in "{"packed"" (value for qtree_check_stg0)
There was a problem hiding this comment.
I will keep the current implementation, thanks @PaulYuuu for the review
There was a problem hiding this comment.
You should check the usage of get_dict from utils_params.py. it requests like key = foo1=bar1 foo2=bar2
Creates a new test case that boots up a VM with a disk configured with non-common options such as packed or page-per-vq. Unplug the disk and hotplugs it again. Signed-off-by: Mario Casquero <[email protected]>
mcasquer
force-pushed
the
3007_booting_hotplugging_disk_non_default_options
branch
from
93f8ce5 to
2c3cd2d
Compare
Depends on #4219
block_disk_not_default_options: creates new test case
Creates a new test case that boots up a VM with a disk
configured with non-common options such as packed or
page-per-vq. Unplug the disk and hotplugs it again.
Signed-off-by: Mario Casquero [email protected]
ID: 3007