On Mon, Nov 21, 2016 at 06:35:51PM -0500, John Ferlan wrote:
> Add support to read/parse the iotune group setting for qemu.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> 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(¶ms[(*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;
> +
One more thing, you can make the GetString call directly from VIR_STRDUP and
then get rid of the group_name variable.
Erik
> 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
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
