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

Hong Xiang hxiang at linux.vnet.ibm.com
Fri Oct 14 01:28:11 UTC 2011


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 at linux.vnet.ibm.com>
>
> Thank you.
>

-- 
Thanks.
Hong Xiang




More information about the libvir-list mailing list