Add the capability to detect if the qemu binary can support the feature to use throttling.group. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + 9 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cfd090c..02b3ecc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -352,6 +352,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "ivshmem-doorbell", /* 240 */ "query-qmp-schema", "gluster.debug_level", + "drive-iotune-group", ); @@ -2858,6 +2859,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "name", "guest", QEMU_CAPS_NAME_GUEST }, { "spice", "unix", QEMU_CAPS_SPICE_UNIX }, { "drive", "throttling.bps-total-max-length", QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH }, + { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9163572..4d5d201 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -387,6 +387,7 @@ typedef enum { QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL, /* -device ivshmem-doorbell */ QEMU_CAPS_QUERY_QMP_SCHEMA, /* query-qmp-schema command */ QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive gluster.debug_level={0..9} */ + QEMU_CAPS_DRIVE_IOTUNE_GROUP, /* -drive throttling.group=<name> */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index 3162758..e65ebe6 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -181,6 +181,7 @@ <flag name='virtio-pci-disable-legacy'/> <flag name='machine-iommu'/> <flag name='virtio-vga'/> + <flag name='drive-iotune-group'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index bea01c7..cca0361 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -187,6 +187,7 @@ <flag name='machine-iommu'/> <flag name='virtio-vga'/> <flag name='query-qmp-schema'/> + <flag name='drive-iotune-group'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index b283444..b42b690 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -162,6 +162,7 @@ <flag name='ivshmem-plain'/> <flag name='ivshmem-doorbell'/> <flag name='query-qmp-schema'/> + <flag name='drive-iotune-group'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index 612a747..7b1b0e6 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -162,6 +162,7 @@ <flag name='ivshmem-plain'/> <flag name='ivshmem-doorbell'/> <flag name='query-qmp-schema'/> + <flag name='drive-iotune-group'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 5f22ed5..74513f2 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -159,6 +159,7 @@ <flag name='ivshmem-plain'/> <flag name='ivshmem-doorbell'/> <flag name='query-qmp-schema'/> + <flag name='drive-iotune-group'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index e9bd5bb..20fe2e7 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -196,6 +196,7 @@ <flag name='ivshmem-plain'/> <flag name='ivshmem-doorbell'/> <flag name='query-qmp-schema'/> + <flag name='drive-iotune-group'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index b6e34b0..29b9df9 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -198,6 +198,7 @@ <flag name='ivshmem-doorbell'/> <flag name='query-qmp-schema'/> <flag name='gluster.debug_level'/> + <flag name='drive-iotune-group'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> -- 2.7.4

On Mon, Nov 21, 2016 at 06:35:48PM -0500, John Ferlan wrote: I'm curious about the else branch. Can you elaborate more on why do you assume qemu supports all the features when you retrieve a persistentdef vs livedef? From what I understand, this is one of the APIs that you have to call twice, since the RPC layer does not allocate a structure of an appropriate size automatically. So with the first run, you say, please allocate a structure of size maxparams equal _ALL_PARAMS (without knowing that qemu actually supports all these features) and rerun. Then during the second run, you run BLOCK_IOTUNE_ASSIGN on all of them, what am I missing? Erik

On Mon, Nov 28, 2016 at 06:53:35AM -0500, John Ferlan wrote: Nah, I missed this line: reply = disk->blkdeviotune; - it all makes more sense now and all the 'invalid' - more like 'unsupported' - attributes will just be zeroed out automatically during the initial allocation. Thanks anyway, Erik

Rather than have multiple bool values, create a single enum with bits representing what can be set. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++-------------------------- 1 file changed, 54 insertions(+), 59 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 87d219f..73b58d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17338,34 +17338,38 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, return ret; } +typedef enum { + QEMU_BLOCK_IOTUNE_SET_BYTES = 1 << 0, + QEMU_BLOCK_IOTUNE_SET_IOPS = 1 << 1, + QEMU_BLOCK_IOTUNE_SET_BYTES_MAX = 1 << 2, + QEMU_BLOCK_IOTUNE_SET_IOPS_MAX = 1 << 3, + QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS = 1 << 4, + QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH = 1 << 5, + QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH = 1 << 6, +} qemuBlockIoTuneSetFlags; + /* If the user didn't specify bytes limits, inherit previous values; * likewise if the user didn't specify iops limits. */ static void qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, virDomainBlockIoTuneInfoPtr oldinfo, - bool set_bytes, - bool set_iops, - bool set_bytes_max, - bool set_iops_max, - bool set_size_iops, - bool set_bytes_max_length, - bool set_iops_max_length) + qemuBlockIoTuneSetFlags set_flag) { #define SET_IOTUNE_DEFAULTS(BOOL, FIELD) \ - if (!BOOL) { \ + if (!(set_flag & QEMU_BLOCK_IOTUNE_SET_##BOOL)) { \ newinfo->total_##FIELD = oldinfo->total_##FIELD; \ newinfo->read_##FIELD = oldinfo->read_##FIELD; \ newinfo->write_##FIELD = oldinfo->write_##FIELD; \ } - SET_IOTUNE_DEFAULTS(set_bytes, bytes_sec); - SET_IOTUNE_DEFAULTS(set_bytes_max, bytes_sec_max); - SET_IOTUNE_DEFAULTS(set_iops, iops_sec); - SET_IOTUNE_DEFAULTS(set_iops_max, iops_sec_max); + SET_IOTUNE_DEFAULTS(BYTES, bytes_sec); + SET_IOTUNE_DEFAULTS(BYTES_MAX, bytes_sec_max); + SET_IOTUNE_DEFAULTS(IOPS, iops_sec); + SET_IOTUNE_DEFAULTS(IOPS_MAX, iops_sec_max); #undef SET_IOTUNE_DEFAULTS - if (!set_size_iops) + if (!(set_flag & QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS)) newinfo->size_iops_sec = oldinfo->size_iops_sec; /* The length field is handled a bit differently. If not defined/set, @@ -17381,19 +17385,20 @@ qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, * our newinfo is clearing, then set max_length based on whether we * have a value in the family set/defined. */ #define SET_MAX_LENGTH(BOOL, FIELD) \ - if (!BOOL) \ + if (!(set_flag & QEMU_BLOCK_IOTUNE_SET_##BOOL)) \ newinfo->FIELD##_max_length = oldinfo->FIELD##_max_length; \ - else if (BOOL && oldinfo->FIELD##_max_length && \ + else if ((set_flag & QEMU_BLOCK_IOTUNE_SET_##BOOL) && \ + oldinfo->FIELD##_max_length && \ !newinfo->FIELD##_max_length) \ newinfo->FIELD##_max_length = (newinfo->FIELD || \ newinfo->FIELD##_max) ? 1 : 0; - SET_MAX_LENGTH(set_bytes_max_length, total_bytes_sec); - SET_MAX_LENGTH(set_bytes_max_length, read_bytes_sec); - SET_MAX_LENGTH(set_bytes_max_length, write_bytes_sec); - SET_MAX_LENGTH(set_iops_max_length, total_iops_sec); - SET_MAX_LENGTH(set_iops_max_length, read_iops_sec); - SET_MAX_LENGTH(set_iops_max_length, write_iops_sec); + SET_MAX_LENGTH(BYTES_MAX_LENGTH, total_bytes_sec); + SET_MAX_LENGTH(BYTES_MAX_LENGTH, read_bytes_sec); + SET_MAX_LENGTH(BYTES_MAX_LENGTH, write_bytes_sec); + SET_MAX_LENGTH(IOPS_MAX_LENGTH, total_iops_sec); + SET_MAX_LENGTH(IOPS_MAX_LENGTH, read_iops_sec); + SET_MAX_LENGTH(IOPS_MAX_LENGTH, write_iops_sec); #undef SET_MAX_LENGTH @@ -17418,13 +17423,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, size_t i; virDomainDiskDefPtr conf_disk = NULL; virDomainDiskDefPtr disk; - bool set_bytes = false; - bool set_iops = false; - bool set_bytes_max = false; - bool set_iops_max = false; - bool set_size_iops = false; - bool set_bytes_max_length = false; - bool set_iops_max_length = false; + qemuBlockIoTuneSetFlags set_flag = 0; bool supportMaxOptions = true; bool supportMaxLengthOptions = true; virQEMUDriverConfigPtr cfg = NULL; @@ -17502,7 +17501,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, #define SET_IOTUNE_FIELD(FIELD, BOOL, CONST) \ if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_##CONST)) { \ info.FIELD = param->value.ul; \ - BOOL = true; \ + set_flag |= QEMU_BLOCK_IOTUNE_SET_##BOOL; \ if (virTypedParamsAddULLong(&eventParams, &eventNparams, \ &eventMaxparams, \ VIR_DOMAIN_TUNABLE_BLKDEV_##CONST, \ @@ -17521,38 +17520,38 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } - SET_IOTUNE_FIELD(total_bytes_sec, set_bytes, TOTAL_BYTES_SEC); - SET_IOTUNE_FIELD(read_bytes_sec, set_bytes, READ_BYTES_SEC); - SET_IOTUNE_FIELD(write_bytes_sec, set_bytes, WRITE_BYTES_SEC); - SET_IOTUNE_FIELD(total_iops_sec, set_iops, TOTAL_IOPS_SEC); - SET_IOTUNE_FIELD(read_iops_sec, set_iops, READ_IOPS_SEC); - SET_IOTUNE_FIELD(write_iops_sec, set_iops, WRITE_IOPS_SEC); + SET_IOTUNE_FIELD(total_bytes_sec, BYTES, TOTAL_BYTES_SEC); + SET_IOTUNE_FIELD(read_bytes_sec, BYTES, READ_BYTES_SEC); + SET_IOTUNE_FIELD(write_bytes_sec, BYTES, WRITE_BYTES_SEC); + SET_IOTUNE_FIELD(total_iops_sec, IOPS, TOTAL_IOPS_SEC); + SET_IOTUNE_FIELD(read_iops_sec, IOPS, READ_IOPS_SEC); + SET_IOTUNE_FIELD(write_iops_sec, IOPS, WRITE_IOPS_SEC); - SET_IOTUNE_FIELD(total_bytes_sec_max, set_bytes_max, + SET_IOTUNE_FIELD(total_bytes_sec_max, BYTES_MAX, TOTAL_BYTES_SEC_MAX); - SET_IOTUNE_FIELD(read_bytes_sec_max, set_bytes_max, + SET_IOTUNE_FIELD(read_bytes_sec_max, BYTES_MAX, READ_BYTES_SEC_MAX); - SET_IOTUNE_FIELD(write_bytes_sec_max, set_bytes_max, + SET_IOTUNE_FIELD(write_bytes_sec_max, BYTES_MAX, WRITE_BYTES_SEC_MAX); - SET_IOTUNE_FIELD(total_iops_sec_max, set_iops_max, + SET_IOTUNE_FIELD(total_iops_sec_max, IOPS_MAX, TOTAL_IOPS_SEC_MAX); - SET_IOTUNE_FIELD(read_iops_sec_max, set_iops_max, + SET_IOTUNE_FIELD(read_iops_sec_max, IOPS_MAX, READ_IOPS_SEC_MAX); - SET_IOTUNE_FIELD(write_iops_sec_max, set_iops_max, + SET_IOTUNE_FIELD(write_iops_sec_max, IOPS_MAX, WRITE_IOPS_SEC_MAX); - SET_IOTUNE_FIELD(size_iops_sec, set_size_iops, SIZE_IOPS_SEC); + SET_IOTUNE_FIELD(size_iops_sec, SIZE_IOPS, SIZE_IOPS_SEC); - SET_IOTUNE_FIELD(total_bytes_sec_max_length, set_bytes_max_length, + SET_IOTUNE_FIELD(total_bytes_sec_max_length, BYTES_MAX_LENGTH, TOTAL_BYTES_SEC_MAX_LENGTH); - SET_IOTUNE_FIELD(read_bytes_sec_max_length, set_bytes_max_length, + SET_IOTUNE_FIELD(read_bytes_sec_max_length, BYTES_MAX_LENGTH, READ_BYTES_SEC_MAX_LENGTH); - SET_IOTUNE_FIELD(write_bytes_sec_max_length, set_bytes_max_length, + SET_IOTUNE_FIELD(write_bytes_sec_max_length, BYTES_MAX_LENGTH, WRITE_BYTES_SEC_MAX_LENGTH); - SET_IOTUNE_FIELD(total_iops_sec_max_length, set_iops_max_length, + SET_IOTUNE_FIELD(total_iops_sec_max_length, IOPS_MAX_LENGTH, TOTAL_IOPS_SEC_MAX_LENGTH); - SET_IOTUNE_FIELD(read_iops_sec_max_length, set_iops_max_length, + SET_IOTUNE_FIELD(read_iops_sec_max_length, IOPS_MAX_LENGTH, READ_IOPS_SEC_MAX_LENGTH); - SET_IOTUNE_FIELD(write_iops_sec_max_length, set_iops_max_length, + SET_IOTUNE_FIELD(write_iops_sec_max_length, IOPS_MAX_LENGTH, WRITE_IOPS_SEC_MAX_LENGTH); } @@ -17604,7 +17603,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (!supportMaxOptions && - (set_iops_max || set_bytes_max || set_size_iops)) { + (set_flag & (QEMU_BLOCK_IOTUNE_SET_BYTES_MAX | + QEMU_BLOCK_IOTUNE_SET_IOPS_MAX | + QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("a block I/O throttling parameter is not " "supported with this QEMU binary")); @@ -17612,7 +17613,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (!supportMaxLengthOptions && - (set_iops_max_length || set_bytes_max_length)) { + (set_flag & (QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH | + QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("a block I/O throttling length parameter is not " "supported with this QEMU binary")); @@ -17625,11 +17627,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (!(device = qemuAliasFromDisk(disk))) goto endjob; - qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, - set_bytes, set_iops, set_bytes_max, - set_iops_max, set_size_iops, - set_bytes_max_length, - set_iops_max_length); + qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, set_flag); #define CHECK_MAX(val) \ do { \ @@ -17683,10 +17681,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } qemuDomainSetBlockIoTuneDefaults(&info, &conf_disk->blkdeviotune, - set_bytes, set_iops, set_bytes_max, - set_iops_max, set_size_iops, - set_bytes_max_length, - set_iops_max_length); + set_flag); conf_disk->blkdeviotune = info; ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef); if (ret < 0) -- 2.7.4

On 11/25/2016 10:53 AM, Erik Skultety wrote: I could go with set_mask, but mask has a different connotation for some. It's not a bitmap and there are those that would say that a bitmap should use the Bitmap functions (which would be overkill at this point). Maybe a 'set_fields_flag' would be clearer? It's a flag to indicate a grouping of (usually) 3 elements ({read|write|total}) were set, where size_iops and group_name become the exception. Unless someone else has a brilliant suggestion... John

Add support to read/parse the iotune group setting for qemu. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 8 ++++ src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 45 +++++++++++++++++++- src/qemu/qemu_monitor.c | 2 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 14 ++++++- src/qemu/qemu_monitor_json.h | 1 + tests/qemumonitorjsontest.c | 88 ++++++++++++++++++++++++++++++++-------- 9 files changed, 140 insertions(+), 21 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8c9876c..1212e5a 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3854,6 +3854,14 @@ typedef void (*virConnectDomainEventJobCompletedCallback)(virConnectPtr conn, # define VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC "blkdeviotune.size_iops_sec" /** + * VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME: + * + * Macro represents the group name to be used, + * as VIR_TYPED_PARAM_STRING. + */ +# define VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME "blkdeviotune.group_name" + +/** * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH: * * Macro represents the length in seconds allowed for a burst period diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e008e2..88bd098 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1649,6 +1649,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->vendor); VIR_FREE(def->product); VIR_FREE(def->domain_name); + VIR_FREE(def->blkdeviotune.group_name); virDomainDeviceInfoClear(&def->info); virObjectUnref(def->privateData); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 541b600..152a6a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -548,6 +548,7 @@ struct _virDomainBlockIoTuneInfo { unsigned long long read_iops_sec_max; unsigned long long write_iops_sec_max; unsigned long long size_iops_sec; + char *group_name; unsigned long long total_bytes_sec_max_length; unsigned long long read_bytes_sec_max_length; unsigned long long write_bytes_sec_max_length; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 73b58d0..7f17975 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -115,8 +115,10 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS 6 #define QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS 7 #define QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS 6 +#define QEMU_NB_BLOCK_IO_TUNE_GROUP_PARAMS 1 #define QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS (QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS + \ QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS + \ + QEMU_NB_BLOCK_IO_TUNE_GROUP_PARAMS + \ QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS) #define QEMU_NB_NUMA_PARAM 2 @@ -17344,8 +17346,9 @@ typedef enum { QEMU_BLOCK_IOTUNE_SET_BYTES_MAX = 1 << 2, QEMU_BLOCK_IOTUNE_SET_IOPS_MAX = 1 << 3, QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS = 1 << 4, - QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH = 1 << 5, - QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH = 1 << 6, + QEMU_BLOCK_IOTUNE_SET_GROUP_NAME = 1 << 5, + QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH = 1 << 6, + QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH = 1 << 7, } qemuBlockIoTuneSetFlags; @@ -17371,6 +17374,8 @@ qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, if (!(set_flag & QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS)) newinfo->size_iops_sec = oldinfo->size_iops_sec; + if (!(set_flag & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME)) + VIR_STEAL_PTR(newinfo->group_name, oldinfo->group_name); /* The length field is handled a bit differently. If not defined/set, * QEMU will default these to 0 or 1 depending on whether something in @@ -17425,6 +17430,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainDiskDefPtr disk; qemuBlockIoTuneSetFlags set_flag = 0; bool supportMaxOptions = true; + bool supportGroupNameOption = true; bool supportMaxLengthOptions = true; virQEMUDriverConfigPtr cfg = NULL; virObjectEventPtr event = NULL; @@ -17461,6 +17467,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, + VIR_TYPED_PARAM_STRING, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, @@ -17540,6 +17548,16 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, SET_IOTUNE_FIELD(write_iops_sec_max, IOPS_MAX, WRITE_IOPS_SEC_MAX); SET_IOTUNE_FIELD(size_iops_sec, SIZE_IOPS, SIZE_IOPS_SEC); + if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME)) { + if (VIR_STRDUP(info.group_name, params->value.s) < 0) + goto endjob; + set_flag |= QEMU_BLOCK_IOTUNE_SET_GROUP_NAME; + if (virTypedParamsAddString(&eventParams, &eventNparams, + &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME, + param->value.s) < 0) + goto endjob; + } SET_IOTUNE_FIELD(total_bytes_sec_max_length, BYTES_MAX_LENGTH, TOTAL_BYTES_SEC_MAX_LENGTH); @@ -17592,6 +17610,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (def) { supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); + supportGroupNameOption = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_DRIVE_IOTUNE_GROUP); supportMaxLengthOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH); @@ -17612,6 +17632,14 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } + if (!supportGroupNameOption && + (set_flag & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the block I/O throttling group parameter is not " + "supported with this QEMU binary")); + goto endjob; + } + if (!supportMaxLengthOptions && (set_flag & (QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH | QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH))) { @@ -17655,12 +17683,14 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, supportMaxOptions, + supportGroupNameOption, supportMaxLengthOptions); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret < 0) goto endjob; disk->blkdeviotune = info; + info.group_name = NULL; ret = virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps); if (ret < 0) @@ -17683,6 +17713,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainSetBlockIoTuneDefaults(&info, &conf_disk->blkdeviotune, set_flag); conf_disk->blkdeviotune = info; + info.group_name = NULL; ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef); if (ret < 0) goto endjob; @@ -17693,6 +17724,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: + VIR_FREE(info.group_name); VIR_FREE(device); virDomainObjEndAPI(&vm); if (eventNparams) @@ -17754,6 +17786,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, maxparams = QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) maxparams += QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_GROUP)) + maxparams += QEMU_NB_BLOCK_IO_TUNE_GROUP_PARAMS; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) maxparams += QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS; } else { @@ -17821,6 +17855,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, BLOCK_IOTUNE_ASSIGN(SIZE_IOPS_SEC, size_iops_sec); + if (*nparams < maxparams && + virTypedParameterAssign(&params[(*nparams)++], + VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, + VIR_TYPED_PARAM_STRING, + reply.group_name) < 0) + goto endjob; + BLOCK_IOTUNE_ASSIGN(TOTAL_BYTES_SEC_MAX_LENGTH, total_bytes_sec_max_length); BLOCK_IOTUNE_ASSIGN(READ_BYTES_SEC_MAX_LENGTH, read_bytes_sec_max_length); BLOCK_IOTUNE_ASSIGN(WRITE_BYTES_SEC_MAX_LENGTH, write_bytes_sec_max_length); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3ff31e4..42c9dd2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3468,6 +3468,7 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, + bool supportGroupNameOption, bool supportMaxLengthOptions) { VIR_DEBUG("device=%p, info=%p", device, info); @@ -3477,6 +3478,7 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, if (mon->json) return qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions, + supportGroupNameOption, supportMaxLengthOptions); else return qemuMonitorTextSetBlockIoThrottle(mon, device, info); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 100730b..a9da71d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -880,6 +880,7 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, + bool supportGroupNameOption, bool supportMaxLengthOptions); int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c2b81b7..aa6ded7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4496,6 +4496,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i); virJSONValuePtr inserted; const char *current_dev; + const char *group_name; if (!temp_dev || temp_dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4521,7 +4522,6 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, "was not in expected format")); goto cleanup; } - GET_THROTTLE_STATS("bps", total_bytes_sec); GET_THROTTLE_STATS("bps_rd", read_bytes_sec); GET_THROTTLE_STATS("bps_wr", write_bytes_sec); @@ -4535,6 +4535,11 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, GET_THROTTLE_STATS_OPTIONAL("iops_rd_max", read_iops_sec_max); GET_THROTTLE_STATS_OPTIONAL("iops_wr_max", write_iops_sec_max); GET_THROTTLE_STATS_OPTIONAL("iops_size", size_iops_sec); + + if ((group_name = virJSONValueObjectGetString(inserted, "group")) && + VIR_STRDUP(reply->group_name, group_name) < 0) + goto cleanup; + GET_THROTTLE_STATS_OPTIONAL("bps_max_length", total_bytes_sec_max_length); GET_THROTTLE_STATS_OPTIONAL("bps_rd_max_length", read_bytes_sec_max_length); GET_THROTTLE_STATS_OPTIONAL("bps_wr_max_length", write_bytes_sec_max_length); @@ -4563,6 +4568,7 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, + bool supportGroupNameOption, bool supportMaxLengthOptions) { int ret = -1; @@ -4596,6 +4602,12 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, NULL) < 0) goto cleanup; + if (supportGroupNameOption && + virJSONValueObjectAdd(args, + "s:group", info->group_name, + NULL) < 0) + goto cleanup; + if (supportMaxLengthOptions && virJSONValueObjectAdd(args, "P:bps_max_length", diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4780d53..adff0c3 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -328,6 +328,7 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, + bool supportGroupNameOption, bool supportMaxLengthOptions); int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ed4190b..c6bcf80 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -67,12 +67,13 @@ const char *queryBlockReply = " \"iops_rd_max\": 11," " \"iops_wr_max\": 12," " \"iops_size\": 13," -" \"bps_max_length\": 14," -" \"bps_rd_max_length\": 15," -" \"bps_wr_max_length\": 16," -" \"iops_max_length\": 17," -" \"iops_rd_max_length\": 18," -" \"iops_wr_max_length\": 19," +" \"group\": \"group14\"," +" \"bps_max_length\": 15," +" \"bps_rd_max_length\": 16," +" \"bps_wr_max_length\": 17," +" \"iops_max_length\": 18," +" \"iops_rd_max_length\": 19," +" \"iops_wr_max_length\": 20," " \"file\": \"/home/zippy/work/tmp/gentoo.qcow2\"," " \"encryption_key_missing\": false" " }," @@ -1991,6 +1992,55 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data) return ret; } + +static int +testValidateGetBlockIoThrottle(virDomainBlockIoTuneInfo info, + virDomainBlockIoTuneInfo expectedInfo) +{ +#define VALIDATE_IOTUNE(field) \ + if (info.field != expectedInfo.field) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "info.%s=%llu != expected=%llu", \ + #field, info.field, expectedInfo.field); \ + return -1; \ + } \ + if (info.field##_max != expectedInfo.field##_max) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "info.%s_max=%llu != expected=%llu", \ + #field, info.field##_max, expectedInfo.field##_max); \ + return -1; \ + } \ + if (info.field##_max_length != expectedInfo.field##_max_length) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "info.%s_max_length=%llu != expected=%llu", \ + #field, info.field##_max_length, \ + expectedInfo.field##_max_length); \ + return -1; \ + } + VALIDATE_IOTUNE(total_bytes_sec); + VALIDATE_IOTUNE(read_bytes_sec); + VALIDATE_IOTUNE(write_bytes_sec); + VALIDATE_IOTUNE(total_iops_sec); + VALIDATE_IOTUNE(read_iops_sec); + VALIDATE_IOTUNE(write_iops_sec); + if (info.size_iops_sec != expectedInfo.size_iops_sec) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "info.size_iops_sec=%llu != expected=%llu", + info.size_iops_sec, expectedInfo.size_iops_sec); + return -1; + } + if (STRNEQ(info.group_name, expectedInfo.group_name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "info.group_name=%s != expected=%s", + info.group_name, expectedInfo.group_name); + return -1; + } +#undef VALIDATE_IOTUNE + + return 0; +} + + static int testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) { @@ -2002,7 +2052,9 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) if (!test) return -1; - expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, NULL, 15, 16, 17, 18, 19, 20}; + if (VIR_STRDUP(expectedInfo.group_name, "group14") < 0) + return -1; if (qemuMonitorTestAddItem(test, "query-block", queryBlockReply) < 0 || qemuMonitorTestAddItemParams(test, "block_set_io_throttle", @@ -2014,12 +2066,13 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) "bps_wr_max", "9", "iops_max", "10", "iops_rd_max", "11", "iops_wr_max", "12", "iops_size", "13", - "bps_max_length", "14", - "bps_rd_max_length", "15", - "bps_wr_max_length", "16", - "iops_max_length", "17", - "iops_rd_max_length", "18", - "iops_wr_max_length", "19", + "group", "\"group14\"", + "bps_max_length", "15", + "bps_rd_max_length", "16", + "bps_wr_max_length", "17", + "iops_max_length", "18", + "iops_rd_max_length", "19", + "iops_wr_max_length", "20", NULL, NULL) < 0) goto cleanup; @@ -2027,19 +2080,18 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) "drive-virtio-disk0", &info) < 0) goto cleanup; - if (memcmp(&info, &expectedInfo, sizeof(info)) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - "Invalid @info"); + if (testValidateGetBlockIoThrottle(info, expectedInfo) < 0) goto cleanup; - } if (qemuMonitorJSONSetBlockIoThrottle(qemuMonitorTestGetMonitor(test), "drive-virtio-disk1", &info, true, - true) < 0) + true, true) < 0) goto cleanup; ret = 0; cleanup: + VIR_FREE(info.group_name); + VIR_FREE(expectedInfo.group_name); qemuMonitorTestFree(test); return ret; } -- 2.7.4

On Mon, Nov 21, 2016 at 06:35:51PM -0500, John Ferlan wrote: One more thing, you can make the GetString call directly from VIR_STRDUP and then get rid of the group_name variable. Erik

On Mon, Nov 28, 2016 at 05:14:54PM -0500, John Ferlan wrote: You've got the parentheses wrong...in both cases... Also, once you do what I'm suggesting, the crash from 6/9 will be resolved, however I still would like to see the @reply structure in qemuDomainGetBlockIoTune initialized properly on the stack. Erik

On Tue, Nov 29, 2016 at 07:28:51AM -0500, John Ferlan wrote: The returned JSON doesn't indicate any default on my qemu 2.6.0: {"return":[{"io-status":"ok","device":"drive-virtio-disk0","locked":false,"removable":false,"inserted":{"iops_rd":0,"detect_zeroes":"off","image":{"virtual-size":21474836480,"filename":"/var/lib/libvirt/images/f23-a.qcow2","cluster-size":65536,"format":"qcow2","actual-size":8880848896,"format-specific":{"type":"qcow2","data":{"compat":"1.1","lazy-refcounts":false,"refcount-bits":16,"corrupt":false}},"dirty-flag":false},"iops_wr":0,"ro":false,"node-name":"#block190","backing_file_depth":0,"drv":"qcow2","iops":0,"bps_wr":0,"write_threshold":0,"encrypted":false,"bps":0,"bps_rd":0,"cache":{"no-flush":false,"direct":false,"writeback":true},"file":"/var/lib/libvirt/images/f23-a.qcow2","encryption_key_missing":false},"type":"unknown"},{"io-status":"ok","device":"drive-virtio-disk1","locked":false,"removable":false,"inserted":{"iops_rd":0,"detect_zeroes":"off","image":{"virtual-size":41943040000,"filename":"/var/lib/libvirt/images/dummy.img","format":"raw","actual-size":41943048192,"dirty-flag":false},"iops_wr":0,"ro":false,"node-name":"#block398","backing_file_depth":0,"drv":"raw","iops":0,"bps_wr":0,"write_threshold":0,"encrypted":false,"bps":0,"bps_rd":0,"cache":{"no-flush":false,"direct":false,"writeback":true},"file":"/var/lib/libvirt/images/dummy.img","encryption_key_missing":false},"type":"unknown"},{"io-status":"ok","device":"drive-virtio-disk2","locked":false,"removable":false,"inserted":{"iops_rd":0,"detect_zeroes":"off","image":{"virtual-size":52428800,"filename":"/var/lib/libvirt/images/vol-qcow2.img","cluster-size":65536,"format":"qcow2","actual-size":274432,"format-specific":{"type":"qcow2","data":{"compat":"1.1","lazy-refcounts":false,"refcount-bits":16,"corrupt":false}},"dirty-flag":false},"iops_wr":0,"ro":false,"node-name":"#block549","backing_file_depth":0,"drv":"qcow2","iops":0,"bps_wr":0,"write_threshold":0,"encrypted":false,"bps":0,"bps_rd":0,"cache":{"no-flush":false,"direct":false,"writeback":true},"file":"/var/lib/libvirt/images/vol-qcow2.img","encryption_key_missing":false},"type":"unknown"}],"id":"libvirt-9"} What I did was just to start a domain that had no block iotuning turned on whatsoever and when I saw the code I was like, wait, I need to try that as it looked suspicious at first glance (since at that moment I'd already known about the missing initialization) so I tried it and it crashed. Yeah, that would do, sure, but is there a problem with a plain static initialization at the function entry point? I mean that's where we (most of the time) put all of our initializations and also where everyone would actually expect such an assignment to take place... Erik

Modify _virDomainBlockIoTuneInfo and rng schema to support the group_name option for iotune throttling. Document the new value. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatdomain.html.in | 11 ++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 9 ++++ .../qemuxml2argv-blkdeviotune-group-num.xml | 61 ++++++++++++++++++++++ .../qemuxml2xmlout-blkdeviotune-group-num.xml | 1 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 88 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-group-num.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4e40aa1..227e6f8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2622,6 +2622,17 @@ <span class="since">Throughput limits since 1.2.11 and QEMU 1.7</span> </p> </dd> + <dt><code>group_name</code></dt> + <dd>The optional <code>group_name</code> provides the cability + to share I/O throttling quota between multiple drives. This + prevents end-users from circumventing a hosting provider's + throttling policy by splitting 1 large drive in N small drives + and getting N times the normal throttling quota. Any name may + be used. + <p> + <span class="since">group_name since 2.5.0 and QEMU 2.4</span> + </p> + </dd> <dt><code>total_bytes_sec_max_length</code></dt> <dd>The optional <code>total_bytes_sec_max_length</code> element is the maximum duration in seconds for the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 19d45fd..c44b414 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4974,6 +4974,11 @@ <data type="unsignedLong"/> </element> </optional> + <optional> + <element name="group_name"> + <text/> + </element> + </optional> <choice> <element name="total_bytes_sec_max_length"> <data type="unsignedLong"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88bd098..714582b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7217,6 +7217,9 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def, PARSE_IOTUNE(read_iops_sec_max_length); PARSE_IOTUNE(write_iops_sec_max_length); + def->blkdeviotune.group_name = + virXPathString("string(./iotune/group_name)", ctxt); + if ((def->blkdeviotune.total_bytes_sec && def->blkdeviotune.read_bytes_sec) || (def->blkdeviotune.total_bytes_sec && @@ -20426,6 +20429,7 @@ virDomainDiskDefFormat(virBufferPtr buf, def->blkdeviotune.read_iops_sec_max || def->blkdeviotune.write_iops_sec_max || def->blkdeviotune.size_iops_sec || + def->blkdeviotune.group_name || def->blkdeviotune.total_bytes_sec_max_length || def->blkdeviotune.read_bytes_sec_max_length || def->blkdeviotune.write_bytes_sec_max_length || @@ -20454,6 +20458,11 @@ virDomainDiskDefFormat(virBufferPtr buf, def->blkdeviotune.size_iops_sec); } + if (def->blkdeviotune.group_name) { + virBufferEscapeString(buf, "<group_name>%s</group_name>\n", + def->blkdeviotune.group_name); + } + FORMAT_IOTUNE(total_bytes_sec_max_length); FORMAT_IOTUNE(read_bytes_sec_max_length); FORMAT_IOTUNE(write_bytes_sec_max_length); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml new file mode 100644 index 0000000..e2a9999 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml @@ -0,0 +1,61 @@ +<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>11000</total_iops_sec_max> + <group_name>libvirt_iotune_group1</group_name> + </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>5500</write_bytes_sec> + <read_iops_sec>3500</read_iops_sec> + <write_iops_sec>4000</write_iops_sec> + <read_bytes_sec_max>6000</read_bytes_sec_max> + <write_bytes_sec_max>6500</write_bytes_sec_max> + <read_iops_sec_max>7000</read_iops_sec_max> + <write_iops_sec_max>7500</write_iops_sec_max> + <size_iops_sec>2000</size_iops_sec> + <group_name>libvirt_iotune_group2</group_name> + </iotune> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-group-num.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-group-num.xml new file mode 120000 index 0000000..2ed9a68 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-group-num.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e22b63f..e338ad0 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -611,6 +611,7 @@ mymain(void) DO_TEST("usb-redir-filter-version", NONE); DO_TEST("blkdeviotune", NONE); DO_TEST("blkdeviotune-max", NONE); + DO_TEST("blkdeviotune-group-num", NONE); DO_TEST("blkdeviotune-max-length", NONE); DO_TEST("controller-usb-order", NONE); -- 2.7.4

https://bugzilla.redhat.com/show_bug.cgi?id=1336564 Add the ability to set/display the group_name for block device iotune Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tools/virsh-domain.c | 17 +++++++++++++++++ tools/virsh.pod | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a133c81..4be0b95 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1263,6 +1263,10 @@ static const vshCmdOptDef opts_blkdeviotune[] = { .type = VSH_OT_INT, .help = N_("I/O size in bytes") }, + {.name = "group_name", + .type = VSH_OT_STRING, + .help = N_("group name to share I/O quota between multiple drives") + }, {.name = "total_bytes_sec_max_length", .type = VSH_OT_ALIAS, .help = "total-bytes-sec-max-length" @@ -1322,6 +1326,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; const char *name, *disk; + const char *group_name = NULL; unsigned long long value; int nparams = 0; int maxparams = 0; @@ -1393,6 +1398,18 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) VSH_ADD_IOTUNE(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH); #undef VSH_ADD_IOTUNE + rv = vshCommandOptStringReq(ctl, cmd, "group_name", &group_name); + if (rv < 0) { + vshError(ctl, "%s", _("Unable to parse group parameter")); + goto cleanup; + } else if (rv > 0) { + if (virTypedParamsAddString(&params, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, + group_name) < 0) + goto save_error; + } + + if (nparams == 0) { if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) { vshError(ctl, "%s", diff --git a/tools/virsh.pod b/tools/virsh.pod index f278fec..e98cd0f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1139,7 +1139,7 @@ command. [I<read-bytes-sec-max-length>] [I<write-bytes-sec-max-length>]] [[I<total-iops-sec-max-length>] | [I<read-iops-sec-max-length>] [I<write-iops-sec-max-length>]] -[I<size-iops-sec>] +[I<size-iops-sec>] [I<group_name>] Set or query the block disk io parameters for a block device of I<domain>. I<device> specifies a unique target name (<target dev='name'/>) or source @@ -1179,6 +1179,9 @@ read I/O operations limit. I<--write-iops-sec-max-length> specifies duration in seconds to allow maximum write I/O operations limit. I<--size-iops-sec> specifies size I/O operations limit per second. +I<--group_name> specifies group name to share I/O quota between multiple drives. +For a qemu domain, if no name is provided, then the default is to have a single +group for each I<device>. Older versions of virsh only accepted these options with underscore instead of dash, as in I<--total_bytes_sec>. -- 2.7.4