Re: [libvirt] [PATCHv3 5/6] Add virDomain{S, G}etInterfaceParameters support to qemu driver

Wednesday, 28 December 2011

On 2011年12月28日 17:56, Hu Tao wrote:
...
 * src/qemu/qemu_driver.c: implement the qemu driver support
 ---
   src/qemu/qemu_driver.c |  337 ++++++++++++++++++++++++++++++++++++++++++++++++
   1 files changed, 337 insertions(+), 0 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index eeeb935..ce0f91c 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -119,6 +119,8 @@

   #define QEMU_NB_BLKIO_PARAM  2

 +#define QEMU_NB_BANDWIDTH_PARAM 6
 +
   static void processWatchdogEvent(void *data, void *opaque);

   static int qemudShutdown(void);
 @@ -7850,6 +7852,339 @@ qemudDomainInterfaceStats (virDomainPtr dom,
   #endif

   static int
 +qemuDomainSetInterfaceParameters(virDomainPtr dom,
 +                                 const char *device,
 +                                 virTypedParameterPtr params,
 +                                 int nparams,
 +                                 unsigned int flags)
 +{
 +    struct qemud_driver *driver = dom->conn->privateData;
 +    int i;
 +    virCgroupPtr group = NULL;
 +    virDomainObjPtr vm = NULL;
 +    virDomainDefPtr persistentDef = NULL;
 +    int ret = -1;
 +    virDomainNetDefPtr net = NULL, persistentNet = NULL;
 +    virNetDevBandwidthPtr bandwidth;
 +
 +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
 +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
 +    qemuDriverLock(driver);
 +
 +    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
 +
 +    if (vm == NULL) {
 +        qemuReportError(VIR_ERR_INTERNAL_ERROR,
 +                        _("No such domain %s"), dom->uuid);
 +        goto cleanup;
 +    }
 +
 +    if (virDomainLiveConfigHelperMethod(driver->caps, vm,&flags,
 +&persistentDef)<  0)
 +        goto cleanup;
 +
 +    if (flags&  VIR_DOMAIN_AFFECT_LIVE) {
 +        net = virDomainFindNetDef(vm->def, device);
 +        if (!net) {
 +            qemuReportError(VIR_ERR_INVALID_ARG,
 +                            _("Can't find device %s"), device);
 +            ret = -1; 
This is redundant. as "ret" was initialized as "-1", and not changed
yet.

...
 +            goto cleanup;
 +        }
 +    }
 +    if (flags&  VIR_DOMAIN_AFFECT_CONFIG) {
 +        persistentNet = virDomainFindNetDef(persistentDef, device);
 +        if (!persistentNet) {
 +            qemuReportError(VIR_ERR_INVALID_ARG,
 +                            _("Can't find device %s"), device);
 +            ret = -1; 
Likewise.

...
 +            goto cleanup;
 +        }
 +    }
 +
 +    if (VIR_ALLOC(bandwidth)<  0) {
 +        virReportOOMError();
 +        goto cleanup;
 +    }
 +    if (VIR_ALLOC(bandwidth->in)<  0) {
 +        virReportOOMError();
 +        goto cleanup;
 +    }
 +    memset(bandwidth->in, 0, sizeof(*bandwidth->in));
 +    if (VIR_ALLOC(bandwidth->out)<  0) {
 +        virReportOOMError();
 +        goto cleanup;
 +    }
 +    memset(bandwidth->out, 0, sizeof(*bandwidth->out));
 +
 +    for (i = 0; i<  nparams; i++) {
 +        virTypedParameterPtr param =&params[i];
 +
 +        if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth average tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->in->average = params[i].value.ui;
 +        } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth peak tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->in->peak = params[i].value.ui;
 +        } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth burst tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->in->burst = params[i].value.ui;
 +        } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth average tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->out->average = params[i].value.ui;
 +        } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth peak tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->out->peak = params[i].value.ui;
 +        } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) {
 +            if (param->type != VIR_TYPED_PARAM_UINT) {
 +                qemuReportError(VIR_ERR_INVALID_ARG, "%s",
 +                                _("invalid type for bandwidth burst tunable,
expected a 'unsigned int'"));
 +                goto cleanup;
 +            }
 +
 +            bandwidth->out->burst = params[i].value.ui;
 +        } else {
 +            qemuReportError(VIR_ERR_INVALID_ARG,
 +                            _("Parameter `%s' not supported"),
 +                            param->field);
 +            goto cleanup;
 +        }
 +    }
 +
 +    /* average is mandatory, peak and burst is optional. So if no
 +     * average is given, we free inbound/outbound here which causes
 +     * inbound/outbound won't be set. */
 +    if (!bandwidth->in->average)
 +        VIR_FREE(bandwidth->in);
 +    if (!bandwidth->out->average)
 +        VIR_FREE(bandwidth->out);
 +
 +    ret = 0; 
This can be avoided.

...
 +    if (flags&  VIR_DOMAIN_AFFECT_LIVE) {
 +        if (virNetDevBandwidthSet(net->ifname, bandwidth)<  0) {
 +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
 +                            _("cannot set bandwidth limits on %s"),
 +                            device);
 +            ret = -1; 
Can be avoided if "ret = 0" is removed.

...
 +            goto cleanup;
 +        }
 +
 +        if (!net->bandwidth) {
 +            net->bandwidth = bandwidth;
 +            bandwidth = NULL;
 +        } else {
 +            if (bandwidth->in) {
 +                VIR_FREE(net->bandwidth->in);
 +                net->bandwidth->in = bandwidth->in;
 +                bandwidth->in = NULL;
 +            }
 +            if (bandwidth->out) {
 +                VIR_FREE(net->bandwidth->out);
 +                net->bandwidth->out = bandwidth->out;
 +                bandwidth->out = NULL;
 +            }
 +        }
 +    }
 +    if (ret<  0)
 +        goto cleanup; 
