[libvirt] [PATCH v6 4/7] qemu: Add bps_max and friends qemu driver

Michal Privoznik mprivozn at redhat.com
Fri Nov 7 15:56:21 UTC 2014


On 29.10.2014 13:16, 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-domain.h |  56 ++++++++++++
>   src/qemu/qemu_driver.c           | 187 +++++++++++++++++++++++++++++++++++++--
>   src/qemu/qemu_monitor.c          |  10 ++-
>   src/qemu/qemu_monitor.h          |   6 +-
>   src/qemu/qemu_monitor_json.c     |  11 ++-
>   src/qemu/qemu_monitor_json.h     |   6 +-
>   tests/qemumonitorjsontest.c      |   4 +-
>   7 files changed, 259 insertions(+), 21 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 6146244..652512b 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3251,6 +3251,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"

Yet again, s/#define/# define/.

> +
> +/**
> + * 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 5eccacc..54ba154 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
>
>   #define QEMU_NB_NUMA_PARAM 2
>
> @@ -16520,6 +16521,10 @@ 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;
> +    bool supportMaxOptions = true;
>       virQEMUDriverConfigPtr cfg = NULL;
>       virCapsPtr caps = NULL;
>       virObjectEventPtr event = NULL;
> @@ -16542,6 +16547,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;
>
> @@ -16634,6 +16653,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;
>           }
>       }
>
> @@ -16651,6 +16733,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,
> @@ -16661,6 +16757,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>       }
>
>       if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE);
>           if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                          _("block I/O throttling not supported with this "
> @@ -16668,6 +16765,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>               goto endjob;
>           }
>
> +        if (!supportMaxOptions && (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;
>
> @@ -16680,13 +16784,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, supportMaxOptions);
>           qemuDomainObjExitMonitor(driver, vm);
>           if (ret < 0)
>               goto endjob;
> @@ -16730,6 +16846,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>       }
>
>    endjob:
> +
>       if (!qemuDomainObjEndJob(driver, vm))
>           vm = NULL;
>
> @@ -16753,13 +16870,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
>   {
>       virQEMUDriverPtr driver = dom->conn->privateData;
>       virDomainObjPtr vm = NULL;
> -    qemuDomainObjPrivatePtr priv;
> +    qemuDomainObjPrivatePtr priv = NULL;
>       virDomainDefPtr persistentDef = NULL;
>       virDomainBlockIoTuneInfo reply;
>       char *device = NULL;
>       int ret = -1;
>       size_t i;
>       virCapsPtr caps = NULL;
> +    bool supportMaxOptions = true;
>
>       virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                     VIR_DOMAIN_AFFECT_CONFIG |
> @@ -16778,8 +16896,9 @@ 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
> +         * in the xml config */
> +        *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;

Isn't the max depending on QEMU_CAPS_DRIVE_IOTUNE_MAX capability here? 
And on the domain being up & running too?

>           ret = 0;
>           goto cleanup;
>       }
> @@ -16798,8 +16917,12 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
>
>       if (flags & VIR_DOMAIN_AFFECT_LIVE) {
>           priv = vm->privateData;
> +        /* If the VM is running, we can check if the current VM can use optional parameters or not */
> +        /* We didn't made this check sooner because we need the VM data to do so */
> +        supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
> +        *nparams = supportMaxOptions ? QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX : QEMU_NB_BLOCK_IO_TUNE_PARAM;

I don't think we should overwrite nparams here. I mean, if user told us 
he's interested just in first N params, we shouldn't go any further and 
overwrite memory after the bounds.

>           qemuDomainObjEnterMonitor(driver, vm);
> -        ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply);
> +        ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions);
>           qemuDomainObjExitMonitor(driver, vm);
>           if (ret < 0)
>               goto endjob;
> @@ -16816,7 +16939,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 = &params[i];
>
>           switch (i) {
> @@ -16862,14 +16985,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:

Michal




More information about the libvir-list mailing list