[libvirt] [PATCH v4 3/6] qemu: Add bps_max and friends qemu driver
Michal Privoznik
mprivozn at redhat.com
Wed Oct 1 16:04:54 UTC 2014
On 30.09.2014 16:09, Matthias Gatto wrote:
> Add support for bps_max and friends in the driver part.
> In the part checking if a qemu is running, check if the running binary support bps_max,
> if not print an error message, if yes add it to "info" variable
>
> Signed-off-by: Matthias Gatto <matthias.gatto at outscale.com>
> ---
> include/libvirt/libvirt.h.in | 56 +++++++++++++
> src/qemu/qemu_driver.c | 181 +++++++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_monitor.c | 10 ++-
> src/qemu/qemu_monitor.h | 6 +-
> src/qemu/qemu_monitor_json.c | 13 +++-
> src/qemu/qemu_monitor_json.h | 6 +-
> tests/qemumonitorjsontest.c | 4 +-
> 7 files changed, 256 insertions(+), 20 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 4eab987..acaca54 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -5373,6 +5373,62 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
> #define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC "blkdeviotune.write_iops_sec"
>
> /**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX:
> + *
> + * Marco represents the total throughput limit in maximum bytes per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX "blkdeviotune.total_bytes_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX:
> + *
> + * Marco represents the read throughput limit in maximum bytes per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX "blkdeviotune.read_bytes_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX:
> + *
> + * Macro represents the write throughput limit in maximum bytes per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX "blkdeviotune.write_bytes_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX:
> + *
> + * Macro represents the total maximum I/O operations per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX "blkdeviotune.total_iops_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX:
> + *
> + * Macro represents the read maximum I/O operations per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX "blkdeviotune.read_iops_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX:
> + *
> + * Macro represents the write maximum I/O operations per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX "blkdeviotune.write_iops_sec_max"
> +
> +/**
> + * VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC:
> + *
> + * Macro represents the size maximum I/O operations per second,
> + * as VIR_TYPED_PARAM_ULLONG.
> + */
> +#define VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC "blkdeviotune.size_iops_sec"
> +
> +/**
> * virConnectDomainEventTunableCallback:
> * @conn: connection object
> * @dom: domain on which the event occurred
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6606154..94cfa58 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -105,6 +105,7 @@ VIR_LOG_INIT("qemu.qemu_driver");
> #define QEMU_NB_MEM_PARAM 3
>
> #define QEMU_NB_BLOCK_IO_TUNE_PARAM 6
> +#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX 13
Okay, so this makes a little bit more sense now. If we are talking to
newer qemu so that _max attributes are supported, we ought to return 13,
and return 6 otherwise.
>
> #define QEMU_NB_NUMA_PARAM 2
>
> @@ -16273,6 +16274,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> int conf_idx = -1;
> bool set_bytes = false;
> bool set_iops = false;
> + bool set_bytes_max = false;
> + bool set_iops_max = false;
> + bool set_size_iops = false;
> virQEMUDriverConfigPtr cfg = NULL;
> virCapsPtr caps = NULL;
> virObjectEventPtr event = NULL;
> @@ -16295,6 +16299,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> VIR_TYPED_PARAM_ULLONG,
> VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
> VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
> + VIR_TYPED_PARAM_ULLONG,
> NULL) < 0)
> return -1;
>
> @@ -16387,6 +16405,69 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC,
> param->value.ul) < 0)
> goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX)) {
> + info.total_bytes_sec_max = param->value.ul;
> + set_bytes_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX)) {
> + info.read_bytes_sec_max = param->value.ul;
> + set_bytes_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX)) {
> + info.write_bytes_sec_max = param->value.ul;
> + set_bytes_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX)) {
> + info.total_iops_sec_max = param->value.ul;
> + set_iops_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX)) {
> + info.read_iops_sec_max = param->value.ul;
> + set_iops_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX)) {
> + info.write_iops_sec_max = param->value.ul;
> + set_iops_max = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX,
> + param->value.ul) < 0)
> + goto endjob;
> + } else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC)) {
> + info.size_iops_sec = param->value.ul;
> + set_size_iops = true;
> + if (virTypedParamsAddULLong(&eventParams, &eventNparams,
> + &eventMaxparams,
> + VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC,
> + param->value.ul) < 0)
> + goto endjob;
> }
> }
>
> @@ -16404,6 +16485,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> goto endjob;
> }
>
> + if ((info.total_bytes_sec_max && info.read_bytes_sec_max) ||
> + (info.total_bytes_sec_max && info.write_bytes_sec_max)) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("total and read/write of bytes_sec_max cannot be set at the same time"));
> + goto endjob;
> + }
> +
> + if ((info.total_iops_sec_max && info.read_iops_sec_max) ||
> + (info.total_iops_sec_max && info.write_iops_sec_max)) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("total and read/write of iops_sec_max cannot be set at the same time"));
> + goto endjob;
> + }
> +
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> if ((conf_idx = virDomainDiskIndexByName(persistentDef, disk, true)) < 0) {
> virReportError(VIR_ERR_INVALID_ARG,
> @@ -16421,6 +16516,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> goto endjob;
> }
>
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX) && (set_iops_max || set_bytes_max)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("a block I/O throttling parameter is not supported with this "
> + "QEMU binary"));
> + goto endjob;
> + }
> +
> if (!(device = qemuDiskPathToAlias(vm, disk, &idx)))
> goto endjob;
>
> @@ -16433,13 +16535,25 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> info.read_bytes_sec = oldinfo->read_bytes_sec;
> info.write_bytes_sec = oldinfo->write_bytes_sec;
> }
> + if (!set_bytes_max) {
> + info.total_bytes_sec_max = oldinfo->total_bytes_sec_max;
> + info.read_bytes_sec_max = oldinfo->read_bytes_sec_max;
> + info.write_bytes_sec_max = oldinfo->write_bytes_sec_max;
> + }
> if (!set_iops) {
> info.total_iops_sec = oldinfo->total_iops_sec;
> info.read_iops_sec = oldinfo->read_iops_sec;
> info.write_iops_sec = oldinfo->write_iops_sec;
> }
> + if (!set_iops_max) {
> + info.total_iops_sec_max = oldinfo->total_iops_sec_max;
> + info.read_iops_sec_max = oldinfo->read_iops_sec_max;
> + info.write_iops_sec_max = oldinfo->write_iops_sec_max;
> + }
> + if (!set_size_iops)
> + info.size_iops_sec = oldinfo->size_iops_sec;
> qemuDomainObjEnterMonitor(driver, vm);
> - ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info);
> + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX));
The last argument should be rather turned into a variable - like you're
doing later in the patch.
> qemuDomainObjExitMonitor(driver, vm);
> if (ret < 0)
> goto endjob;
> @@ -16483,6 +16597,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> }
>
> endjob:
> +
> if (!qemuDomainObjEndJob(driver, vm))
> vm = NULL;
>
> @@ -16513,6 +16628,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> int ret = -1;
> size_t i;
> virCapsPtr caps = NULL;
> + bool supportMaxOptions = true;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> VIR_DOMAIN_AFFECT_CONFIG |
> @@ -16531,8 +16647,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> goto cleanup;
>
> if ((*nparams) == 0) {
> - /* Current number of parameters supported by QEMU Block I/O Throttling */
> - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
> + /* Current number of parameters supported by QEMU Block I/O Throttling including the optionals parameters */
> + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
So this should rather be *nparams = supportMaxOptions ?
QEMU_.._PARAM_MAX : QEMU_.._PARAM;
And also the long lines should be capped.
> ret = 0;
> goto cleanup;
> }
> @@ -16551,8 +16667,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
>
> if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> priv = vm->privateData;
> + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
and this check should be moved before first supportMaxOptions usage.
> + if (!supportMaxOptions)
> + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
> qemuDomainObjEnterMonitor(driver, vm);
> - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply);
> + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions);
> qemuDomainObjExitMonitor(driver, vm);
> if (ret < 0)
> goto endjob;
> @@ -16569,7 +16688,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> reply = persistentDef->disks[idx]->blkdeviotune;
> }
>
> - for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) {
> + for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) {
> virTypedParameterPtr param = ¶ms[i];
>
> switch (i) {
> @@ -16615,14 +16734,64 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> reply.write_iops_sec) < 0)
> goto endjob;
> break;
> + case 6:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.total_bytes_sec_max) < 0)
> + goto endjob;
> + break;
> + case 7:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.read_bytes_sec_max) < 0)
> + goto endjob;
> + break;
> + case 8:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.write_bytes_sec_max) < 0)
> + goto endjob;
> + break;
> + case 9:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.total_iops_sec_max) < 0)
> + goto endjob;
> + break;
> + case 10:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.read_iops_sec_max) < 0)
> + goto endjob;
> + break;
> + case 11:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.write_iops_sec_max) < 0)
> + goto endjob;
> + break;
> + case 12:
> + if (virTypedParameterAssign(param,
> + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
> + VIR_TYPED_PARAM_ULLONG,
> + reply.size_iops_sec) < 0)
> + goto endjob;
> /* coverity[dead_error_begin] */
> default:
> break;
> }
> }
>
> - if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM)
> + if (!supportMaxOptions && *nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM)
> *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
> + else if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX)
> + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
> ret = 0;
>
> endjob:
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c25f002..b57a0d8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3416,14 +3416,15 @@ qemuMonitorBlockJobInfo(qemuMonitorPtr mon,
>
> int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
> const char *device,
> - virDomainBlockIoTuneInfoPtr info)
> + virDomainBlockIoTuneInfoPtr info,
> + bool supportMaxOptions)
> {
> int ret;
>
> VIR_DEBUG("mon=%p, device=%p, info=%p", mon, device, info);
>
> if (mon->json) {
> - ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info);
> + ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions);
> } else {
> ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info);
> }
> @@ -3432,14 +3433,15 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
>
> int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
> const char *device,
> - virDomainBlockIoTuneInfoPtr reply)
> + virDomainBlockIoTuneInfoPtr reply,
> + bool supportMaxOptions)
> {
> int ret;
>
> VIR_DEBUG("mon=%p, device=%p, reply=%p", mon, device, reply);
>
> if (mon->json) {
> - ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply);
> + ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, supportMaxOptions);
> } else {
> ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply);
> }
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 6b91e29..ef7719d 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -747,11 +747,13 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
>
> int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
> const char *device,
> - virDomainBlockIoTuneInfoPtr info);
> + virDomainBlockIoTuneInfoPtr info,
> + bool supportMaxOptions);
>
> int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
> const char *device,
> - virDomainBlockIoTuneInfoPtr reply);
> + virDomainBlockIoTuneInfoPtr reply,
> + bool supportMaxOptions);
>
> int qemuMonitorSystemWakeup(qemuMonitorPtr mon);
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a3d7c2c..a8759dd 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4042,13 +4042,15 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
> static int
> qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
> const char *device,
> - virDomainBlockIoTuneInfoPtr reply)
> + virDomainBlockIoTuneInfoPtr reply,
> + bool supportMaxOptions)
> {
> virJSONValuePtr io_throttle;
> int ret = -1;
> size_t i;
> bool found = false;
>
> + (void)supportMaxOptions;
We have the ATTRIBUTE_UNUSED for this.
> io_throttle = virJSONValueObjectGet(result, "return");
>
> if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) {
> @@ -4113,12 +4115,14 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
>
> int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
> const char *device,
> - virDomainBlockIoTuneInfoPtr info)
> + virDomainBlockIoTuneInfoPtr info,
> + bool supportMaxOptions)
> {
> int ret = -1;
> virJSONValuePtr cmd = NULL;
> virJSONValuePtr result = NULL;
>
> + (void)supportMaxOptions;
And here as well.
Michal
More information about the libvir-list
mailing list