[libvirt] [PATCH] 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 | 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) + 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); + + } + 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" -- 1.7.3.1

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@linux.vnet.ibm.com> -- Thanks. Hong Xiang

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

On 10/14/2011 09:06 AM, Hu Tao wrote:
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?
It's 'virStrToLong_ull(peak + 1,&burst, 10,&rate->peak)' that will overwrite burst, maybe you can pass a NULL instead of '&burst' to this call, and leave below 'if(burst)' as is.
... 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. Hong Xiang

+/* 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'. Here is v2:
From 3c5885380ffd68769f2c7d82eb21bd7aca49393e Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@cn.fujitsu.com> 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" -- 1.7.3.1 -- Thanks, Hu Tao

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@cn.fujitsu.com> 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

On 14.10.2011 04:49, Hu Tao wrote:
Here is v2:
From 3c5885380ffd68769f2c7d82eb21bd7aca49393e Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@cn.fujitsu.com> 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;
I'd vote for const correctness here; So, either change average to be const char *, or leave it out and use passed rateStr directly instead.
+ 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"); + } +
Since [in|out]bound average, peak and burst are defined as unsigned long long, you can actually check for (outbound.peak) instead of (outbound.peak > 0), but you can leave it as-is. But I'd prefer to change print format from '%lld' to '%llu'. I am surprised my gcc does not warn about it.
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"
ACK with those nits fixed. Michal

+/* 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;
I'd vote for const correctness here; So, either change average to be const char *, or leave it out and use passed rateStr directly instead.
used const char * in v3.
+ 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"); + } +
Since [in|out]bound average, peak and burst are defined as unsigned long long, you can actually check for (outbound.peak) instead of (outbound.peak > 0), but you can leave it as-is. But I'd prefer to change print format from '%lld' to '%llu'. I am surprised my gcc does not warn about it.
changed format from '%lld' to '%llu'.
ACK with those nits fixed.
Thanks for you comments. -- Thanks, Hu Tao

Adds two options, inbound and outbound, to attach-interface to set bandwidth when attaching interfaces --- tools/virsh.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 5 ++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 54684f6..5b72b61 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,53 @@ 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) +{ + const char *average = NULL, *peak = NULL, *burst = NULL; + + average = rateStr; + if (!average) + return -1; + if (virStrToLong_ull(average, (char **)&peak, 10, &rate->average) < 0) + return -1; + + /* peak will be updated to point to the end of rateStr in case + * of 'average' */ + if (peak && *peak != '\0') { + burst = strchr(peak + 1, ','); + if (!(burst && (burst - peak == 1))) { + if (virStrToLong_ull(peak + 1, (char **)&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 +11296,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 +11314,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 +11354,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='%llu'", inbound.average); + if (inbound.peak > 0) + virBufferAsprintf(&buf, " peak='%llu'", inbound.peak); + if (inbound.burst > 0) + virBufferAsprintf(&buf, " burst='%llu'", inbound.burst); + virBufferAsprintf(&buf, "/>\n"); + } + if (outboundStr && outbound.average > 0) { + virBufferAsprintf(&buf, " <outbound average='%llu'", outbound.average); + if (outbound.peak > 0) + virBufferAsprintf(&buf, " peak='%llu'", outbound.peak); + if (outbound.burst > 0) + virBufferAsprintf(&buf, " burst='%llu'", 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" -- 1.7.3.1

On 18.10.2011 09:32, Hu Tao wrote:
Adds two options, inbound and outbound, to attach-interface to set bandwidth when attaching interfaces --- tools/virsh.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 5 ++- 2 files changed, 91 insertions(+), 3 deletions(-)
ACK and pushed with this squashed in: diff --git a/tools/virsh.c b/tools/virsh.c index 3e2c46f..42f62d7 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11239,12 +11239,13 @@ static const vshCmdOptDef opts_attach_interface[] = { * thus 'average,,burst' and 'average,peak' are also legal. */ static int parseRateStr(const char *rateStr, virRatePtr rate) { - const char *average = NULL, *peak = NULL, *burst = NULL; + const char *average = NULL; + char *peak = NULL, *burst = NULL; average = rateStr; if (!average) return -1; - if (virStrToLong_ull(average, (char **)&peak, 10, &rate->average) < 0) + if (virStrToLong_ull(average, &peak, 10, &rate->average) < 0) return -1; /* peak will be updated to point to the end of rateStr in case @@ -11252,7 +11253,7 @@ static int parseRateStr(const char *rateStr, virRatePtr rate) if (peak && *peak != '\0') { burst = strchr(peak + 1, ','); if (!(burst && (burst - peak == 1))) { - if (virStrToLong_ull(peak + 1, (char **)&burst, 10, &rate->peak) < 0) + if (virStrToLong_ull(peak + 1, &burst, 10, &rate->peak) < 0) return -1; }
participants (3)
-
Hong Xiang
-
Hu Tao
-
Michal Privoznik