[libvirt] [PATCH 0/2] Add missing check and test for new blkdeviotune parameters
Found missing 'size_iops_sec' check in qemu_command.c. Add a test for the new blkdeviotune parameters. John Ferlan (2): qemu: Add checks for blkdeviotune 'size_iops_sec' and adjust error qemu: Add tests for new blkdeviotune arguments src/qemu/qemu_command.c | 10 +++-- .../qemuxml2argv-blkdeviotune-max.args | 12 +++++ .../qemuxml2argv-blkdeviotune-max.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml -- 1.9.3
Seems the 'size_iops_sec' was a late add and the checks for whether the field was defined, but unsupported and the maximum size of the field were not being made. Also, adjust blkdeviotune support error message for grammar, spelling (paramater), and remove the "(need QEMU 1.7 or superior)". None of our other similar error messages list which QEMU version is required. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f674ba9..701831e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3676,11 +3676,12 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.write_bytes_sec_max || disk->blkdeviotune.total_iops_sec_max || disk->blkdeviotune.read_iops_sec_max || - disk->blkdeviotune.write_iops_sec_max) && + disk->blkdeviotune.write_iops_sec_max || + disk->blkdeviotune.size_iops_sec) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("there is some block I/O throttling paramater that are not supported with this " - "QEMU binary(need QEMU 1.7 or superior)")); + _("there are some block I/O throttling parameters " + "that are not supported with this QEMU binary")); goto error; } @@ -3695,7 +3696,8 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.write_bytes_sec_max > LLONG_MAX || disk->blkdeviotune.total_iops_sec_max > LLONG_MAX || disk->blkdeviotune.read_iops_sec_max > LLONG_MAX || - disk->blkdeviotune.write_iops_sec_max > LLONG_MAX) { + disk->blkdeviotune.write_iops_sec_max > LLONG_MAX || + disk->blkdeviotune.size_iops_sec > LLONG_MAX) { virReportError(VIR_ERR_OVERFLOW, _("block I/O throttle limit must " "be less than %llu using QEMU"), LLONG_MAX); -- 1.9.3
The recent commit to add support for block_set_io_throttle parameters from version 1.7 of qemu did not add any tests - this adds the tests Signed-off-by: John Ferlan <jferlan@redhat.com> --- .../qemuxml2argv-blkdeviotune-max.args | 12 +++++ .../qemuxml2argv-blkdeviotune-max.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 66 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args new file mode 100644 index 0000000..82948f1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 -S -M pc -m 214 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0,cache=off,\ +bps=5000,iops=6000,bps_max=10000,iops_max=6000 -device \ +ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-1,cache=off,\ +bps_rd=5000,bps_wr=5000,iops_rd=3500,iops_wr=3500,bps_rd_max=5000,\ +bps_wr_max=5000,iops_rd_max=3500,iops_wr_max=3500,iops_size=2000 \ +-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml new file mode 100644 index 0000000..45bcaaa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml @@ -0,0 +1,51 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <iotune> + <total_bytes_sec>5000</total_bytes_sec> + <total_iops_sec>6000</total_iops_sec> + <total_bytes_sec_max>10000</total_bytes_sec_max> + <total_iops_sec_max>6000</total_iops_sec_max> + </iotune> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdb' bus='ide'/> + <iotune> + <read_bytes_sec>5000</read_bytes_sec> + <write_bytes_sec>5000</write_bytes_sec> + <read_iops_sec>3500</read_iops_sec> + <write_iops_sec>3500</write_iops_sec> + <read_bytes_sec_max>5000</read_bytes_sec_max> + <write_bytes_sec_max>5000</write_bytes_sec_max> + <read_iops_sec_max>3500</read_iops_sec_max> + <write_iops_sec_max>3500</write_iops_sec_max> + <size_iops_sec>2000</size_iops_sec> + </iotune> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index fe58a24..c52b647 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1265,6 +1265,9 @@ mymain(void) DO_TEST("numad-static-memory-auto-vcpu", NONE); DO_TEST("blkdeviotune", QEMU_CAPS_NAME, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_IOTUNE); + DO_TEST("blkdeviotune-max", QEMU_CAPS_NAME, QEMU_CAPS_DEVICE, + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_IOTUNE, + QEMU_CAPS_DRIVE_IOTUNE_MAX); DO_TEST("multifunction-pci-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, -- 1.9.3
On 11/12/2014 04:16 PM, John Ferlan wrote:
The recent commit to add support for block_set_io_throttle parameters from version 1.7 of qemu did not add any tests - this adds the tests
Signed-off-by: John Ferlan <jferlan@redhat.com> --- .../qemuxml2argv-blkdeviotune-max.args | 12 +++++ .../qemuxml2argv-blkdeviotune-max.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 66 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml
+ <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdb' bus='ide'/> + <iotune> + <read_bytes_sec>5000</read_bytes_sec> + <write_bytes_sec>5000</write_bytes_sec> + <read_iops_sec>3500</read_iops_sec> + <write_iops_sec>3500</write_iops_sec> + <read_bytes_sec_max>5000</read_bytes_sec_max> + <write_bytes_sec_max>5000</write_bytes_sec_max> + <read_iops_sec_max>3500</read_iops_sec_max> + <write_iops_sec_max>3500</write_iops_sec_max>
It would be nicer to use different values for different options, but coverity already found the copy and paste error :) Jan
+ <size_iops_sec>2000</size_iops_sec> + </iotune> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain>
On 11/12/2014 04:16 PM, John Ferlan wrote:
Found missing 'size_iops_sec' check in qemu_command.c.
Add a test for the new blkdeviotune parameters.
John Ferlan (2): qemu: Add checks for blkdeviotune 'size_iops_sec' and adjust error qemu: Add tests for new blkdeviotune arguments
src/qemu/qemu_command.c | 10 +++-- .../qemuxml2argv-blkdeviotune-max.args | 12 +++++ .../qemuxml2argv-blkdeviotune-max.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml
ACK Jan
On 11/14/2014 04:19 AM, Ján Tomko wrote:
On 11/12/2014 04:16 PM, John Ferlan wrote:
Found missing 'size_iops_sec' check in qemu_command.c.
Add a test for the new blkdeviotune parameters.
John Ferlan (2): qemu: Add checks for blkdeviotune 'size_iops_sec' and adjust error qemu: Add tests for new blkdeviotune arguments
src/qemu/qemu_command.c | 10 +++-- .../qemuxml2argv-blkdeviotune-max.args | 12 +++++ .../qemuxml2argv-blkdeviotune-max.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max.xml
ACK
Jan
I adjusted the xml/args to use different values for each field and pushed the patches. Thanks, John
participants (2)
-
John Ferlan -
Ján Tomko