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(a)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