[libvirt] [PATCH 0/7] Couple of tiny fixes for virsh and translation.

Lin Ma (7): virsh: Error out while domain not found for 'qemu-monitor-event' command virsh: Error out while domain not found for 'event' command po: fix typo: remove a redundant Chinese character virsh: Simplify control flow for 'desc' command virsh: Simplify control flow for 'qemu-agent-command' command qemu: stats: Display the net type and net source in bulk stats qemu: stats: Display net count,type and source even if domain is inactive po/zh_CN.mini.po | 2 +- src/libvirt-domain.c | 2 ++ src/qemu/qemu_driver.c | 11 ++++++++--- tools/virsh-domain.c | 33 ++++++++++++++++----------------- tools/virsh.pod | 2 ++ 5 files changed, 29 insertions(+), 21 deletions(-) -- 2.15.1

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2b775fc4cc..3b2a34f936 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9661,7 +9661,9 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) return false; if (vshCommandOptBool(cmd, "domain")) - dom = virshCommandOptDomain(ctl, cmd, NULL); + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + if (vshEventStart(ctl, timeout) < 0) goto cleanup; -- 2.15.1

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3b2a34f936..1f3ea0c939 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13445,7 +13445,9 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (vshCommandOptBool(cmd, "domain")) - dom = virshCommandOptDomain(ctl, cmd, NULL); + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + if (vshEventStart(ctl, timeout) < 0) goto cleanup; -- 2.15.1

Signed-off-by: Lin Ma <lma@suse.com> --- po/zh_CN.mini.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/zh_CN.mini.po b/po/zh_CN.mini.po index c5574cf1df..0e47184c93 100644 --- a/po/zh_CN.mini.po +++ b/po/zh_CN.mini.po @@ -15340,7 +15340,7 @@ msgid "entry was missing 'type'" msgstr "条目缺少 'type' " msgid "enumerate devices on this host" -msgstr "这台主机中中的枚举设备" +msgstr "这台主机中的枚举设备" msgid "error" msgstr "错误" -- 2.15.1

On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> ---
NACK This needs to be fixed in Zanata where we pull the translations from since it would be overwritten by the next sync.

On Fri, May 04, 2018 at 12:07:39PM +0200, Peter Krempa wrote:
On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> ---
NACK
This needs to be fixed in Zanata where we pull the translations from since it would be overwritten by the next sync.
To save time, I will fix this mistake in Zanata, so it gets synced. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 05/04/2018 06:11 PM, Daniel P. Berrangé wrote:
On Fri, May 04, 2018 at 12:07:39PM +0200, Peter Krempa wrote:
On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- NACK
This needs to be fixed in Zanata where we pull the translations from since it would be overwritten by the next sync. To save time, I will fix this mistake in Zanata, so it gets synced.
OK, Thanks for your help and review. Lin

Just like the commit 8941c800, It does the similar thing. Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1f3ea0c939..65170225a7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8330,7 +8330,6 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) char *tmpstr; const vshCmdOpt *opt = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - bool pad = false; bool ret = false; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; @@ -8348,18 +8347,16 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) if ((state = virshDomainState(ctl, dom, NULL)) < 0) goto cleanup; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (pad) - virBufferAddChar(&buf, ' '); - pad = true; - virBufferAdd(&buf, opt->data, -1); - } - if (title) type = VIR_DOMAIN_METADATA_TITLE; else type = VIR_DOMAIN_METADATA_DESCRIPTION; + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) + virBufferAsprintf(&buf, "%s ", opt->data); + + virBufferTrim(&buf, " ", -1); + if (virBufferError(&buf)) { vshError(ctl, "%s", _("Failed to collect new description/title")); goto cleanup; -- 2.15.1

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 65170225a7..598d2fa4a4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9796,19 +9796,17 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; const vshCmdOpt *opt = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - bool pad = false; virJSONValuePtr pretty = NULL; dom = virshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (pad) - virBufferAddChar(&buf, ' '); - pad = true; - virBufferAdd(&buf, opt->data, -1); - } + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) + virBufferAsprintf(&buf, "%s ", opt->data); + + virBufferTrim(&buf, " ", -1); + if (virBufferError(&buf)) { vshError(ctl, "%s", _("Failed to collect command")); goto cleanup; -- 2.15.1

Signed-off-by: Lin Ma <lma@suse.com> --- src/libvirt-domain.c | 2 ++ src/qemu/qemu_driver.c | 4 ++++ tools/virsh.pod | 2 ++ 3 files changed, 8 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 2d86e48979..e317ca00d0 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11310,6 +11310,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "net.count" - number of network interfaces on this domain * as unsigned int. * "net.<num>.name" - name of the interface <num> as string. + * "net.<num>.type" - type of the interface <num> as string. + * "net.<num>.source" - source of the interface <num> as string. * "net.<num>.rx.bytes" - bytes received as unsigned long long. * "net.<num>.rx.pkts" - packets received as unsigned long long. * "net.<num>.rx.errs" - receive errors as unsigned long long. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 83fc191085..7a9a2bcf97 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19823,6 +19823,10 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, QEMU_ADD_NAME_PARAM(record, maxparams, "net", "name", i, net->ifname); + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "type", i, virDomainNetTypeToString(net->type)); + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "source", i, net->data.bridge.brname); if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 929958a953..7ec57c0b4b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -974,6 +974,8 @@ I<--interface> returns: "net.count" - number of network interfaces on this domain "net.<num>.name" - name of the interface <num> + "net.<num>.type" - type of the interface <num> + "net.<num>.source" - source of the interface <num> "net.<num>.rx.bytes" - number of bytes received "net.<num>.rx.pkts" - number of packets received "net.<num>.rx.errs" - number of receive errors -- 2.15.1

