search

avocado-framework / avocado-vt

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

Qemu blockdev raw format elimination #3952

Closed luckyh closed 2 months ago

luckyh commented 3 months ago

ID: 2408

qingwangrh commented 3 months ago

hit Error occurred while executing make_create_command() @luckyh Please check comment in 2408

luckyh commented 3 months ago

@qingwangrh good catch, have modified the code accordingly, thanks!

luckyh commented 3 months ago

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

aliang123 commented 3 months ago

(1/1) Host_RHEL.m9.ux.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.x.0.x86_64.io-github-autotest-qemu.blockdev_snapshot.q35: ERROR: QMP command 'blockdev-add' failed (arguments: {'node-name': 'drive_sn1', 'driver': 'qcow2', 'file': 'drive_sn1', 'read-only': False}, error message: {'class': 'GenericError', 'desc': "Duplicate nodes with node-name='drive_sn1'"}) (57.94 s)

When adding qcow2 image with blockdev-add, failed to add the format node, as same node-name assigned to both protocol node and format node. {"execute": "blockdev-add", "arguments": {"node-name": "drive_sn1", "driver": "file", "filename": "/root/avocado/data/avocado-vt/sn1.qcow2", "aio": "threads", "auto-read-only": true, "discard": "unmap"}, "id": "RnkXjzdy"} {"execute": "blockdev-add", "arguments": {"node-name": "drive_sn1", "driver": "qcow2", "file": "drive_sn1", "read-only": false}, "id": "96qDODUP"} virttest.qemu_monitor.QMPCmdError: QMP command 'blockdev-add' failed (arguments: {'node-name': 'drive_sn1', 'driver': 'qcow2', 'file': 'drive_sn1', 'read-only': False}, error message: {'class': 'GenericError', 'desc': "Duplicate nodes with node-name='drive_sn1'"})

correct qmp cmd for adding a qcow2 target: {"execute": "blockdev-add", "arguments": {"node-name": "file_sn1", "driver": "file", "filename": "/root/avocado/data/avocado-vt/sn1.qcow2", "aio": "threads", "auto-read-only": true, "discard": "unmap"}, "id": "ktTo6xjQ"} {"execute": "blockdev-add", "arguments": {"node-name": "drive_sn1", "driver": "qcow2", "file": "file_sn1", "read-only": false}, "id": "AFpzKVRh"}

luckyh commented 3 months ago

Hi @aliang123 , it seems to be a bug on the test script side, so let's fix it there.

qingwangrh commented 3 months ago

qcow2 protocol discard=unmap even if discard=ignore in xml

-blockdev '{"driver":"file","filename":"/home/kvm_autotest_root/images/mstg4.qcow2","node-name":"libvirt-10-storage","auto-read-only":true,"discard":"unmap"}' -blockdev '{"node-name":"libvirt-10-format","read-only":false,"discard":"ignore","detect-zeroes":"off","driver":"qcow2","file":"libvirt-10-storage"}' -device '{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":4,"device_id":"drive-scsi0-0-0-4","drive":"libvirt-10-format","id":"scsi0-0-0-4"}'

luckyh commented 3 months ago

qcow2 protocol discard=unmap even if discard=ignore in xml

@qingwangrh yeah, it's a expected behavior performed libvirt (see libvirt/src/qemu/qemu_block.c@qemuBlockStorageSourceAddBlockdevCommonProps). In short, the discard option of the protocol node will always be set to unmap if the format node presents.

However, it seems that it has already been implemented as so even before the raw format elimination been applied. Therefore, I'd suggest filing a separated issue to track it, and we could investigate and discuss there, thanks.

    if (effective) {
        virDomainDiskDetectZeroes dz = virDomainDiskGetDetectZeroesMode(src->discard,
                                                                        src->detect_zeroes);

        if (src->discard != VIR_DOMAIN_DISK_DISCARD_DEFAULT)
            discard = virDomainDiskDiscardTypeToString(src->discard);

        if (dz != VIR_DOMAIN_DISK_DETECT_ZEROES_DEFAULT)
            detectZeroes = virDomainDiskDetectZeroesTypeToString(dz);

        autoReadOnly = VIR_TRISTATE_BOOL_ABSENT;
        readOnly = virTristateBoolFromBool(src->readonly);
    } else {
        /* when passing a FD to qemu via the /dev/fdset mechanism qemu
         * fetches the appropriate FD from the fdset by checking that it has
         * the correct accessmode. Now with 'auto-read-only' in effect qemu
         * wants to use a read-only FD first. If the user didn't pass multiple
         * FDs the feature will not work regardless, so we'll not enable it. */
        if ((actualType == VIR_STORAGE_TYPE_FILE || actualType == VIR_STORAGE_TYPE_BLOCK) &&
            src->fdtuple && src->fdtuple->nfds == 1) {
            autoReadOnly = VIR_TRISTATE_BOOL_ABSENT;

            /* now we setup the normal readonly flag. If user requested write access honour it */
            if (src->fdtuple->writable)
                readOnly = VIR_TRISTATE_BOOL_NO;
            else
                readOnly = virTristateBoolFromBool(src->readonly);
        } else {
            autoReadOnly = VIR_TRISTATE_BOOL_YES;
        }

        discard = "unmap";  // <--- HERE
    }
aliang123 commented 3 months ago
qingwangrh commented 2 months ago

Pass python ConfigTest.py --testcase=insert_media,multi_disk.virtio_scsi_variants.passthrough.generic.with_cache_writethrough --guestname=RHEL.9.4.0 --driveformat=virtio_blk,virtio_scsi --imageformat=qcow2,raw --firmware=default_bios --customsparams="no prepare\nvm_mem_limit = 12G\n" --clone=no

LGTM