On Wed, Aug 12, 2015 at 09:59:08 +0200, Michal Privoznik wrote:
On 12.08.2015 07:45, Peter Krempa wrote:
cmdAttachInterface doesn't support the 'floor' field that was added in d7f5c88961b52 but that commit didn't properly reject it from cmdAttachInterface where it's unused. --- tools/virsh-domain.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9f54691..94e7a5c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -963,8 +963,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) memset(&inbound, 0, sizeof(inbound)); if (vshParseRateStr(ctl, inboundStr, &inbound) < 0) goto cleanup; - if (!inbound.average && !inbound.floor) { - vshError(ctl, _("either inbound average or floor is mandatory")); + if (!inbound.average) { + vshError(ctl, _("inbound average is mandatory")); + goto cleanup; + } + if (inbound.floor) { + vshError(ctl, _("inbound floor is unsupported yet")); goto cleanup; } }
Ah, it isn't. But it's not limitation of our API (which does support @floor of course). It's a virsh limitation. So how about instead of you patch have the following in?
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a957836..7cd521e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1041,12 +1041,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (inboundStr || outboundStr) { virBufferAddLit(&buf, "<bandwidth>\n"); virBufferAdjustIndent(&buf, 2); - if (inboundStr && inbound.average > 0) { - virBufferAsprintf(&buf, "<inbound average='%llu'", inbound.average); + if (inboundStr && (inbound.average || inbound.floor)) { + virBufferAddLit(&buf, "<inbound"); + if (inbound.average > 0) + virBufferAsprintf(&buf, " average='%llu'", inbound.average); if (inbound.peak > 0) virBufferAsprintf(&buf, " peak='%llu'", inbound.peak); if (inbound.burst > 0) virBufferAsprintf(&buf, " burst='%llu'", inbound.burst); + if (inbound.floor > 0) + virBufferAsprintf(&buf, " floor='%llu'", inbound.floor); virBufferAddLit(&buf, "/>\n"); } if (outboundStr && outbound.average > 0) {
Feel free to do it that way. I've noticed that the check didn't make much sense as it was now and I didn't care enough to see whether it was possible to use that. Peter