diff mbox

[V2,1/3] xenconfig: support vif bandwidth in sexpr parser and formatter

Message ID 1451956095-25353-2-git-send-email-jfehlig@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jim Fehlig Jan. 5, 2016, 1:08 a.m. UTC
The xen sexpr config format has long supported specifying vif rate
limiting, e.g.

  (device
    (vif
      (mac '00:16:3e:1b:b1:47')
      (rate '10240KB/s')
      ...
    )
  )

Add support for mapping rate to and from <bandwidth> in the xenconfig
sexpr parser and formatter. rate is mapped to the required 'average'
attribute of the <outbound> element, e.g.

  <interface type='bridge'>
    ...
    <bandwidth>
      <outbound average='10240'/>
    </bandwidth>
  </interface>

Also add unit tests to check the conversion logic.

This patch benefits both the old xen driver and the libxl driver.
Both drivers gain support for vif bandwidth when converting to/from
domXML and xen-sxpr. In addition, the old xen driver will now be
able to handle vif 'rate' setting when communicating with xend.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

I used a bit of code from libxlu_vif.c to implement xenParseSxprVifRate()
instead of using the libxlutil lib directly, since rate limiting applies
to the old xen driver (no libxl) as well.

 src/libvirt_xenconfig.syms                      |  1 +
 src/xenconfig/xen_sxpr.c                        | 74 +++++++++++++++++++++++++
 src/xenconfig/xen_sxpr.h                        |  2 +
 tests/sexpr2xmldata/sexpr2xml-vif-rate.sexpr    | 11 ++++
 tests/sexpr2xmldata/sexpr2xml-vif-rate.xml      | 51 +++++++++++++++++
 tests/sexpr2xmltest.c                           |  2 +
 tests/xml2sexprdata/xml2sexpr-fv-net-rate.sexpr | 10 ++++
 tests/xml2sexprdata/xml2sexpr-fv-net-rate.xml   | 34 ++++++++++++
 tests/xml2sexprtest.c                           |  1 +
 9 files changed, 186 insertions(+)

Comments

John Ferlan Jan. 12, 2016, 1:41 p.m. UTC | #1
On 01/04/2016 08:08 PM, Jim Fehlig wrote:
> The xen sexpr config format has long supported specifying vif rate
> limiting, e.g.
> 
>   (device
>     (vif
>       (mac '00:16:3e:1b:b1:47')
>       (rate '10240KB/s')
>       ...
>     )
>   )
> 
> Add support for mapping rate to and from <bandwidth> in the xenconfig
> sexpr parser and formatter. rate is mapped to the required 'average'
> attribute of the <outbound> element, e.g.
> 
>   <interface type='bridge'>
>     ...
>     <bandwidth>
>       <outbound average='10240'/>
>     </bandwidth>
>   </interface>
> 
> Also add unit tests to check the conversion logic.
> 
> This patch benefits both the old xen driver and the libxl driver.
> Both drivers gain support for vif bandwidth when converting to/from
> domXML and xen-sxpr. In addition, the old xen driver will now be
> able to handle vif 'rate' setting when communicating with xend.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> I used a bit of code from libxlu_vif.c to implement xenParseSxprVifRate()
> instead of using the libxlutil lib directly, since rate limiting applies
> to the old xen driver (no libxl) as well.
> 
>  src/libvirt_xenconfig.syms                      |  1 +
>  src/xenconfig/xen_sxpr.c                        | 74 +++++++++++++++++++++++++
>  src/xenconfig/xen_sxpr.h                        |  2 +
>  tests/sexpr2xmldata/sexpr2xml-vif-rate.sexpr    | 11 ++++
>  tests/sexpr2xmldata/sexpr2xml-vif-rate.xml      | 51 +++++++++++++++++
>  tests/sexpr2xmltest.c                           |  2 +
>  tests/xml2sexprdata/xml2sexpr-fv-net-rate.sexpr | 10 ++++
>  tests/xml2sexprdata/xml2sexpr-fv-net-rate.xml   | 34 ++++++++++++
>  tests/xml2sexprtest.c                           |  1 +
>  9 files changed, 186 insertions(+)
> 

Coverity found an 'issue'...

> diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
> index 6541685..b69f2ab 100644
> --- a/src/libvirt_xenconfig.syms
> +++ b/src/libvirt_xenconfig.syms
> @@ -15,6 +15,7 @@ xenParseSxpr;
>  xenParseSxprChar;
>  xenParseSxprSound;
>  xenParseSxprString;
> +xenParseSxprVifRate;
>  
>  # xenconfig/xen_xm.h
>  xenFormatXM;
> diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
> index d99bac0..9ae50b0 100644
> --- a/src/xenconfig/xen_sxpr.c
> +++ b/src/xenconfig/xen_sxpr.c
> @@ -26,6 +26,8 @@
>  
>  #include <config.h>
>  
> +#include <regex.h>
> +
>  #include "internal.h"
>  #include "virerror.h"
>  #include "virconf.h"
> @@ -315,6 +317,56 @@ xenParseSxprChar(const char *value,
>  }
>  
>  
> +static const char *vif_bytes_per_sec_re = "^[0-9]+[GMK]?[Bb]/s$";
> +
> +int
> +xenParseSxprVifRate(const char *rate, unsigned long long *kbytes_per_sec)
> +{
> +    char *trate = NULL;
> +    char *p;
> +    regex_t rec;
> +    char *suffix;
> +    unsigned long long tmp;
> +    int ret = -1;
> +
> +    if (VIR_STRDUP(trate, rate) < 0)
> +        return -1;
> +
> +    p = strchr(trate, '@');
> +    if (p != NULL)
> +        *p = 0;
> +
> +    regcomp(&rec, vif_bytes_per_sec_re, REG_EXTENDED|REG_NOSUB);

5) Event check_return: 	Calling "regcomp" without checking return value
(as is done elsewhere 14 out of 15 times).

IOW: What if regcomp fails...

John
> +    if (regexec(&rec, trate, 0, NULL, 0)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invalid rate '%s' specified"), rate);
> +        goto cleanup;
> +    }
> +
> +    if (virStrToLong_ull(rate, &suffix, 10, &tmp)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to parse rate '%s'"), rate);
> +        goto cleanup;
> +    }
> +
> +    if (*suffix == 'G')
> +       tmp *= 1024 * 1024;
> +    else if (*suffix == 'M')
> +       tmp *= 1024;
> +
> +    if (*suffix == 'b' || *(suffix + 1) == 'b')
> +       tmp /= 8;
> +
> +    *kbytes_per_sec = tmp;
> +    ret = 0;
> +
> + cleanup:
> +    regfree(&rec);
> +    VIR_FREE(trate);
> +    return ret;
> +}
> +
> +

<...>
diff mbox

Patch

diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
index 6541685..b69f2ab 100644
--- a/src/libvirt_xenconfig.syms
+++ b/src/libvirt_xenconfig.syms
@@ -15,6 +15,7 @@  xenParseSxpr;
 xenParseSxprChar;
 xenParseSxprSound;
 xenParseSxprString;
+xenParseSxprVifRate;
 
 # xenconfig/xen_xm.h
 xenFormatXM;
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index d99bac0..9ae50b0 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -26,6 +26,8 @@ 
 
 #include <config.h>
 
+#include <regex.h>
+
 #include "internal.h"
 #include "virerror.h"
 #include "virconf.h"
@@ -315,6 +317,56 @@  xenParseSxprChar(const char *value,
 }
 
 
