[libvirt] [PATCH v4] virsh: use virConnectGetDomainCapabilities with maxvcpus

virsh maxvcpus --type kvm output is useless on PPC. Also, in commit e6806d79 we documented not rely on virConnectGetMaxVcpus output. Fix the maxvcpus to use virConnectGetDomainCapabilities now to make it useful. The call is made to use the default emulator binary and to check for the host machine and arch which is what the command intends to show anyway. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- tools/virsh-host.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 57f0c0e..dbdf23d 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -606,18 +606,46 @@ static bool cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) { const char *type = NULL; - int vcpus; + int vcpus = -1; + char *caps = NULL; + const unsigned int flags = 0; /* No flags so far */ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; virshControlPtr priv = ctl->privData; if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) return false; - if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) + caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, flags); + if (!caps) { + if (last_error && last_error->code != VIR_ERR_NO_SUPPORT) + return false; + + vshResetLibvirtError(); + goto fallback; + } + + xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), &ctxt); + if (!xml) { + VIR_FREE(caps); return false; + } - vshPrint(ctl, "%d\n", vcpus); + virXPathInt("string(./vcpu[1]/@max)", ctxt, &vcpus); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + VIR_FREE(caps); + if (vcpus < 0) + goto fallback; + + exit: + vshPrint(ctl, "%d\n", vcpus); return true; + fallback: + if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) + return false; + goto exit; } /*

On Wed, Sep 07, 2016 at 14:25:42 +0530, Shivaprasad G Bhat wrote:
virsh maxvcpus --type kvm output is useless on PPC. Also, in commit e6806d79 we documented not rely on virConnectGetMaxVcpus output. Fix the maxvcpus to use virConnectGetDomainCapabilities now to make it useful. The call is made to use the default emulator binary and to check for the host machine and arch which is what the command intends to show anyway.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- tools/virsh-host.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 57f0c0e..dbdf23d 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -606,18 +606,46 @@ static bool cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) {
[...]
+ if (vcpus < 0) + goto fallback; + + exit: + vshPrint(ctl, "%d\n", vcpus); return true; + fallback: + if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) + return false; + goto exit;
I don't quite like this control flow. I'll respond to this with my suggested solution. Peter

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> virsh maxvcpus --type kvm output is useless on PPC. Also, in commit e6806d79 we documented not rely on virConnectGetMaxVcpus output. Fix the maxvcpus to use virConnectGetDomainCapabilities now to make it useful. The call is made to use the default emulator binary and to check for the host machine and arch which is what the command intends to show anyway. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- tools/virsh-host.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 57f0c0e..2337ce8 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -606,18 +606,40 @@ static bool cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) { const char *type = NULL; - int vcpus; + int vcpus = -1; + char *caps = NULL; + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; virshControlPtr priv = ctl->privData; + bool ret = false; if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) return false; - if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) - return false; + if ((caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, + type, 0))) { + if (!(xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), &ctxt))) + goto cleanup; + + ignore_value(virXPathInt("string(./vcpu[1]/@max)", ctxt, &vcpus)); + } else { + if (last_error && last_error->code != VIR_ERR_NO_SUPPORT) + goto cleanup; + + vshResetLibvirtError(); + } + + if (vcpus < 0 && (vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) + goto cleanup; vshPrint(ctl, "%d\n", vcpus); + ret = true; - return true; + cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + VIR_FREE(caps); + return ret; } /* -- 2.10.0

Hi Peter, This saves few lines of code for sure :) I see the ignore_error for virXPathInt(). I see no issues. ACK Thanks a lot! -Shivaprasad On 09/15/2016 01:47 PM, Peter Krempa wrote:
From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
virsh maxvcpus --type kvm output is useless on PPC. Also, in commit e6806d79 we documented not rely on virConnectGetMaxVcpus output. Fix the maxvcpus to use virConnectGetDomainCapabilities now to make it useful. The call is made to use the default emulator binary and to check for the host machine and arch which is what the command intends to show anyway.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- tools/virsh-host.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 57f0c0e..2337ce8 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -606,18 +606,40 @@ static bool cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) { const char *type = NULL; - int vcpus; + int vcpus = -1; + char *caps = NULL; + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; virshControlPtr priv = ctl->privData; + bool ret = false;
if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) return false;
- if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) - return false; + if ((caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, + type, 0))) { + if (!(xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), &ctxt))) + goto cleanup; + + ignore_value(virXPathInt("string(./vcpu[1]/@max)", ctxt, &vcpus)); + } else { + if (last_error && last_error->code != VIR_ERR_NO_SUPPORT) + goto cleanup; + + vshResetLibvirtError(); + } + + if (vcpus < 0 && (vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0) + goto cleanup;
vshPrint(ctl, "%d\n", vcpus); + ret = true;
- return true; + cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + VIR_FREE(caps); + return ret; }
/*
participants (2)
-
Peter Krempa
-
Shivaprasad G Bhat