On Fri, May 04, 2018 at 17:28:53 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> ---
Could you justify this? This is really configuration and will not change during the lifetime of the interface, thus there's no pressing need to report it in the stats which should report only data which is changing.

On 05/04/2018 06:10 PM, Peter Krempa wrote:
On Fri, May 04, 2018 at 17:28:53 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- Could you justify this? This is really configuration and will not change during the lifetime of the interface, thus there's no pressing need to report it in the stats which should report only data which is changing.
well, Actually there is nothing special usecase, Just want to provide more corresponding information in a single command to users, Please forget it if it doesn't make sense. Thanks, Lin

Signed-off-by: Lin Ma <lma@suse.com> --- src/qemu/qemu_driver.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a9a2bcf97..33ae68129d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19804,9 +19804,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, struct _virDomainInterfaceStats tmp; int ret = -1; - if (!virDomainObjIsActive(dom)) - return 0; - QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets); /* Check the path is one of the domain's network interfaces. */ @@ -19814,6 +19811,14 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainNetDefPtr net = dom->def->nets[i]; virDomainNetType actualType; + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "type", i, virDomainNetTypeToString(net->type)); + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "source", i, net->data.bridge.brname); + + if (!virDomainObjIsActive(dom)) + return 0; + if (!net->ifname) continue; @@ -19823,10 +19828,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, QEMU_ADD_NAME_PARAM(record, maxparams, "net", "name", i, net->ifname); - QEMU_ADD_NAME_PARAM(record, maxparams, - "net", "type", i, virDomainNetTypeToString(net->type)); - QEMU_ADD_NAME_PARAM(record, maxparams, - "net", "source", i, net->data.bridge.brname); if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { -- 2.15.1

On Fri, May 04, 2018 at 17:28:54 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- src/qemu/qemu_driver.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
The network is not enabled so there's no stats to report when the VM is offline so I don't think this is justified.

On 05/04/2018 06:11 PM, Peter Krempa wrote:
On Fri, May 04, 2018 at 17:28:54 +0800, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- src/qemu/qemu_driver.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) The network is not enabled so there's no stats to report when the VM is offline so I don't think this is justified. OK, Got it.

On Fri, May 04, 2018 at 17:28:47 +0800, Lin Ma wrote:
Lin Ma (7): virsh: Error out while domain not found for 'qemu-monitor-event' command virsh: Error out while domain not found for 'event' command po: fix typo: remove a redundant Chinese character virsh: Simplify control flow for 'desc' command virsh: Simplify control flow for 'qemu-agent-command' command qemu: stats: Display the net type and net source in bulk stats qemu: stats: Display net count,type and source even if domain is inactive
Patches 1, 2, 4, and 5 are ACKed and pushed.
participants (3)
-
Daniel P. Berrangé
-
Lin Ma
-
Peter Krempa