12:22 p.m.
See 2/2
Peter Krempa (2):
qemuDomainSetBlockIoTune: Remove old uninformative comment
qemuDomainSetBlockIoTune: Skip monitor call for empty cdrom
src/qemu/qemu_driver.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
--
2.29.2
12:22 p.m.
New subject: [PATCH 1/2] qemuDomainSetBlockIoTune: Remove old uninformative comment
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 26c6e9b2e1..1283e61785 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16301,9 +16301,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
#undef CHECK_MAX
- /* NB: Let's let QEMU decide how to handle issues with _length
- * via the JSON error code from the block_set_io_throttle call */
-
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid,
&info, supportMaxOptions,
--
2.29.2
12:22 p.m.
New subject: [PATCH 2/2] qemuDomainSetBlockIoTune: Skip monitor call for empty cdrom
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(a)redhat.com>
---
CC: Han Han <hhan(a)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
1:04 p.m.
New subject: [PATCH 2/2] qemuDomainSetBlockIoTune: Skip monitor call for empty cdrom
On Thu, Jan 7, 2021 at 5:23 PM Peter Krempa <pkrempa(a)redhat.com> wrote:
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(a)redhat.com>
1555
days inactive
1555
days old
4 comments
3 participants
participants (3)
-
Han Han -
Ján Tomko -
Peter Krempa