[libvirt] [PATCH] libxl: libxl_get_max_cpus returning a libxl error from 4.4 onward
Starting from commit 2e82c18c in Xen (will be included in Xen 4.4) libxl_get_max_cpus() start returning a proper libxl error code, in case of failure. It returning 0 is now basically impossible but, theoretically, still wrong, not to mention that using '<= 0' makes this correct for both Xen 4.4 and Xen 4.3 (and 4.2). That's why we go for it (rather than just '< 0'). Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> --- src/libxl/libxl_driver.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 692c3b7..a31b094 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1101,9 +1101,11 @@ libxlConnectGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) cfg = libxlDriverConfigGet(driver); ret = libxl_get_max_cpus(cfg->ctx); - /* libxl_get_max_cpus() will return 0 if there were any failures, - e.g. xc_physinfo() failing */ - if (ret == 0) + /* On failure, libxl_get_max_cpus() will return ERROR_FAIL from Xen 4.4 + * onward, but it ever returning 0 is obviously wrong too (and it is + * what happens, on failure, on Xen 4.3 and earlier). Therefore, a 'less + * or equal' is the catchall we want. */ + if (ret <= 0) ret = -1; virObjectUnref(cfg);
On Tue, Dec 17, 2013 at 06:43:25PM +0100, Dario Faggioli wrote:
Starting from commit 2e82c18c in Xen (will be included in Xen 4.4) libxl_get_max_cpus() start returning a proper libxl error code, in case of failure. It returning 0 is now basically impossible but, theoretically, still wrong, not to mention that using '<= 0' makes this correct for both Xen 4.4 and Xen 4.3 (and 4.2). That's why we go for it (rather than just '< 0').
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> --- src/libxl/libxl_driver.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 692c3b7..a31b094 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1101,9 +1101,11 @@ libxlConnectGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED)
cfg = libxlDriverConfigGet(driver); ret = libxl_get_max_cpus(cfg->ctx); - /* libxl_get_max_cpus() will return 0 if there were any failures, - e.g. xc_physinfo() failing */ - if (ret == 0) + /* On failure, libxl_get_max_cpus() will return ERROR_FAIL from Xen 4.4 + * onward, but it ever returning 0 is obviously wrong too (and it is + * what happens, on failure, on Xen 4.3 and earlier). Therefore, a 'less + * or equal' is the catchall we want. */ + if (ret <= 0) ret = -1;
virObjectUnref(cfg);
Good to change that, thanks, but the call to: virBitmapNew(libxl_get_max_nodes(priv->ctx)) // in the same file should be fixed as well, especially since virBitmapNew takes an unsigned argument. Could you modify that bit as well? Martin
On mer, 2013-12-18 at 07:39 +0100, Martin Kletzander wrote:
On Tue, Dec 17, 2013 at 06:43:25PM +0100, Dario Faggioli wrote:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 692c3b7..a31b094 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1101,9 +1101,11 @@ libxlConnectGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED)
cfg = libxlDriverConfigGet(driver); ret = libxl_get_max_cpus(cfg->ctx); - /* libxl_get_max_cpus() will return 0 if there were any failures, - e.g. xc_physinfo() failing */ - if (ret == 0) + /* On failure, libxl_get_max_cpus() will return ERROR_FAIL from Xen 4.4 + * onward, but it ever returning 0 is obviously wrong too (and it is + * what happens, on failure, on Xen 4.3 and earlier). Therefore, a 'less + * or equal' is the catchall we want. */ + if (ret <= 0) ret = -1;
virObjectUnref(cfg);
Good to change that, thanks, but the call to: virBitmapNew(libxl_get_max_nodes(priv->ctx)) // in the same file should be fixed as well, especially since virBitmapNew takes an unsigned argument. Could you modify that bit as well?
Right! I checkd for other uses of *_max_cpus() and forgot to do the same for *_max_nodes()! :-/ I'll do that, thanks for looking at the patch. Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
participants (2)
-
Dario Faggioli -
Martin Kletzander