[libvirt] [PATCH 8/9] virsh: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR
Michal Privoznik
mprivozn at redhat.com
Tue Aug 11 10:08:33 UTC 2015
On 11.08.2015 03:25, John Ferlan wrote:
>
>
> On 08/03/2015 02:39 AM, Michal Privoznik wrote:
>> We have a function parseRateStr() that parses --inbound and
>> --outbound arguments to both attach-interface and domiftune.
>> Now that we have all virTypedParams macros needed for QoS,
>> lets parse even floor attribute. The extended format for the
>> arguments looks like this then:
>>
>> --inbound average[,peak[,burst[,floor]]]
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>> tools/virsh-domain.c | 23 ++++++++++++++++++-----
>> tools/virsh.pod | 12 ++++++------
>> 2 files changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> index bb40ddd..5b7e623 100644
>> --- a/tools/virsh-domain.c
>> +++ b/tools/virsh-domain.c
>
> Some comments above here need adjustment to describe the new field and
> possible options...
>
> Does it matter if someone provides "floor" on outbound (it's a testing
> question ;-))
>
>> @@ -873,7 +873,7 @@ static int parseRateStr(vshControl *ctl,
>> char *next;
>> char *saveptr;
>> enum {
>> - AVERAGE, PEAK, BURST
>> + AVERAGE, PEAK, BURST, FLOOR
>> } state;
>> int ret = -1;
>>
>> @@ -882,7 +882,7 @@ static int parseRateStr(vshControl *ctl,
>>
>> next = vshStrdup(ctl, rateStr);
>>
>> - for (state = AVERAGE; state <= BURST; state++) {
>> + for (state = AVERAGE; state <= FLOOR; state++) {
>> unsigned long long *tmp;
>> const char *field_name;
>>
>> @@ -905,6 +905,11 @@ static int parseRateStr(vshControl *ctl,
>> tmp = &rate->burst;
>> field_name = "burst";
>> break;
>> +
>> + case FLOOR:
>> + tmp = &rate->floor;
>> + field_name = "floor";
>> + break;
>> }
>>
>> if (virStrToLong_ullp(token, NULL, 10, tmp) < 0) {
>> @@ -976,7 +981,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>> memset(&inbound, 0, sizeof(inbound));
>> if (parseRateStr(ctl, inboundStr, &inbound) < 0)
>> goto cleanup;
>> - if (inbound.average == 0) {
>> + if (!inbound.average && !inbound.floor) {
>> vshError(ctl, _("inbound average is mandatory"));
>
> Why checking !inbound.floor?
>
> What if someone does ",,,<floor value>"
Yeah, I should have updated the error message too. At least one of average and floor is required. Your example is perfectly valid and in fact I've just just that during my testing when writing the patches.
>
>
>> goto cleanup;
>> }
>
> Also should we check below here for outboundStr having floor? (improperly)
We can, but so far the outbound.floor is just ignored.
>
>
>> @@ -3308,8 +3313,10 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
>> UINT_MAX);
>> goto cleanup;
>> }
>> - if (inbound.average == 0 && (inbound.burst || inbound.peak)) {
>> - vshError(ctl, _("inbound average is mandatory"));
>> +
>> + if ((!inbound.average && (inbound.burst || inbound.peak)) &&
>> + !inbound.floor) {
>> + vshError(ctl, _("either inbound average or floor is mandatory"));
>> goto cleanup;
>> }
>>
>> @@ -3329,6 +3336,12 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
>> VIR_DOMAIN_BANDWIDTH_IN_BURST,
>> inbound.burst) < 0)
>> goto save_error;
>> +
>> + if (inbound.floor &&
>> + virTypedParamsAddUInt(¶ms, &nparams, &maxparams,
>> + VIR_DOMAIN_BANDWIDTH_IN_FLOOR,
>> + inbound.floor) < 0)
>> + goto save_error;
>> }
>>
>> if (outboundStr) {
>
> ...
> should we check here if someone provides outbound.floor value and fail?
>
>
>> diff --git a/tools/virsh.pod b/tools/virsh.pod
>> index 5ee9a96..a6148d3 100644
>> --- a/tools/virsh.pod
>> +++ b/tools/virsh.pod
>> @@ -770,7 +770,7 @@ I<interface-device> can be the interface's target name or the MAC address.
>>
>> =item B<domiftune> I<domain> I<interface-device>
>> [[I<--config>] [I<--live>] | [I<--current>]]
>> -[I<--inbound average,peak,burst>]
>> +[I<--inbound average,peak,burst,floor>]
>> [I<--outbound average,peak,burst>]
>>
>> Set or query the domain's network interface's bandwidth parameters.
>> @@ -779,9 +779,9 @@ or the MAC address.
>>
>> If no I<--inbound> or I<--outbound> is specified, this command will
>> query and show the bandwidth settings. Otherwise, it will set the
>> -inbound or outbound bandwidth. I<average,peak,burst> is the same as
>> -in command I<attach-interface>. Values for I<average> and I<peak> are
>> -expressed in kilobytes per second, while I<burst> is expressed in kilobytes
>> +inbound or outbound bandwidth. I<average,peak,burst,floor> is the same as
>> +in command I<attach-interface>. Values for I<average>, I<peak> and I<floor>
>> +are expressed in kilobytes per second, while I<burst> is expressed in kilobytes
>> in a single burst at -I<peak> speed as described in the Network XML
>> documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>.
>>
>> @@ -2499,7 +2499,7 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>.
>> =item B<attach-interface> I<domain> I<type> I<source>
>> [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
>> [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
>> -[I<--inbound average,peak,burst>] [I<--outbound average,peak,burst>]
>> +[I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
>>
>> Attach a new network interface to the domain. I<type> can be
>> I<network> to indicate connection via a libvirt virtual network, or
>> @@ -2520,7 +2520,7 @@ instead of the default script not in addition to it; --script is valid
>> only for interfaces of type I<bridge> and only for Xen domains.
>> I<model> specifies the network device model to be presented to the
>> domain. I<inbound> and I<outbound> control the bandwidth of the
>> -interface. I<peak> and I<burst> are optional, so "average,peak",
>> +interface. I<peak>, I<burst> and I<floor> are optional, so "average,peak",
>> "average,,burst" and "average" are also legal. Values for I<average>
> ^
> Insert?
>
> "average,,,floor",
>
> I'm OK with just seeing a diff for a final ACK...
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a1913a9..a957836 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -982,7 +982,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
if (parseRateStr(ctl, inboundStr, &inbound) < 0)
goto cleanup;
if (!inbound.average && !inbound.floor) {
- vshError(ctl, _("inbound average is mandatory"));
+ vshError(ctl, _("either inbound average or floor is mandatory"));
goto cleanup;
}
}
@@ -994,6 +994,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("outbound average is mandatory"));
goto cleanup;
}
+ if (outbound.floor) {
+ vshError(ctl, _("outbound floor is unsupported yet"));
+ goto cleanup;
+ }
}
/* Make XML of interface */
@@ -3358,6 +3362,11 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+ if (outbound.floor) {
+ vshError(ctl, _("outbound floor is unsupported yet"));
+ goto cleanup;
+ }
+
if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE,
outbound.average) < 0)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a6148d3..07e6ba7 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -782,7 +782,7 @@ query and show the bandwidth settings. Otherwise, it will set the
inbound or outbound bandwidth. I<average,peak,burst,floor> is the same as
in command I<attach-interface>. Values for I<average>, I<peak> and I<floor>
are expressed in kilobytes per second, while I<burst> is expressed in kilobytes
-in a single burst at -I<peak> speed as described in the Network XML
+in a single burst at I<peak> speed as described in the Network XML
documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>.
To clear inbound or outbound settings, use I<--inbound> or I<--outbound>
@@ -2520,11 +2520,13 @@ instead of the default script not in addition to it; --script is valid
only for interfaces of type I<bridge> and only for Xen domains.
I<model> specifies the network device model to be presented to the
domain. I<inbound> and I<outbound> control the bandwidth of the
-interface. I<peak>, I<burst> and I<floor> are optional, so "average,peak",
-"average,,burst" and "average" are also legal. Values for I<average>
-and I<peak> are expressed in kilobytes per second, while I<burst> is
-expressed in kilobytes in a single burst at -I<peak> speed as
-described in the Network XML documentation at
+interface. At least one from the I<average>, I<floor> pair must be
+specified. The other two I<peak> and I<burst> are optional, so
+"average,peak", "average,,burst", "average,,,floor", "average" and
+",,,floor" are also legal. Values for I<average>, I<floor> and I<peak>
+are expressed in kilobytes per second, while I<burst> is expressed in
+kilobytes in a single burst at I<peak> speed as described in the
+Network XML documentation at
L<http://libvirt.org/formatnetwork.html#elementQoS>.
If I<--live> is specified, affect a running domain.
Michal
More information about the libvir-list
mailing list