[libvirt] [PATCH] domain: Improve error output for virDomainListGetStats
When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats. error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
Improve error to: error: --domain and --list-* flags are mutually exclusive Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/libvirt-domain.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6ae6dd2 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11053,6 +11053,19 @@ virDomainListGetStats(virDomainPtr *doms, goto cleanup; } + if (flags & (VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE | + VIR_CONNECT_LIST_DOMAINS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_TRANSIENT | + VIR_CONNECT_LIST_DOMAINS_RUNNING | + VIR_CONNECT_LIST_DOMAINS_SHUTOFF | + VIR_CONNECT_LIST_DOMAINS_PAUSED | + VIR_CONNECT_LIST_DOMAINS_OTHER)) { + virReportInvalidArg(flags, "%s", + _("--domain and --list-* flags are mutually exclusive")); + goto cleanup; + } + conn = doms[0]->conn; virCheckConnectReturn(conn, -1); -- 1.8.3.1
On 11/04/2014 07:51 AM, Luyao Huang wrote:
When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
NACK. While they cannot be combined in the current implementation, I see nothing that would prohibit a future implementation from allowing the combination (for example, show me all stats for a given domain, but only if the domain is running). Prohibiting in libvirt-domian.c would prevent an older RPC client from talking to a newer server that supports the combination. We should only prohibit combinations in libvirt-*.c if there is no chance we will ever extend things to allow the combination. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Thanks for pointing out the mistake.I will move the check in qemuConnectGetAllDomainStats, this won't make a old client cannot use the future server and will give a good error when use future client to connect to old server. Thanks, Luyao Huang ----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: "Luyao Huang" <lhuang@redhat.com>, libvir-list@redhat.com Sent: Tuesday, November 4, 2014 7:19:41 PM Subject: Re: [libvirt] [PATCH] domain: Improve error output for virDomainListGetStats On 11/04/2014 07:51 AM, Luyao Huang wrote:
When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
NACK. While they cannot be combined in the current implementation, I see nothing that would prohibit a future implementation from allowing the combination (for example, show me all stats for a given domain, but only if the domain is running). Prohibiting in libvirt-domian.c would prevent an older RPC client from talking to a newer server that supports the combination. We should only prohibit combinations in libvirt-*.c if there is no chance we will ever extend things to allow the combination. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/04/14 07:51, Luyao Huang wrote:
When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
Improve error to:
error: --domain and --list-* flags are mutually exclusive
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/libvirt-domain.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6ae6dd2 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11053,6 +11053,19 @@ virDomainListGetStats(virDomainPtr *doms, goto cleanup; }
+ if (flags & (VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE | + VIR_CONNECT_LIST_DOMAINS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_TRANSIENT | + VIR_CONNECT_LIST_DOMAINS_RUNNING | + VIR_CONNECT_LIST_DOMAINS_SHUTOFF | + VIR_CONNECT_LIST_DOMAINS_PAUSED | + VIR_CONNECT_LIST_DOMAINS_OTHER)) { + virReportInvalidArg(flags, "%s", + _("--domain and --list-* flags are mutually exclusive")); + goto cleanup; + }
From what I remember it was a deliberate design decision to avoid reporting this error either from virsh or the library itself so that we possibly can add the filtering later on. This should be done in the qemu driver impl of the function so that we can possibly do it later without breaking old clients
+ conn = doms[0]->conn; virCheckConnectReturn(conn, -1);
Peter
Thanks your advise :) and I have moved the check in qemuConnectGetAllDomainStats. And the v2: https://www.redhat.com/archives/libvir-list/2014-November/msg00069.html Thanks, Luyao Huang ----- Original Message ----- From: "Peter Krempa" <pkrempa@redhat.com> To: "Luyao Huang" <lhuang@redhat.com>, libvir-list@redhat.com Sent: Tuesday, November 4, 2014 7:20:45 PM Subject: Re: [libvirt] [PATCH] domain: Improve error output for virDomainListGetStats On 11/04/14 07:51, Luyao Huang wrote:
When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
Improve error to:
error: --domain and --list-* flags are mutually exclusive
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/libvirt-domain.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7dc3146..6ae6dd2 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11053,6 +11053,19 @@ virDomainListGetStats(virDomainPtr *doms, goto cleanup; }
+ if (flags & (VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE | + VIR_CONNECT_LIST_DOMAINS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_TRANSIENT | + VIR_CONNECT_LIST_DOMAINS_RUNNING | + VIR_CONNECT_LIST_DOMAINS_SHUTOFF | + VIR_CONNECT_LIST_DOMAINS_PAUSED | + VIR_CONNECT_LIST_DOMAINS_OTHER)) { + virReportInvalidArg(flags, "%s", + _("--domain and --list-* flags are mutually exclusive")); + goto cleanup; + }
From what I remember it was a deliberate design decision to avoid reporting this error either from virsh or the library itself so that we possibly can add the filtering later on.
This should be done in the qemu driver impl of the function so that we can possibly do it later without breaking old clients
+ conn = doms[0]->conn; virCheckConnectReturn(conn, -1);
Peter
participants (3)
-
Eric Blake -
Luyao Huang -
Peter Krempa