Re: [libvirt] [PATCH] support setting bandwidth from virsh attach-interface

On 10/13/2011 02:52 PM, Hu Tao wrote: >Adds two options, inbound and outbound, to attach-interface to set >bandwidth when attaching interfaces >--- > tools/virsh.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++- > tools/virsh.pod | 5 ++- > 2 files changed, 98 insertions(+), 3 deletions(-) > >diff --git a/tools/virsh.c b/tools/virsh.c >index bcf0603..d6c9450 100644 >--- a/tools/virsh.c >+++ b/tools/virsh.c >@@ -59,6 +59,7 @@ > #include "threads.h" > #include "command.h" > #include "virkeycode.h" >+#include "network.h" > > static char *progname; > >@@ -11225,15 +11226,49 @@ static const vshCmdOptDef opts_attach_interface[] = { > {"script", VSH_OT_DATA, 0, N_("script used to bridge network interface")}, > {"model", VSH_OT_DATA, 0, N_("model type")}, > {"persistent", VSH_OT_BOOL, 0, N_("persist interface attachment")}, >+ {"inbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's incoming traffics")}, >+ {"outbound", VSH_OT_DATA, VSH_OFLAG_NONE, N_("control domain's outgoing traffics")}, > {NULL, 0, 0, NULL} > }; > >+/* parse inbound and outbound which are in the format of >+ * 'average,peak,burst', in which peak and burst are optional, >+ * thus 'average,,burst' and 'average,peak' are also legal. */ >+static int parseRateStr(const char *rateStr, virRatePtr rate) >+{ >+ char *average = NULL, *peak = NULL, *burst = NULL; >+ >+ average = (char *)rateStr; >+ if (!average) >+ return -1; >+ rate->average = atol(average); >+ >+ peak = strchr(average, ','); >+ if (peak) { >+ burst = strchr(peak + 1, ','); >+ if (!(burst&& (burst - peak == 1))) { >+ if (virStrToLong_ull(peak + 1,&burst, 10,&rate->peak)< 0) For input like '100,200', after this burst will point to end of string and cause below 'if(burst)' case to fail.

