Re: [PATCH v2] support setting bandwidth from virsh attach-interface(was Re: [PATCH] support setting bandwidth from virsh attach-interface)

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

 



On 10/14/2011 10:49 AM, Hu Tao wrote:
+/* 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?

It's 'virStrToLong_ull(peak + 1,&burst, 10,&rate->peak)' that will

You're right.

overwrite burst, maybe you can pass a NULL instead of '&burst' to
this call, and leave below 'if(burst)' as is.

Passing a NULL fails virStrToLong_ull in case of '100,200,300'.
Oh, I thought virStrToLong_ull() accepts a NULL endptr and I was wrong.

Here is v2:

 From 3c5885380ffd68769f2c7d82eb21bd7aca49393e Mon Sep 17 00:00:00 2001
From: Hu Tao<hutao@xxxxxxxxxxxxxx>
Date: Fri, 14 Oct 2011 09:10:39 +0800
Subject: [PATCH v2] support setting bandwidth from virsh attach-interface

Adds two options, inbound and outbound, to attach-interface to set
bandwidth when attaching interfaces
---
  tools/virsh.c   |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
  tools/virsh.pod |    5 ++-
  2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 54684f6..ce40a57 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;

@@ -11228,15 +11229,51 @@ 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)
+                return -1;
+        }
+
+        /* burst will be updated to point to the end of rateStr in case
+         * of 'average,peak' */
+        if (burst&&  *burst != '\0') {
+            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;
@@ -11257,7 +11294,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;
      }
@@ -11273,6 +11312,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);

@@ -11290,6 +11352,27 @@ 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) {
+            virBufferAsprintf(&buf, "<inbound average='%lld'", inbound.average);
+            if (inbound.peak>  0)
+                virBufferAsprintf(&buf, " peak='%lld'", inbound.peak);
+            if (inbound.burst>  0)
+                virBufferAsprintf(&buf, " burst='%lld'", inbound.burst);
+            virBufferAsprintf(&buf, "/>\n");
+        }
+        if (outboundStr&&  outbound.average>  0) {
+            virBufferAsprintf(&buf, "<outbound average='%lld'", outbound.average);
+            if (outbound.peak>  0)
+                virBufferAsprintf(&buf, " peak='%lld'", outbound.peak);
+            if (outbound.burst>  0)
+                virBufferAsprintf(&buf, " burst='%lld'", outbound.burst);
+            virBufferAsprintf(&buf, "/>\n");
+        }
+        virBufferAsprintf(&buf, "</bandwidth>\n");
+    }
+
      virBufferAddLit(&buf, "</interface>\n");

      if (virBufferError(&buf)) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 74ae647..43a4f4c 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1237,7 +1237,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.
@@ -1248,6 +1248,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"

I'm ok with this new patch, now sure if I can ACK or not though, :)

--
Thanks.
Hong Xiang

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]