[libvirt] virDomainGetMaxVcpus does not work as expected
Hi. When calling virDomainGetMaxVcpus (http://libvirt.org/html/libvirt-libvirt.html#virDomainGetMaxVcpus) on an inactive domain, I receive this error: scala> res2.getMaxVcpus() libvirt: Domain Config error : Requested operation is not valid: domain is not running org.libvirt.LibvirtException: Requested operation is not valid: domain is not running at org.libvirt.ErrorHandler.processError(ErrorHandler.java:31) at org.libvirt.ErrorHandler.processError(ErrorHandler.java:46) at org.libvirt.Domain.getMaxVcpus(Domain.java:571) at .<init>(<console>:13) ... (this is from Java, but that doesn't matter) The docs say:
If the guest is inactive, this is basically the same as virConnectGetMaxVcpus(). If the guest is running this will reflect the maximum number of virtual CPUs the guest was booted with.
But, apparently, all the driver implementations for virDomainGetMaxVcpus forward to <driver>DomainGetVcpusFlags(.., VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM). _______________________________,~~~~~~~~~~~~~~~~~~~~~~ $ git grep --show-function 'GetVcpusFlags.*AFFECT_LIVE' src/esx/esx_driver.c=esxDomainGetMaxVcpus(virDomainPtr domain) src/esx/esx_driver.c: return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/openvz/openvz_driver.c=static int openvzDomainGetMaxVcpus(virDomainPtr dom) src/openvz/openvz_driver.c: return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/qemu/qemu_driver.c=qemuDomainGetMaxVcpus(virDomainPtr dom) src/qemu/qemu_driver.c: return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/test/test_driver.c=testDomainGetMaxVcpus(virDomainPtr domain) src/test/test_driver.c: return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/vbox/vbox_tmpl.c=vboxDomainGetMaxVcpus(virDomainPtr dom) src/vbox/vbox_tmpl.c: return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | AFAICS, this was introduced with commit 50c51f13e2af04afac46e181c4ed62581545a488 Author: Eric Blake <eblake@redhat.com> Date: Mon Sep 27 16:37:53 2010 -0600 vcpu: make old API trivially wrap to new API Whereas the function's contract was documented earlier by commit b412cfadb502c76df095c2c4548c27abf7c4873f Author: Daniel Veillard <veillard@redhat.com> Date: Thu Mar 8 08:31:07 2007 +0000 To be honest, I'm not sure whether this worked as described at some time in the past _at all_. How to fix this? Change the documentation or the flag? Claudio
Hi. Any opinions on this? At Thu, 13 Feb 2014 14:50:08 +0100, Claudio Bley wrote:
Hi.
When calling virDomainGetMaxVcpus (http://libvirt.org/html/libvirt-libvirt.html#virDomainGetMaxVcpus) on an inactive domain, I receive this error:
scala> res2.getMaxVcpus() libvirt: Domain Config error : Requested operation is not valid: domain is not running org.libvirt.LibvirtException: Requested operation is not valid: domain is not running at org.libvirt.ErrorHandler.processError(ErrorHandler.java:31) at org.libvirt.ErrorHandler.processError(ErrorHandler.java:46) at org.libvirt.Domain.getMaxVcpus(Domain.java:571) at .<init>(<console>:13) ...
(this is from Java, but that doesn't matter)
The docs say:
If the guest is inactive, this is basically the same as virConnectGetMaxVcpus(). If the guest is running this will reflect the maximum number of virtual CPUs the guest was booted with.
But, apparently, all the driver implementations for virDomainGetMaxVcpus forward to <driver>DomainGetVcpusFlags(.., VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM). _______________________________,~~~~~~~~~~~~~~~~~~~~~~
$ git grep --show-function 'GetVcpusFlags.*AFFECT_LIVE' src/esx/esx_driver.c=esxDomainGetMaxVcpus(virDomainPtr domain) src/esx/esx_driver.c: return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/openvz/openvz_driver.c=static int openvzDomainGetMaxVcpus(virDomainPtr dom) src/openvz/openvz_driver.c: return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/qemu/qemu_driver.c=qemuDomainGetMaxVcpus(virDomainPtr dom) src/qemu/qemu_driver.c: return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/test/test_driver.c=testDomainGetMaxVcpus(virDomainPtr domain) src/test/test_driver.c: return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/vbox/vbox_tmpl.c=vboxDomainGetMaxVcpus(virDomainPtr dom) src/vbox/vbox_tmpl.c: return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
AFAICS, this was introduced with
commit 50c51f13e2af04afac46e181c4ed62581545a488 Author: Eric Blake <eblake@redhat.com> Date: Mon Sep 27 16:37:53 2010 -0600
vcpu: make old API trivially wrap to new API
Whereas the function's contract was documented earlier by
commit b412cfadb502c76df095c2c4548c27abf7c4873f Author: Daniel Veillard <veillard@redhat.com> Date: Thu Mar 8 08:31:07 2007 +0000
To be honest, I'm not sure whether this worked as described at some time in the past _at all_.
How to fix this? Change the documentation or the flag?
Claudio
On 02/18/2014 02:11 AM, Claudio Bley wrote:
Hi. Any opinions on this?
At Thu, 13 Feb 2014 14:50:08 +0100, Claudio Bley wrote:
Hi.
When calling virDomainGetMaxVcpus (http://libvirt.org/html/libvirt-libvirt.html#virDomainGetMaxVcpus) on an inactive domain, I receive this error:
scala> res2.getMaxVcpus() libvirt: Domain Config error : Requested operation is not valid: domain is not running org.libvirt.LibvirtException: Requested operation is not valid: domain is not running at org.libvirt.ErrorHandler.processError(ErrorHandler.java:31) at org.libvirt.ErrorHandler.processError(ErrorHandler.java:46) at org.libvirt.Domain.getMaxVcpus(Domain.java:571) at .<init>(<console>:13) ...
(this is from Java, but that doesn't matter)
The docs say:
If the guest is inactive, this is basically the same as virConnectGetMaxVcpus(). If the guest is running this will reflect the maximum number of virtual CPUs the guest was booted with.
But, apparently, all the driver implementations for virDomainGetMaxVcpus forward to <driver>DomainGetVcpusFlags(.., VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM). _______________________________,~~~~~~~~~~~~~~~~~~~~~~
$ git grep --show-function 'GetVcpusFlags.*AFFECT_LIVE' src/esx/esx_driver.c=esxDomainGetMaxVcpus(virDomainPtr domain) src/esx/esx_driver.c: return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/openvz/openvz_driver.c=static int openvzDomainGetMaxVcpus(virDomainPtr dom) src/openvz/openvz_driver.c: return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/qemu/qemu_driver.c=qemuDomainGetMaxVcpus(virDomainPtr dom) src/qemu/qemu_driver.c: return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | src/test/test_driver.c=testDomainGetMaxVcpus(virDomainPtr domain) src/test/test_driver.c: return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | src/vbox/vbox_tmpl.c=vboxDomainGetMaxVcpus(virDomainPtr dom) src/vbox/vbox_tmpl.c: return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
AFAICS, this was introduced with
commit 50c51f13e2af04afac46e181c4ed62581545a488 Author: Eric Blake <eblake@redhat.com> Date: Mon Sep 27 16:37:53 2010 -0600
vcpu: make old API trivially wrap to new API
Whereas the function's contract was documented earlier by
commit b412cfadb502c76df095c2c4548c27abf7c4873f Author: Daniel Veillard <veillard@redhat.com> Date: Thu Mar 8 08:31:07 2007 +0000
To be honest, I'm not sure whether this worked as described at some time in the past _at all_.
How to fix this? Change the documentation or the flag?
I think the implementations just need to do something like: if (domain_is_active) return DomainGetMaxVCPUs() return ConnectGetMaxVCPUs() So, code change not documentation change. - Cole
On 02/13/2014 06:50 AM, Claudio Bley wrote:
The docs say:
If the guest is inactive, this is basically the same as virConnectGetMaxVcpus(). If the guest is running this will reflect the maximum number of virtual CPUs the guest was booted with.
But, apparently, all the driver implementations for virDomainGetMaxVcpus forward to <driver>DomainGetVcpusFlags(.., VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM). _______________________________,~~~~~~~~~~~~~~~~~~~~~~
Looks like I caused a regression when I added the Flags variant in 2010 (that was my first real API addition when I was new to libvirt coding, if I recall). Fixing the code to match the docs sounds like the right way to go here.
To be honest, I'm not sure whether this worked as described at some time in the past _at all_.
How to fix this? Change the documentation or the flag?
The newer Flags API should be preferred, but that doesn't mean we can't fix the older API to behave more sanely. On the back-compat perspective, changing something that used to fail into something that now succeeds is okay (you can tell whether you are talking to an older libvirt by whether the failure happens). [The opposite direction of changing something that succeeds into an error is not nice unless it is done to plug a security hole, since older code may be depending on the success path and should not need to be rewritten to new API just to keep things working] -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch? While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency? src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..6e05d78 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) static int esxDomainGetMaxVcpus(virDomainPtr domain) { - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b27ac4c..e1f9738 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1318,7 +1318,7 @@ openvzDomainGetVcpusFlags(virDomainPtr dom ATTRIBUTE_UNUSED, static int openvzDomainGetMaxVcpus(virDomainPtr dom) { - return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9adb6b0..4510b82 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1372,7 +1372,7 @@ phypDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int phypDomainGetMaxVcpus(virDomainPtr dom) { - return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ea837e..2068ace 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4986,7 +4986,7 @@ cleanup: static int qemuDomainGetMaxVcpus(virDomainPtr dom) { - return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..a320241 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2620,7 +2620,7 @@ cleanup: static int testDomainGetMaxVcpus(virDomainPtr domain) { - return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 382d7b4..fbae940 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2208,7 +2208,7 @@ vboxDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int vboxDomainGetMaxVcpus(virDomainPtr dom) { - return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7506e8c..38bbce0 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1539,7 +1539,7 @@ cleanup: static int xenUnifiedDomainGetMaxVcpus(virDomainPtr dom) { - return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index a547306..688d162 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1347,7 +1347,7 @@ xenapiDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int xenapiDomainGetMaxVcpus(virDomainPtr dom) { - return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); } -- 1.7.9.5
Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +-
I think the libxl driver should be included here too. Regards, Jim
8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..6e05d78 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) static int esxDomainGetMaxVcpus(virDomainPtr domain) { - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b27ac4c..e1f9738 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1318,7 +1318,7 @@ openvzDomainGetVcpusFlags(virDomainPtr dom ATTRIBUTE_UNUSED,
static int openvzDomainGetMaxVcpus(virDomainPtr dom) { - return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9adb6b0..4510b82 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1372,7 +1372,7 @@ phypDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int phypDomainGetMaxVcpus(virDomainPtr dom) { - return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ea837e..2068ace 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4986,7 +4986,7 @@ cleanup: static int qemuDomainGetMaxVcpus(virDomainPtr dom) { - return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..a320241 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2620,7 +2620,7 @@ cleanup: static int testDomainGetMaxVcpus(virDomainPtr domain) { - return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 382d7b4..fbae940 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2208,7 +2208,7 @@ vboxDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int vboxDomainGetMaxVcpus(virDomainPtr dom) { - return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7506e8c..38bbce0 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1539,7 +1539,7 @@ cleanup: static int xenUnifiedDomainGetMaxVcpus(virDomainPtr dom) { - return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index a547306..688d162 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1347,7 +1347,7 @@ xenapiDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int xenapiDomainGetMaxVcpus(virDomainPtr dom) { - return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
At Thu, 20 Feb 2014 12:19:35 -0700, Jim Fehlig wrote:
Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +-
I think the libxl driver should be included here too.
Seems the libxl driver does not implement the virDomainGetMaxVcpus function. Claudio -- BSc (Comp) Claudio Bley - Principal Software Engineer AV-TEST GmbH, Klewitzstr. 7, 39112 Magdeburg, Germany Phone: +49 391 6075460, Fax: +49 391 6075469 Web: <http://www.av-test.org> * https://twitter.com/avtestorg * https://facebook.com/avtestorg * * https://plus.google.com/100383867141221115206/ * Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern Our services shall be effected on the basis of the General Terms and Conditions of AV-TEST GmbH, which are accessible under <http://www.av-test.org/en/av-test/terms-and-conditions/> or obtainable upon request. Unsere Leistungen erfolgen auf der Grundlage der Allgemeinen Geschäftsbedingungen der AV-TEST GmbH, die unter <http://www.av-test.org/av-test/agb/> abrufbar sind oder auf Anfrage übersandt werden.
Claudio Bley wrote:
At Thu, 20 Feb 2014 12:19:35 -0700, Jim Fehlig wrote:
Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +-
I think the libxl driver should be included here too.
Seems the libxl driver does not implement the virDomainGetMaxVcpus function.
Opps, you are right. The libxl driver only has domainGetVcpus. Sorry for the noise. Regards, Jim
On 19.02.2014 13:03, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..6e05d78 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) static int esxDomainGetMaxVcpus(virDomainPtr domain) { - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This won't work esxDomainGetVcpusFlags() requires flags to be exactly (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM).
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b27ac4c..e1f9738 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1318,7 +1318,7 @@ openvzDomainGetVcpusFlags(virDomainPtr dom ATTRIBUTE_UNUSED,
static int openvzDomainGetMaxVcpus(virDomainPtr dom) { - return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
Same here.
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 9adb6b0..4510b82 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1372,7 +1372,7 @@ phypDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int phypDomainGetMaxVcpus(virDomainPtr dom) { - return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
And here.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ea837e..2068ace 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4986,7 +4986,7 @@ cleanup: static int qemuDomainGetMaxVcpus(virDomainPtr dom) { - return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This bit is okay.
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..a320241 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2620,7 +2620,7 @@ cleanup: static int testDomainGetMaxVcpus(virDomainPtr domain) { - return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
And so is this.
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 382d7b4..fbae940 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2208,7 +2208,7 @@ vboxDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int vboxDomainGetMaxVcpus(virDomainPtr dom) { - return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | + return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This is not again. vboxDomainGetVcpusFlags() requires flags == (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM).
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7506e8c..38bbce0 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1539,7 +1539,7 @@ cleanup: static int xenUnifiedDomainGetMaxVcpus(virDomainPtr dom) { - return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This good.
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index a547306..688d162 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1347,7 +1347,7 @@ xenapiDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) static int xenapiDomainGetMaxVcpus(virDomainPtr dom) { - return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE | + return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This is not. I haven't checked if all *DomainGetVcpusFlags() that require just _LIVE really needs it or if they can work on inactive domains as well. But if they do, it should be a separate patch queued before this one. Michal
At Mon, 24 Feb 2014 15:18:21 +0100, Michal Privoznik wrote:
On 19.02.2014 13:03, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..6e05d78 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) static int esxDomainGetMaxVcpus(virDomainPtr domain) { - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This won't work esxDomainGetVcpusFlags() requires flags to be exactly (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM).
Would it be OK to change the function like this, until the esxDomainGetVcpusFlags function is fixed? ---------------------------------------------------------------------- static int esxDomainGetMaxVcpus(virDomainPtr domain) { switch (esxDomainIsActive(domain)) { case 0: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("esxDomainGetMaxVcpus is not supported for inactive domains")); case -1: return -1; default: return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM)); } } ---------------------------------------------------------------------- Likewise for the other broken drivers... Claudio
At Mon, 10 Mar 2014 09:22:53 +0100, Claudio Bley wrote:
At Mon, 24 Feb 2014 15:18:21 +0100, Michal Privoznik wrote:
On 19.02.2014 13:03, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
src/esx/esx_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xen_driver.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..6e05d78 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) static int esxDomainGetMaxVcpus(virDomainPtr domain) { - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM)); }
This won't work esxDomainGetVcpusFlags() requires flags to be exactly (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM).
Would it be OK to change the function like this, until the esxDomainGetVcpusFlags function is fixed?
Ditch this. I'm obviously still half asleep this morning... You basically get the same error without touching the code. Claudio
On 02/19/2014 05:03 AM, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- OK, how about this patch?
While at it, should I convert the VIR_DOMAIN_VCPU_* instances to VIR_DOMAIN_AFFECT_* instances for consistency?
Yes, making that conversion as a separate patch for consistency would be nice.
{ - return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE | + return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_VCPU_MAXIMUM));
VIR_DOMAIN_AFFECT_CURRENT == 0, so you could simplify this to just passing VIR_DOMAIN_VCPU_MAXIMUM instead of a bitwise-or. On the other hand, making the _CURRENT explicit may be a bit more self-documenting, so I'm not going to insist. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (5)
-
Claudio Bley -
Cole Robinson -
Eric Blake -
Jim Fehlig -
Michal Privoznik