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(a)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>"
goto cleanup;
}
Also should we check below here for outboundStr having floor? (improperly)
> @@ -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...
John
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