... virsh # attach-interface fc15 network default --inbound 100,200 error: inbound format is incorrect ... >+ return -1; >+ } >+ >+ if (burst) { >+ if (virStrToLong_ull(burst + 1, NULL, 10,&rate->burst)< 0) >+ return -1; >+ } >+ } >+ >+ >+ return 0; >+} >+ > static bool > cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) > { > virDomainPtr dom = NULL; > const char *mac = NULL, *target = NULL, *script = NULL, >- *type = NULL, *source = NULL, *model = NULL; >+ *type = NULL, *source = NULL, *model = NULL, >+ *inboundStr = NULL, *outboundStr = NULL; >+ virRate inbound, outbound; > int typ; > int ret; > bool functionReturn = false; >@@ -11254,7 +11289,9 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) > vshCommandOptString(cmd, "target",&target)< 0 || > vshCommandOptString(cmd, "mac",&mac)< 0 || > vshCommandOptString(cmd, "script",&script)< 0 || >- vshCommandOptString(cmd, "model",&model)< 0) { >+ vshCommandOptString(cmd, "model",&model)< 0 || >+ vshCommandOptString(cmd, "inbound",&inboundStr)< 0 || >+ vshCommandOptString(cmd, "outbound",&outboundStr)< 0) { > vshError(ctl, "missing argument"); > goto cleanup; > } >@@ -11270,6 +11307,29 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) > goto cleanup; > } > >+ if (inboundStr) { >+ memset(&inbound, 0, sizeof(inbound)); >+ if (parseRateStr(inboundStr,&inbound)< 0) { >+ vshError(ctl, _("inbound format is incorrect")); >+ goto cleanup; >+ } >+ if (inbound.average == 0) { >+ vshError(ctl, _("inbound average is mandatory")); >+ goto cleanup; >+ } >+ } >+ if (outboundStr) { >+ memset(&outbound, 0, sizeof(outbound)); >+ if (parseRateStr(outboundStr,&outbound)< 0) { >+ vshError(ctl, _("outbound format is incorrect")); >+ goto cleanup; >+ } >+ if (outbound.average == 0) { >+ vshError(ctl, _("outbound average is mandatory")); >+ goto cleanup; >+ } >+ } >+ > /* Make XML of interface */ > virBufferAsprintf(&buf, "<interface type='%s'>\n", type); > >@@ -11287,6 +11347,38 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) > if (model != NULL) > virBufferAsprintf(&buf, "<model type='%s'/>\n", model); > >+ if (inboundStr || outboundStr) { >+ virBufferAsprintf(&buf, "<bandwidth>\n"); >+ if (inboundStr&& inbound.average> 0) { >+ if (inbound.peak> 0&& inbound.burst> 0) >+ virBufferAsprintf(&buf, "<inbound average='%lld' peak='%lld' burst='%lld'/>\n", >+ inbound.average, inbound.peak, inbound.burst); >+ else if (inbound.peak> 0) >+ virBufferAsprintf(&buf, "<inbound average='%lld' peak='%lld'/>\n", >+ inbound.average, inbound.peak); >+ else if (inbound.burst> 0) >+ virBufferAsprintf(&buf, "<inbound average='%lld' burst='%lld'/>\n", >+ inbound.average, inbound.burst); >+ else >+ virBufferAsprintf(&buf, "<inbound average='%lld'/>\n", inbound.average); >+ } >+ if (outboundStr&& outbound.average> 0) { >+ if (outbound.peak> 0&& outbound.burst> 0) >+ virBufferAsprintf(&buf, "<outbound average='%lld' peak='%lld' burst='%lld'/>\n", >+ outbound.average, outbound.peak, outbound.burst); >+ else if (outbound.peak> 0) >+ virBufferAsprintf(&buf, "<outbound average='%lld' peak='%lld'/>\n", >+ outbound.average, outbound.peak); >+ else if (outbound.burst> 0) >+ virBufferAsprintf(&buf, "<outbound average='%lld' burst='%lld'/>\n", >+ outbound.average, outbound.burst); >+ else >+ virBufferAsprintf(&buf, "<outbound average='%lld'/>\n", outbound.average); >+ >+ } Maybe can save some typing here, :) e.g.: ... if (inbound.peak > 0) virBufferAsprintf(&buf, "peak='%lld' ", inbound.peak); ...

>+ virBufferAsprintf(&buf, "</bandwidth>\n"); >+ } >+ > virBufferAddLit(&buf, "</interface>\n"); > > if (virBufferError(&buf)) { >diff --git a/tools/virsh.pod b/tools/virsh.pod >index d0b7937..06dc06a 100644 >--- a/tools/virsh.pod >+++ b/tools/virsh.pod >@@ -1235,7 +1235,7 @@ scsi:controller.bus.unit or ide:controller.bus.unit. > > =item B<attach-interface> I<domain-id> I<type> I<source> > [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] >-[I<--persistent>] >+[I<--persistent>] [I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>] > > Attach a new network interface to the domain. > I<type> can be either I<network> to indicate a physical network device or I<bridge> to indicate a bridge to a device. >@@ -1246,6 +1246,9 @@ I<script> allows to specify a path to a script handling a bridge instead of > the default one. > I<model> allows to specify the model type. > I<persistent> indicates the changes will affect the next boot of the domain. >+I<inbound> and I<outbound> control the bandwidth of the interface. I<peak> >+and I<burst> are optional, so "average,peak", "average,,burst" and >+"average" are also legal. > > B<Note>: the optional target value is the name of a device to be created > as the back-end on the node. If not provided a device named "vnetN" or "vifN" Tested-by: Hong Xiang <hxiang(a)linux.vnet.ibm.com&gt;