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

[Shivaprasad G Bhat]

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 

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 
---
  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", ) < 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)"), 
)))
+goto cleanup;
+
+ignore_value(virXPathInt("string(./vcpu[1]/@max)", ctxt, ));
+} 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;
  }

  /*


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Peter Krempa]

From: Shivaprasad G Bhat 

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 
---
 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", ) < 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)"), 
)))
+goto cleanup;
+
+ignore_value(virXPathInt("string(./vcpu[1]/@max)", ctxt, ));
+} 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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Shivaprasad G Bhat]

On 09/06/2016 05:56 PM, Peter Krempa wrote:

On Tue, Sep 06, 2016 at 16:59:09 +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 
---
  tools/virsh-host.c |   28 +---
  1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..505cfbb 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
  {
  const char *type = NULL;
  int vcpus;
+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", ) < 0)

  return false;
  
-if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)

-return false;
+caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, 
flags);
+if (caps) {
+xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
+if (!xml) {
+VIR_FREE(caps);
+return false;
+}
  
-vshPrint(ctl, "%d\n", vcpus);

+virXPathInt("string(./vcpu[1]/@max)", ctxt, );

This doesn't handle the case when the capability XML does not contain
the required data. This still should fall back to the legacy approach.
Ah bhyve doesn't report the vcpus in domcapabilities. Thanks. Fixed it 
in next version I posted just now.


Regards,
Shivaprasad

Additionally virXPathInt does not initialize vcpus on failure and since
it's not initialized when declared this would trigger a compiler
warning.


+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+VIR_FREE(caps);


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Peter Krempa]

On Tue, Sep 06, 2016 at 16:59:09 +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 
> ---
>  tools/virsh-host.c |   28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 57f0c0e..505cfbb 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
>  {
>  const char *type = NULL;
>  int vcpus;
> +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", ) < 0)
>  return false;
>  
> -if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
> -return false;
> +caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, 
> type, flags);
> +if (caps) {
> +xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
> +if (!xml) {
> +VIR_FREE(caps);
> +return false;
> +}
>  
> -vshPrint(ctl, "%d\n", vcpus);
> +virXPathInt("string(./vcpu[1]/@max)", ctxt, );

This doesn't handle the case when the capability XML does not contain
the required data. This still should fall back to the legacy approach.

Additionally virXPathInt does not initialize vcpus on failure and since
it's not initialized when declared this would trigger a compiler
warning.

> +xmlXPathFreeContext(ctxt);
> +xmlFreeDoc(xml);
> +VIR_FREE(caps);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Shivaprasad G Bhat]

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 
---
 tools/virsh-host.c |   28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..505cfbb 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
 {
 const char *type = NULL;
 int vcpus;
+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", ) < 0)
 return false;
 
-if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
-return false;
+caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, 
flags);
+if (caps) {
+xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
+if (!xml) {
+VIR_FREE(caps);
+return false;
+}
 
-vshPrint(ctl, "%d\n", vcpus);
+virXPathInt("string(./vcpu[1]/@max)", ctxt, );
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+VIR_FREE(caps);
+} else {
+if (last_error && last_error->code != VIR_ERR_NO_SUPPORT)
+return false;
+
+vshResetLibvirtError();
 
+if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
+return false;
+}
+
+vshPrint(ctl, "%d\n", vcpus);
 return true;
 }
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Peter Krempa]

On Wed, Jul 27, 2016 at 18:08:29 +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 qemu
> binary and to check for the host machine and arch which is what the
> command intends to do anyway.
> 
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  tools/virsh-host.c |   33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 57f0c0e..cf001c6 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -606,18 +606,43 @@ static bool
>  cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
>  {
>  const char *type = NULL;
> -int vcpus;
> +unsigned int vcpus;
> +char *caps = NULL;
> +const unsigned int flags = 0; /* No flags so far */
> +xmlDocPtr xml = NULL;
> +xmlXPathContextPtr ctxt = NULL;
> +bool ret = false;
>  virshControlPtr priv = ctl->privData;
>  
>  if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
>  return false;
>  
> -if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
> -return false;
> +caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, 
> type, flags);
> +if (!caps) {
> +vshError(ctl, "%s", _("failed to get domain capabilities"));
> +goto cleanup;
> +}

This will break compatibility when connecting to older versions of the
daemon which don't support the virConnectGetDomainCapabilities.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Shivaprasad G Bhat]

The following patch fixes the maxvcpus output on PPC64.
Earlier I tried fixing this in kvmGetMaxVcpus() which was
later decided to be document not use the virConnectGetMaxVcpus() instead
use the virConnectGetDomainCapabilities api.

I have not implemented the suggestedcpus as mentioned in my previous
patches in this yet.

Previous series fixing the virConnectGetDomainCapabilities can be found here.
https://www.redhat.com/archives/libvir-list/2016-June/msg01873.html

---

Shivaprasad G Bhat (1):
  virsh: use virConnectGetDomainCapabilities with maxvcpus


 tools/virsh-host.c |   33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

--
Signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Shivaprasad G Bhat]

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 qemu
binary and to check for the host machine and arch which is what the
command intends to do anyway.

Signed-off-by: Shivaprasad G Bhat 
---
 tools/virsh-host.c |   33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..cf001c6 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -606,18 +606,43 @@ static bool
 cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
 {
 const char *type = NULL;
-int vcpus;
+unsigned int vcpus;
+char *caps = NULL;
+const unsigned int flags = 0; /* No flags so far */
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+bool ret = false;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 return false;
 
-if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
-return false;
+caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, 
flags);
+if (!caps) {
+vshError(ctl, "%s", _("failed to get domain capabilities"));
+goto cleanup;
+}
+
+xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
+if (!xml) {
+vshError(ctl, "%s", _("unable to parse domain capabilities"));
+goto cleanup;
+}
+
+if ((virXPathUInt("string(./vcpu[1]/@max)", ctxt, )) < 0) {
+vshError(ctl, "%s", _("unable to get maxvcpus"));
+goto cleanup;
+}
 
 vshPrint(ctl, "%d\n", vcpus);
 
-return true;
+ret = true;
+ cleanup:
+VIR_FREE(caps);
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+
+return ret;
 }
 
 /*

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list