This checking can be avoided if previous "ret = 0" is removed.

...
 +    if (flags&  VIR_DOMAIN_AFFECT_CONFIG) {
 +        if (!persistentNet->bandwidth) {
 +            persistentNet->bandwidth = bandwidth;
 +            bandwidth = NULL;
 +        } else {
 +            if (bandwidth->in) {
 +                VIR_FREE(persistentNet->bandwidth->in);
 +                persistentNet->bandwidth->in = bandwidth->in;
 +                bandwidth->in = NULL;
 +            }
 +            if (bandwidth->out) {
 +                VIR_FREE(persistentNet->bandwidth->out);
 +                persistentNet->bandwidth->out = bandwidth->out;
 +                bandwidth->out = NULL;
 +            }
 +        }
 +
 +        if (virDomainSaveConfig(driver->configDir, persistentDef)<  0)
 +            ret = -1; 
All the "ret" use in this function can be simplified as:
           ret = virDomainSaveConfig(driver->configDir, persistentDef);
           goto cleanup;
...
 +    }
 + 
And:
    ret = 0;

> +cleanup:
> +    if (bandwidth) {
> +        VIR_FREE(bandwidth->in);
> +        VIR_FREE(bandwidth->out);
> +        VIR_FREE(bandwidth);
> +    }
> +    virCgroupFree(&group);
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    qemuDriverUnlock(driver);
> +    return ret;
> +}
> +
> +static int
> +qemuDomainGetInterfaceParameters(virDomainPtr dom,
> +                                 const char *device,
> +                                 virTypedParameterPtr params,
> +                                 int *nparams,
> +                                 unsigned int flags)
> +{
> +    struct qemud_driver *driver = dom->conn->privateData;
> +    int i;
> +    virCgroupPtr group = NULL;
> +    virDomainObjPtr vm = NULL;
> +    virDomainDefPtr def = NULL;
> +    virDomainDefPtr persistentDef = NULL;
> +    virDomainNetDefPtr net = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG |
> +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +    qemuDriverLock(driver);
> +
> +    flags&= ~VIR_TYPED_PARAM_STRING_OKAY;
> +
> +    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +
> +    if (vm == NULL) {
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                        _("No such domain %s"), dom->uuid);
> +        goto cleanup;
...
 +    }
 + > +    if (virDomainLiveConfigHelperMethod(driver->caps, vm,&flags,
> +&persistentDef)<  0)
> +        goto cleanup;
> +
> +    if ((*nparams) == 0) {
> +        *nparams = QEMU_NB_BANDWIDTH_PARAM;
> +        ret = 0;
> +        goto cleanup;
...
 +    }
 + > +    def = persistentDef;
> +    if (!def)
> +        def = vm->def;
> +
> +    net = virDomainFindNetDef(def, device);
> +    if (!net) {
> +        qemuReportError(VIR_ERR_INVALID_ARG,
> +                        _("Can't find device %s"), device);
> +        ret = -1;
> +        goto cleanup;
...
 +    }
 + > +    for (i = 0; i<  *nparams&&  i<  QEMU_NB_BANDWIDTH_PARAM;
i++) {
> +        params[i].value.ui = 0;
> +        params[i].type = VIR_TYPED_PARAM_UINT;
> +
> +        switch(i) {
> +        case 0: /* inbound.average */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE) ==
NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_IN_AVERAGE);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->in)
> +                params[i].value.ui = net->bandwidth->in->average;
> +            break;
> +        case 1: /* inbound.peak */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK) ==
NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_IN_PEAK);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->in)
> +                params[i].value.ui = net->bandwidth->in->peak;
> +            break;
> +        case 2: /* inbound.burst */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST) ==
NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_IN_BURST);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->in)
> +                params[i].value.ui = net->bandwidth->in->burst;
> +            break;
> +        case 3: /* outbound.average */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)
== NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->out)
> +                params[i].value.ui = net->bandwidth->out->average;
> +            break;
> +        case 4: /* outbound.peak */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK) ==
NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_OUT_PEAK);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->out)
> +                params[i].value.ui = net->bandwidth->out->peak;
> +            break;
> +        case 5: /* outbound.burst */
> +            if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST) ==
NULL) {
> +                qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                                _("Field name '%s' too long"),
> +                                VIR_DOMAIN_BANDWIDTH_OUT_BURST);
> +                goto cleanup;
> +            }
> +            if (net->bandwidth&&  net->bandwidth->out)
> +                params[i].value.ui = net->bandwidth->out->burst;
> +            break;
> +        default:
> +            break;
> +            /* should not hit here */
> +        }
...
 +    }
 + > +    if (*nparams>  QEMU_NB_BANDWIDTH_PARAM)
> +        *nparams = QEMU_NB_BANDWIDTH_PARAM;
> +    ret = 0;
> +
> +cleanup:
> +    if (group)
> +        virCgroupFree(&group);
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    qemuDriverUnlock(driver);
> +    return ret;
> +}
> +
> +static int
>   qemudDomainMemoryStats (virDomainPtr dom,
>                           struct _virDomainMemoryStat *stats,
>                           unsigned int nr_stats,
> @@ -11640,6 +11975,8 @@ static virDriver qemuDriver = {
>       .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */
>       .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */
>       .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */
> +    .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */
> +    .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */
>   };
> 
> 

ACK with those nits of "ret" using fixed.

Regards,
Osier

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] [PATCHv3 5/6] Add virDomain{S, G}etInterfaceParameters support to qemu driver