On Thu, Jan 7, 2021 at 5:23 PM Peter Krempa <pkrempa@redhat.com> wrote:

Similarly to startup of the VM qemu doesn't like setting throttling for an empty drive. Just skip it since we do the correct thing once new media is inserted.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/117 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- CC: Han Han <hhan@redhat.com>

Please test this commit since you have the environment prepared.

src/qemu/qemu_driver.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1283e61785..027617deef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16301,16 +16301,23 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,

#undef CHECK_MAX

- qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, - &info, supportMaxOptions, - set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME, - supportMaxLengthOptions); - if (qemuDomainObjExitMonitor(driver, vm) < 0) + /* blockdev-based qemu doesn't want to set the throttling when a cdrom + * is empty. Skip the monitor call here since we will set the throttling + * once new media is inserted */ + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) || + !virStorageSourceIsEmpty(disk->src)) { + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, + &info, supportMaxOptions, + set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME, + supportMaxLengthOptions); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + if (ret < 0) + goto endjob; ret = -1; - if (ret < 0) - goto endjob; - ret = -1; + }

virDomainDiskSetBlockIOTune(disk, &info);

-- 2.29.2

Test on libvirt-v6.10.0-335-gc5904929e8 qemu-5.2.0-4.fc34.x86_64: Start a VM with empty cdrom: <disk type="file" device="cdrom"> <driver name="qemu"/> <target dev="sda" bus="scsi"/> <readonly/> <alias name="scsi0-0-0"/> <address type="drive" controller="0" bus="0" target="0" unit="0"/> </disk>

Set the blkiotune for the cdrom: ➜ ~ virsh blkdeviotune test sda --total-bytes-sec 10000 ➜ ~ virsh blkdeviotune test sda --group-name test Dumpxml: <disk type="file" device="cdrom"> <driver name="qemu"/> <target dev="sda" bus="scsi"/> <iotune> <total_bytes_sec>10000</total_bytes_sec> <group_name>test</group_name> </iotune> <readonly/> <alias name="scsi0-0-0"/> <address type="drive" controller="0" bus="0" target="0" unit="0"/> </disk> Insert the media: ➜ ~ virsh change-media test sda --insert /var/lib/libvirt/images/boot.iso Successfully inserted media. QMP log: 56.306 > 0x7f8b04043030 {"execute":"blockdev-open-tray","arguments":{"id":"scsi0-0-0","force":false},"id":"libvirt-379"} 56.310 ! 0x7f8b04043030 {"timestamp": {"seconds": 1610013536, "microseconds": 856358}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "", "tray-open": true, "id": "scsi0-0-0"}} 56.311 < 0x7f8b04043030 {"return": {}, "id": "libvirt-379"} 56.311 > 0x7f8b04043030 {"execute":"blockdev-remove-medium","arguments":{"id":"scsi0-0-0"},"id":"libvirt-380"} 56.314 < 0x7f8b04043030 {"return": {}, "id": "libvirt-380"} 56.314 > 0x7f8b04043030 {"execute":"blockdev-add","arguments":{"driver":"file","filename":"/var/lib/libvirt/images/boot.iso","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"},"id":"libvirt-381"} 56.318 < 0x7f8b04043030 {"return": {}, "id": "libvirt-381"} 56.319 > 0x7f8b04043030 {"execute":"blockdev-add","arguments":{"node-name":"libvirt-3-format","read-only":true,"driver":"raw","file":"libvirt-3-storage"},"id":"libvirt-382"} 56.322 < 0x7f8b04043030 {"return": {}, "id": "libvirt-382"} 56.322 > 0x7f8b04043030 {"execute":"blockdev-insert-medium","arguments":{"id":"scsi0-0-0","node-name":"libvirt-3-format"},"id":"libvirt-383"} 56.325 < 0x7f8b04043030 {"return": {}, "id": "libvirt-383"} 56.325 > 0x7f8b04043030 {"execute":"block_set_io_throttle","arguments":{"id":"scsi0-0-0","bps":10000,"bps_rd":0,"bps_wr":0,"iops":0,"iops_rd":0,"iops_wr":0,"bps_max":0,"bps_rd_max":0,"bps_wr_max":0,"iops_max":0,"iops_rd_max":0,"iops_wr_max":0,"iops_size":0,"group":"test"},"id":"libvirt-384"} 56.331 < 0x7f8b04043030 {"return": {}, "id": "libvirt-384"} 56.331 > 0x7f8b04043030 {"execute":"blockdev-close-tray","arguments":{"id":"scsi0-0-0"},"id":"libvirt-385"} 56.333 ! 0x7f8b04043030 {"timestamp": {"seconds": 1610013536, "microseconds": 880060}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "", "tray-open": false, "id": "scsi0-0-0"}} 56.333 < 0x7f8b04043030 {"return": {}, "id": "libvirt-385"} So the total_bytes_sec and group_name are set after medium inserted. Eject the media: ➜ ~ virsh change-media test sda --eject Successfully ejected media. Dumpxml: <disk type="file" device="cdrom"> <driver name="qemu" type="raw"/> <source index="4"/> <target dev="sda" bus="scsi"/> <iotune> <total_bytes_sec>10000</total_bytes_sec> <group_name>test</group_name> </iotune> <readonly/> <alias name="scsi0-0-0"/> <address type="drive" controller="0" bus="0" target="0" unit="0"/> </disk> Work as expected. -- Tested-by: Han Han <hhan@redhat.com>