+static const char *vif_bytes_per_sec_re = "^[0-9]+[GMK]?[Bb]/s$";
+
+int
+xenParseSxprVifRate(const char *rate, unsigned long long *kbytes_per_sec)
+{
+    char *trate = NULL;
+    char *p;
+    regex_t rec;
+    char *suffix;
+    unsigned long long tmp;
+    int ret = -1;
+
+    if (VIR_STRDUP(trate, rate) < 0)
+        return -1;
+
+    p = strchr(trate, '@');
+    if (p != NULL)
+        *p = 0;
+
+    regcomp(&rec, vif_bytes_per_sec_re, REG_EXTENDED|REG_NOSUB);
+    if (regexec(&rec, trate, 0, NULL, 0)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Invalid rate '%s' specified"), rate);
+        goto cleanup;
+    }
+
+    if (virStrToLong_ull(rate, &suffix, 10, &tmp)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to parse rate '%s'"), rate);
+        goto cleanup;
+    }
+
+    if (*suffix == 'G')
+       tmp *= 1024 * 1024;
+    else if (*suffix == 'M')
+       tmp *= 1024;
+
+    if (*suffix == 'b' || *(suffix + 1) == 'b')
+       tmp /= 8;
+
+    *kbytes_per_sec = tmp;
+    ret = 0;
+
+ cleanup:
+    regfree(&rec);
+    VIR_FREE(trate);
+    return ret;
+}
+
+
 /**
  * xenParseSxprDisks:
  * @def: the domain config
@@ -594,6 +646,25 @@  xenParseSxprNets(virDomainDefPtr def,
                 VIR_STRDUP(net->model, "netfront") < 0)
                 goto cleanup;
 
+            tmp = sexpr_node(node, "device/vif/rate");
+            if (tmp) {
+                virNetDevBandwidthPtr bandwidth;
+                unsigned long long kbytes_per_sec;
+
+                if (xenParseSxprVifRate(tmp, &kbytes_per_sec) < 0)
+                    goto cleanup;
+
+                if (VIR_ALLOC(bandwidth) < 0)
+                    goto cleanup;
+                if (VIR_ALLOC(bandwidth->out) < 0) {
+                    VIR_FREE(bandwidth);
+                    goto cleanup;
+                }
+
+                bandwidth->out->average = kbytes_per_sec;
+                net->bandwidth = bandwidth;
+            }
+
             if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0)
                 goto cleanup;
 
@@ -1784,6 +1855,9 @@  xenFormatSxprNet(virConnectPtr conn,
 
     virBufferAsprintf(buf, "(mac '%s')", virMacAddrFormat(&def->mac, macaddr));
 
+    if (def->bandwidth && def->bandwidth->out && def->bandwidth->out->average)
+        virBufferAsprintf(buf, "(rate '%lluKB/s')", def->bandwidth->out->average);
+
     switch (def->type) {
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
         virBufferEscapeSexpr(buf, "(bridge '%s')", def->data.bridge.brname);
diff --git a/src/xenconfig/xen_sxpr.h b/src/xenconfig/xen_sxpr.h
index a4f4c44..76ff943 100644
--- a/src/xenconfig/xen_sxpr.h
+++ b/src/xenconfig/xen_sxpr.h
@@ -49,6 +49,8 @@  int xenParseSxprSound(virDomainDefPtr def, const char *str);
 
 virDomainChrDefPtr xenParseSxprChar(const char *value, const char *tty);
 
+int xenParseSxprVifRate(const char *rate, unsigned long long *kbytes_per_sec);
+
 int xenFormatSxprDisk(virDomainDiskDefPtr def, virBufferPtr buf, int hvm,
                       int isAttach);
 
diff --git a/tests/sexpr2xmldata/sexpr2xml-vif-rate.sexpr b/tests/sexpr2xmldata/sexpr2xml-vif-rate.sexpr
new file mode 100644
index 0000000..c3ed8e8
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-vif-rate.sexpr
@@ -0,0 +1,11 @@ 
+(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)\
+(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')\
+(on_reboot 'restart')(on_crash 'restart')\
+(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')\
+(device_model '/usr/lib64/xen/bin/qemu-dm')(boot c)\
+(acpi 1)(vnc 1)(keymap ja)))(device (vbd (dev 'ioemu:hda')\
+(uname 'file:/root/foo.img')(mode 'w')))\
+(device (vbd (dev 'hdc:cdrom')\
+(uname 'file:/root/boot.iso')(mode 'r')))\
+(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')\
+(script 'vif-bridge')(type 'netfront')(rate '10240KB/s'))))
diff --git a/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml b/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml
new file mode 100644
index 0000000..0531817
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-vif-rate.xml
@@ -0,0 +1,51 @@ 
+<domain type='xen' id='3'>
+  <name>fvtest</name>
+  <uuid>b5d70dd2-75cd-aca5-1776-9660b059d8bc</uuid>
+  <memory unit='KiB'>409600</memory>
+  <currentMemory unit='KiB'>409600</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type>hvm</type>
+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <clock offset='variable' adjustment='0' basis='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/lib64/xen/bin/qemu-dm</emulator>
+    <disk type='file' device='disk'>
+      <driver name='file'/>
+      <source file='/root/foo.img'/>
+      <backingStore/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='file'/>
+      <source file='/root/boot.iso'/>
+      <backingStore/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:1b:b1:47'/>
+      <source bridge='xenbr0'/>
+      <bandwidth>
+        <outbound average='10240'/>
+      </bandwidth>
+      <script path='vif-bridge'/>
+      <target dev='vif3.0'/>
+      <model type='netfront'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='vnc' port='-1' autoport='yes' keymap='ja'/>
+    <memballoon model='xen'/>
+  </devices>
+</domain>
diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
index 26fadaa..de537ed 100644
--- a/tests/sexpr2xmltest.c
+++ b/tests/sexpr2xmltest.c
@@ -188,6 +188,8 @@  mymain(void)
 
     DO_TEST("boot-grub", "boot-grub");
 
+    DO_TEST("vif-rate", "vif-rate");
+
     virObjectUnref(caps);
     virObjectUnref(xmlopt);
 
diff --git a/tests/xml2sexprdata/xml2sexpr-fv-net-rate.sexpr b/tests/xml2sexprdata/xml2sexpr-fv-net-rate.sexpr
new file mode 100644
index 0000000..03afedd
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-fv-net-rate.sexpr
@@ -0,0 +1,10 @@ 
+(vm (name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)\
+(uuid 'b5d70dd2-75cd-aca5-1776-9660b059d8bc')(on_poweroff 'destroy')\
+(on_reboot 'restart')(on_crash 'restart')\
+(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(vcpus 1)(boot c)(acpi 1)\
+(usb 1)(parallel none)(serial none)(device_model '/usr/lib64/xen/bin/qemu-dm')\
+(vnc 1)(vncunused 0)(vncdisplay 17)(keymap 'ja')(rtc_timeoffset 0)(localtime 0)))\
+(localtime 0)\
+(device (vbd (dev 'hda:disk')(uname 'file:/root/foo.img')\
+(mode 'w')))(device (vif (mac '00:16:3e:1b:b1:47')(rate '10240KB/s')\
+(bridge 'xenbr0')(script 'vif-bridge')(type netfront))))
diff --git a/tests/xml2sexprdata/xml2sexpr-fv-net-rate.xml b/tests/xml2sexprdata/xml2sexpr-fv-net-rate.xml
new file mode 100644
index 0000000..caea525
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-fv-net-rate.xml
@@ -0,0 +1,34 @@ 
+<domain type='xen'>
+  <name>fvtest</name>
+  <uuid>b5d70dd275cdaca517769660b059d8bc</uuid>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='hd'/>
+  </os>
+  <memory unit='KiB'>409600</memory>
+  <vcpu>1</vcpu>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <features>
+    <acpi/>
+  </features>
+  <devices>
+    <emulator>/usr/lib64/xen/bin/qemu-dm</emulator>
+    <interface type='bridge'>
+      <source bridge='xenbr0'/>
+      <bandwidth>
+        <outbound average='10240'/>
+      </bandwidth>
+      <mac address='00:16:3e:1b:b1:47'/>
+      <script path='vif-bridge'/>
+      <model type='netfront'/>
+    </interface>
+    <disk type='file'>
+      <source file='/root/foo.img'/>
+      <target dev='ioemu:hda'/>
+    </disk>
+    <graphics type='vnc' port='5917' keymap='ja'/>
+  </devices>
+</domain>
diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c
index b1b0cde..66503b7 100644
--- a/tests/xml2sexprtest.c
+++ b/tests/xml2sexprtest.c
@@ -156,6 +156,7 @@  mymain(void)
     DO_TEST("fv-sound", "fv-sound", "fvtest");
 
     DO_TEST("fv-net-netfront", "fv-net-netfront", "fvtest");
+    DO_TEST("fv-net-rate", "fv-net-rate", "fvtest");
 
     DO_TEST("boot-grub", "boot-grub", "fvtest");
     DO_TEST("escape", "escape", "fvtest");