[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 2011年12月29日 15:33, Hu Tao wrote:
> * src/qemu/qemu_driver.c: implement the qemu driver support
> ---
>   src/qemu/qemu_driver.c |  353 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 353 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a840c1f..c04a5de 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);
> @@ -7852,6 +7854,355 @@ 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 = virDomainNetFind(vm->def, device);
> +        if (!net) {
> +            qemuReportError(VIR_ERR_INVALID_ARG,
> +                            _("Can't find device %s"), device);
> +            goto cleanup;

This can cause libvirtd to crash, as "bandwidth" is not initialized
yet.

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

Likewise

> +        }
> +    }
> +
> +    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);
> +
> +    if (flags&  VIR_DOMAIN_AFFECT_LIVE) {
> +        virNetDevBandwidthPtr newBandwidth;
> +
> +        if (VIR_ALLOC(newBandwidth)<  0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        memset(newBandwidth, 0, sizeof(newBandwidth));
> +
> +        /* virNetDevBandwidthSet() will clear any previous value of
> +         * bandwidth parameters, so merge with old bandwidth parameters
> +         * here to prevent them from losing. */
> +        if (bandwidth->in || net->bandwidth->in) {

Similiar, as bandwidth->in can be free()ed if no "average" is given.

> +            if (VIR_ALLOC(newBandwidth->in)<  0) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +            if (bandwidth->in)
> +                memcpy(newBandwidth->in, bandwidth->in, sizeof(*newBandwidth->in));
> +            else if (net->bandwidth->in)
> +                memcpy(newBandwidth->in, net->bandwidth->in, sizeof(*newBandwidth->in));
> +        }
> +        if (bandwidth->out || net->bandwidth->out) {

Just like bandwidth->in.

> +            if (VIR_ALLOC(newBandwidth->out)<  0) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +            if (bandwidth->out)
> +                memcpy(newBandwidth->out, bandwidth->out, sizeof(*newBandwidth->out));
> +            else if (net->bandwidth->out)
> +                memcpy(newBandwidth->out, net->bandwidth->out, sizeof(*newBandwidth->out));
> +        }
> +
> +        if (virNetDevBandwidthSet(net->ifname, newBandwidth)<  0) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("cannot set bandwidth limits on %s"),
> +                            device);
> +            ret = -1;
> +            goto cleanup;
> +        }
> +
> +        virNetDevBandwidthFree(net->bandwidth);
> +        net->bandwidth = newBandwidth;
> +    }
> +    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)
> +            goto cleanup;
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    if (bandwidth) {
> +        VIR_FREE(bandwidth->in);
> +        VIR_FREE(bandwidth->out);
> +        VIR_FREE(bandwidth);
> +    }

This can be simplified using virNetDevBandwidthFree.

Will push with attached patch squashed in.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c04a5de..fbaa824 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7867,7 +7867,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
     virDomainDefPtr persistentDef = NULL;
     int ret = -1;
     virDomainNetDefPtr net = NULL, persistentNet = NULL;
-    virNetDevBandwidthPtr bandwidth;
+    virNetDevBandwidthPtr bandwidth = NULL;
 
     virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -7898,7 +7898,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
         if (!persistentNet) {
             qemuReportError(VIR_ERR_INVALID_ARG,
                             _("Can't find device %s"), device);
-            ret = -1;
             goto cleanup;
         }
     }
@@ -7980,10 +7979,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
     /* 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)
+    if (!bandwidth->in->average) {
         VIR_FREE(bandwidth->in);
-    if (!bandwidth->out->average)
+        bandwidth->in = NULL;
+    }
+    if (!bandwidth->out->average) {
         VIR_FREE(bandwidth->out);
+        bandwidth->out = NULL;
+    }
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
         virNetDevBandwidthPtr newBandwidth;
@@ -8023,7 +8026,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("cannot set bandwidth limits on %s"),
                             device);
-            ret = -1;
             goto cleanup;
         }
 
@@ -8053,11 +8055,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 
     ret = 0;
 cleanup:
-    if (bandwidth) {
-        VIR_FREE(bandwidth->in);
-        VIR_FREE(bandwidth->out);
-        VIR_FREE(bandwidth);
-    }
+    virNetDevBandwidthFree(bandwidth);
     virCgroupFree(&group);
     if (vm)
         virDomainObjUnlock(vm);

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]