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

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



On Thu, Oct 13, 2011 at 05:17:34PM +0800, Hong Xiang wrote:
> 
> 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.

"if (burst && *burst != '\0')" should fix this. But what the man page says
is

  The strchr() and strrchr() functions return a pointer to the matched
  character or NULL if the character is not found

Am I missing something?

> ...
> 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);
> ...

Thanks. I'll update it.

> 
> >+        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 linux vnet ibm com>

Thank you.

-- 
Thanks,
Hu Tao


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