fix: Empty "image_size" in sn_params when "image_snapshot = yes"

[PaulYuuu]

In qcontainer.py, when creating snapshot images, will get the image size from params. But if the image_size is defined in different variants, the sn image size may smaller than the base image, which is incorrect. The easier way is empty the image_size in sn_params, to make the size of snapshot image align to the base one, as it's just a snapshot.

ID: 1529

[PaulYuuu]

A reproducer is:

1. Install a guest with image_size = 50G.
2. Remove the parameter and use the default one(10G in VT) to boot the guest, but using image_snapshot = yes

qemu-img create -f qcow2 -b /home/kvm_autotest_root/images/rhel920-aarch64-virtio-scsi.qcow2 -F qcow2 /root/avocado/data/avocado-vt/vl_avocado-vt-vm1_image1.qcow2 10G

qemu-img info /var/tmp/vl_avocado-vt-vm1_image1.qcow2
image: /var/tmp/vl_avocado-vt-vm1_image1.qcow2
file format: qcow2
virtual size: 10 GiB (10737418240 bytes)
disk size: 196 KiB
cluster_size: 65536
backing file: /home/kvm_autotest_root/images/rhel920-aarch64-virtio-scsi.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false
Child node '/file':
    filename: /var/tmp/vl_avocado-vt-vm1_image1.qcow2
    protocol type: file
    file length: 192 KiB (197120 bytes)
    disk size: 196 KiB
    Format specific information:
        extent size hint: 1048576

With the fix:

qemu-img create -f qcow2 -b /home/kvm_autotest_root/images/rhel920-aarch64-virtio-scsi.qcow2 -F qcow2 /root/avocado/data/avocado-vt/vl_avocado-vt-vm1_image1.qcow2

qemu-img info /var/tmp/vl_avocado-vt-vm1_image1.qcow2
image: /var/tmp/vl_avocado-vt-vm1_image1.qcow2
file format: qcow2
virtual size: 50 GiB (53687091200 bytes)
disk size: 196 KiB
cluster_size: 65536
backing file: /home/kvm_autotest_root/images/rhel920-aarch64-virtio-scsi.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false
Child node '/file':
    filename: /var/tmp/vl_avocado-vt-vm1_image1.qcow2
    protocol type: file
    file length: 193 KiB (197632 bytes)
    disk size: 196 KiB
    Format specific information:
        extent size hint: 1048576

[PaulYuuu]

Hello @YongxueHong @MiriamDeng, feel free to review, thanks.

[MiriamDeng]

Will provide the test results today. Thank you !

[MiriamDeng]

LGTM, the original issue has gone.

[YongxueHong]

From my understanding of the code changes, I think both solutions are the same, just the implementation is a bit different, one is explicit, another is implicit.

[YongxueHong]

Actually, there is an issue with the attribute conflict of the image that the test case uses the existing image as the system image, but this image's attributes don't match the parameters in the test case like this PR's issue background is that we prepared an existing 50G system image for the next test case to run, but the next case still uses the 20G image size(actually the size is changed) as the system image which will result in the conflict with the existing 50G system image and then met the error about doing the external snapshot.

To sum up, I think the root cause is that we did not follow the normal logic workflow, and should keep the data consistent first

My suggestion: Better to update the attribute of the system image of the test case at the same time.

Hi @luckyh @PaulYuuu
Please let me know your opinions.
Thanks.

[PaulYuuu]

IMO, the temporary snapshot should follow the image's real metadata to create the snapshot image, not from the parameter. Here I used the easier way to let it align with the base image, but the more reasonable solution is to get the base image's metadata, and then create the snapshot based on it.

In -drive usage, only image_snapshot = yes is needed to create an internal snapshot, but -blockdev dropped this option, so if we also need image_size to be provided, then this is a breaking change between -drive and -blockdev.

However, to keep the same usage as previous(-drive), leave image_size empty is one way, another one is still create internal snapshot via qemu-img snapshot -c, and we delete the snapshot at the end of the test.

qemu-img snapshot -l base.qcow2
qemu-img snapshot -c sn1 base.qcow2
qemu-img snapshot -l base.qcow2
Snapshot list:
ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
1         sn1                   0 B 2023-11-28 22:09:04 00:00:00.000          0
qemu-img snapshot -d sn1 base.qcow2
qemu-img snapshot -l base.qcow2

refs:

1. https://wiki.qemu.org/Documentation/CreateSnapshot
2. https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/virtualization_deployment_and_administration_guide/sect-using_qemu_img-listing_creating_applying_and_deleting_a_snapshot

[PaulYuuu]

Add a FIXME as most of the attributes for the snapshot should be obtained from the base image.