[libvirt] [PATCH 0/2] qemu: Fix incorrect qemuDomainGetBlockIoTune() results

The qemuDomainGetBlockIoTune() function was returning incorrect results as I discovered on Fedora Virt Test Day: https://fedoraproject.org/wiki/QA:Testcase_Virtualization_IO_Throttling The problem is that I/O limits can be set but I only see zeroes when displaying them with "virsh blkdeviotune <domain> <device>" on a domain with two disks. First, we're comparing guest device names ("virtio-blk-0") instead of host device names ("drive-virtio-blk-0"). Second, the JSON monitor code has inverted string compare logic so we were skipping the device we were looking for. These patches make "virsh blkdeviotune" report correct results. Stefan Hajnoczi (2): qemu: Keep QEMU host drive prefix in BlkIoTune qemu: Fix device name comparison in qemuMonitorJSONBlockIoThrottleInfo() src/qemu/qemu_monitor_json.c | 5 +---- src/qemu/qemu_monitor_text.c | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) -- 1.7.12.1

The QEMU -drive id= begins with libvirt's QEMU host drive prefix ("drive-"), which is stripped off in several places two convert between host ("-drive") and guest ("-device") device names. In the case of BlkIoTune it is unnecessary to strip the QEMU host drive prefix because we operate on "info block"/"query-block" output that uses host drive names. Stripping the prefix incorrectly caused string comparisons to fail since we were comparing the guest device name against the host device name. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- src/qemu/qemu_monitor_json.c | 3 --- src/qemu/qemu_monitor_text.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9a1f2dc..e0faacf 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3739,9 +3739,6 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, goto cleanup; } - if(STRPREFIX(current_dev, QEMU_DRIVE_HOST_PREFIX)) - current_dev += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STREQ(current_dev, device)) continue; diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 09f2a92..0e46fd9 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3193,9 +3193,6 @@ qemuMonitorTextParseBlockIoThrottle(const char *result, p = result; while (*p) { - if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) - p += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STREQLEN(p, device, devnamelen) && p[devnamelen] == ':' && p[devnamelen+1] == ' ') { -- 1.7.12.1

The string comparison logic was inverted and matched the first drive that does *not* have the name we search for. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e0faacf..2adc537 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3739,7 +3739,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, goto cleanup; } - if (STREQ(current_dev, device)) + if (STRNEQ(current_dev, device)) continue; found = true; -- 1.7.12.1

On 11/01/2012 11:20 AM, Stefan Hajnoczi wrote:
The qemuDomainGetBlockIoTune() function was returning incorrect results as I discovered on Fedora Virt Test Day: https://fedoraproject.org/wiki/QA:Testcase_Virtualization_IO_Throttling
The problem is that I/O limits can be set but I only see zeroes when displaying them with "virsh blkdeviotune <domain> <device>" on a domain with two disks.
First, we're comparing guest device names ("virtio-blk-0") instead of host device names ("drive-virtio-blk-0"). Second, the JSON monitor code has inverted string compare logic so we were skipping the device we were looking for.
These patches make "virsh blkdeviotune" report correct results.
Stefan Hajnoczi (2): qemu: Keep QEMU host drive prefix in BlkIoTune qemu: Fix device name comparison in qemuMonitorJSONBlockIoThrottleInfo()
ACK series, and I'll push shortly. Nice that the test day is just before the 1.0.0 release! I'll also push into the v0.10.2-maint branch, so this will hit the next F18 build of libvirt. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Stefan Hajnoczi