avocado-framework / avocado-vt

Avocado VT Plugin
https://avocado-vt.readthedocs.org/
Other
83 stars 241 forks source link

Introduce image slices #3973

Closed luckyh closed 2 weeks ago

luckyh commented 3 weeks ago

Introduce the image slices support to allow users assigning parts of the block devices to the VMs as storage backends.

Notes that this patch merely implements the support for qemu testing.

ID: 2644

luckyh commented 3 weeks ago

@YongxueHong @qingwangrh @aliang123 would you mind to help review this one? thanks in advance!

aliang123 commented 3 weeks ago

Test cfg: blockdev_stream_filter_nodename.cfg image_size_data1 = 2G image_format_data1 = qcow2 image_slices_data1 = slice1 image_slice_offset_slice1 = 0 image_slice_size_slice1 = 1G nbd: image_format_data1 = raw nbd_port_data1 = 10831 remove_image_data1 = no force_create_image_data1 = no

Scenario 1: nbd+raw+slice(offset/size) Correct qemu cmdline: -blockdev '{"node-name": "nbd_data1", "driver": "nbd", "auto-read-only": true, "discard": "unmap", "server": {"type": "inet", "host": "10.72.140.40", "port": "10831"}, "cache": {"direct": true, "no-flush": false}}' \ -blockdev '{"node-name": "drive_data1", "driver": "raw", "read-only": false, "offset": 0, "size": 1073741824, "cache": {"direct": true, "no-flush": false}, "file": "nbd_data1"}' \ -device '{"driver": "scsi-hd", "id": "data1", "drive": "drive_data1", "write-cache": "on"}' \

Scenario 2: qcow2+slice(offset/size) Errror qemu cmdline: error info: (status: 1, output: 'qemu-kvm: -blockdev {"node-name": "drive_data1", "driver": "qcow2", "read-only": false, "offset": 0, "size": 1073741824, "cache": {"direct": true, "no-flush": false}, "file": "file_data1"}: Parameter \'size\' is unexpected')

After discussed with @qingwangrh, acked for both scenario.

One more question: What can we do if we define the same slice name for different disks, like: image_slices_data1 = slice1 image_slice_offset_slice1 = 0 image_slice_size_slice1 = 1G

image_slices_data2 = slice1 image_slice_offset_slice1 = 0 image_slice_size_slice1 = 2G

qingwangrh commented 3 weeks ago

Does the key change to image_slice_xxx=offset:size , if there are multiple slices of an image(so far one image one slice in libvirt), it can be described to o image_slice_xxx=offset:size,offset2:size2. The API does not need to be changed and simply logic. We check whether the key existence or not. @luckyh ,what do you think ?

luckyh commented 3 weeks ago

@luckyh ,what do you think ?

@qingwangrh actually, I made the first version implemented just as what you suggested above, however, there were some reasons convinced me for choosing the current one:

  1. according to libvirt's domain xml format reference, the slice element also has an attribute called type for indicating the slice type (notes that the doc also mentioned about the future expansion to this attribute) which means we can not emit it in the value expression, therefore the value pattern of a single slice configuration have to be $type:$offset:$size.

    The slices element using its slice sub-elements allows configuring offset and size of either the location of the image format (slice type='storage') inside the storage source or the guest data inside the image format container (future expansion). The offset and size values are in bytes. Since 6.1.0

  2. as some of the attribute(s) is optional, using positional arguments may lead the issue of ambiguity, and hence it had better to use keyword arguments instead, like type=$type[,offset=$offset][,size=$size] type=....

  3. considered about (potential) forward compatibility, when everytime introducing a new param, if we need to ensure that the test can work with both the old and the new version, via using the current approach we just need to append only the new param with introducing a separated variant like

    image_slices = s1 s2
    image_slice_offset_s1 = 0
    image_slice_offset_s2 = 0
    variants:
        - old_version:
        - new_version:
            image_slice_newparam_s1 = foo

    while in the opposite, seems we have to make a replication to the param and then modify that to the target value.

    variants:
        - old_version:
            image_slices = offset=0 offset=0
        - new_version:
            image_slices = offset=0,newparam=foo offset=0

Let me know your opinion, thanks!

qingwangrh commented 3 weeks ago

LGTM