[libvirt] [PATCH] xen: work with ia64 MAX_VIRT_CPUS of 64
* src/xen/xen_hypervisor.c (MAX_VIRT_CPUS): Move... * src/xen/xen_driver.h (MAX_VIRT_CPUS): ...so all xen code can see same value. * src/xen/xend_internal.c (sexpr_to_xend_domain_info) (xenDaemonDomainGetVcpusFlags, xenDaemonParseSxpr) (xenDaemonFormatSxpr): Work if MAX_VIRT_CPUS is 64 on a platform where long is 64-bits. * src/xen/xm_internal.c (xenXMDomainConfigParse) (xenXMDomainConfigFormat): Likewise. --- On at least the xen/stable branch of git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git we have the case where MAX_VIRT_CPUS is 32 for x86 but 64 for ia64. That branch does not have XEN_LEGACY_MAX_VCPUS. But according to our comments, other branches had only XEN_LEGACY_MAX_VCPUS, which means that xm_internal.c would fail to compile without this patch, when targetting xen headers that lack MAX_VIRT_CPUS. src/xen/xen_driver.h | 8 ++++++++ src/xen/xen_hypervisor.c | 8 -------- src/xen/xend_internal.c | 17 +++++++++-------- src/xen/xm_internal.c | 9 +++++---- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 53f97d4..16d22f1 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -29,6 +29,14 @@ # include <winsock2.h> # endif +/* xen-unstable changeset 19788 removed MAX_VIRT_CPUS from public + * headers. Its semantic was retained with XEN_LEGACY_MAX_VCPUS. + * Ensure MAX_VIRT_CPUS is defined accordingly. + */ +# if !defined(MAX_VIRT_CPUS) && defined(XEN_LEGACY_MAX_VCPUS) +# define MAX_VIRT_CPUS XEN_LEGACY_MAX_VCPUS +# endif + extern int xenRegister (void); # define XEN_UNIFIED_HYPERVISOR_OFFSET 0 diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 3797865..ec726fe 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -109,14 +109,6 @@ typedef privcmd_hypercall_t hypercall_t; # define SYS_IFACE_MIN_VERS_NUMA 4 #endif -/* xen-unstable changeset 19788 removed MAX_VIRT_CPUS from public - * headers. Its semanitc was retained with XEN_LEGACY_MAX_VCPUS. - * Ensure MAX_VIRT_CPUS is defined accordingly. - */ -#if !defined(MAX_VIRT_CPUS) && defined(XEN_LEGACY_MAX_VCPUS) -# define MAX_VIRT_CPUS XEN_LEGACY_MAX_VCPUS -#endif - static int xen_ioctl_hypercall_cmd = 0; static int initialized = 0; static int in_init = 0; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 974e96b..614c036 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2192,7 +2192,7 @@ xenDaemonParseSxpr(virConnectPtr conn, } def->maxvcpus = sexpr_int(root, "domain/vcpus"); - def->vcpus = count_one_bits(sexpr_int(root, "domain/vcpu_avail")); + def->vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); if (!def->vcpus || def->maxvcpus < def->vcpus) def->vcpus = def->maxvcpus; @@ -2468,7 +2468,7 @@ sexpr_to_xend_domain_info(virDomainPtr domain, const struct sexpr *root, } info->cpuTime = sexpr_float(root, "domain/cpu_time") * 1000000000; vcpus = sexpr_int(root, "domain/vcpus"); - info->nrVirtCpu = count_one_bits(sexpr_int(root, "domain/vcpu_avail")); + info->nrVirtCpu = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); if (!info->nrVirtCpu || vcpus < info->nrVirtCpu) info->nrVirtCpu = vcpus; @@ -3706,7 +3706,7 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) ret = sexpr_int(root, "domain/vcpus"); if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM)) { - int vcpus = count_one_bits(sexpr_int(root, "domain/vcpu_avail")); + int vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); if (vcpus) ret = MIN(vcpus, ret); } @@ -5770,10 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); if (def->cpumask) { char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen); @@ -5870,8 +5871,8 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", - (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", + (1UL << def->vcpus) - 1); for (i = 0 ; i < def->os.nBootDevs ; i++) { switch (def->os.bootDevs[i]) { diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index f80e252..6c5df0f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -776,7 +776,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { def->maxvcpus = count; if (xenXMConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) goto cleanup; - def->vcpus = MIN(count_one_bits(count), def->maxvcpus); + def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus); if (xenXMConfigGetString(conf, "cpus", &str, NULL) < 0) goto cleanup; @@ -2336,10 +2336,11 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) goto no_memory; - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus && - xenXMConfigSetInt(conf, "vcpu_avail", (1U << def->vcpus) - 1) < 0) + xenXMConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) goto no_memory; if ((def->cpumask != NULL) && -- 1.7.2.3
Il giorno ven, 29/10/2010 alle 11.19 -0600, Eric Blake ha scritto:
* src/xen/xen_hypervisor.c (MAX_VIRT_CPUS): Move... * src/xen/xen_driver.h (MAX_VIRT_CPUS): ...so all xen code can see same value.
This seems to be needed for libvirt to build with xen as I was reported on Gentoo Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=343353 -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/
On Fri, Oct 29, 2010 at 11:19:50AM -0600, Eric Blake wrote:
* src/xen/xen_hypervisor.c (MAX_VIRT_CPUS): Move... * src/xen/xen_driver.h (MAX_VIRT_CPUS): ...so all xen code can see same value. * src/xen/xend_internal.c (sexpr_to_xend_domain_info) (xenDaemonDomainGetVcpusFlags, xenDaemonParseSxpr) (xenDaemonFormatSxpr): Work if MAX_VIRT_CPUS is 64 on a platform where long is 64-bits. * src/xen/xm_internal.c (xenXMDomainConfigParse) (xenXMDomainConfigFormat): Likewise.
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 11/01/2010 03:36 AM, Daniel Veillard wrote:
On Fri, Oct 29, 2010 at 11:19:50AM -0600, Eric Blake wrote:
* src/xen/xen_hypervisor.c (MAX_VIRT_CPUS): Move... * src/xen/xen_driver.h (MAX_VIRT_CPUS): ...so all xen code can see same value. * src/xen/xend_internal.c (sexpr_to_xend_domain_info) (xenDaemonDomainGetVcpusFlags, xenDaemonParseSxpr) (xenDaemonFormatSxpr): Work if MAX_VIRT_CPUS is 64 on a platform where long is 64-bits. * src/xen/xm_internal.c (xenXMDomainConfigParse) (xenXMDomainConfigFormat): Likewise.
ACK,
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Oct 29, 2010 at 11:19:50AM -0600, Eric Blake wrote:
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 974e96b..614c036 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -5770,10 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1);
if (def->cpumask) { char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen);
This chunk causes a compile error for me CC libvirt_driver_xen_la-xend_internal.lo cc1: warnings being treated as errors xen/xend_internal.c: In function 'xenDaemonFormatSxpr': xen/xend_internal.c:5775: error: nested extern declaration of 'verify_function2' [-Wnested-externs] make[3]: *** [libvirt_driver_xen_la-xend_internal.lo] Error 1 Also, what was commited, looks different to this diff here @@ -5770,9 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); if (def->cpumask) { char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen); eg, main GIT repo didn't have any existing use of 'verify' removed, whereas your patch did ?!?!?
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index f80e252..6c5df0f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2336,10 +2336,11 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn,
if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) goto no_memory; - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus && - xenXMConfigSetInt(conf, "vcpu_avail", (1U << def->vcpus) - 1) < 0) + xenXMConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) goto no_memory;
if ((def->cpumask != NULL) &&
The same with this chunk Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/01/2010 10:35 AM, Daniel P. Berrange wrote:
On Fri, Oct 29, 2010 at 11:19:50AM -0600, Eric Blake wrote:
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 974e96b..614c036 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -5770,10 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1);
if (def->cpumask) { char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen);
This chunk causes a compile error for me
CC libvirt_driver_xen_la-xend_internal.lo cc1: warnings being treated as errors xen/xend_internal.c: In function 'xenDaemonFormatSxpr': xen/xend_internal.c:5775: error: nested extern declaration of 'verify_function2' [-Wnested-externs] make[3]: *** [libvirt_driver_xen_la-xend_internal.lo] Error 1
Which version of gcc? This may be a bug in gnulib's verify module. I tested successfully on F13, gcc 4.4.4.
Also, what was commited, looks different to this diff here
@@ -5770,9 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1);
As Diego pointed out, the diff posted to this list was incomplete; it was a partial patch that I had forgotten to squash in to my first attempt. What I actually pushed was both patches squashed together; for reference, here's what the first patch was: From b67847f84d9d4e8dbad5200df247212bc14221ab Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Fri, 29 Oct 2010 10:51:01 -0600 Subject: [PATCH] xen: verify code assumption about MAX_VIRT_CPUS * src/xen/xend_internal.c (xenDaemonFormatSxpr): Detect if upstream xen ever changes to support 64 vcpus. * src/xen/xm_internal.c (xenXMDomainConfigFormat): Likewise. --- src/xen/xend_internal.c | 1 + src/xen/xm_internal.c | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index e96b762..974e96b 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -5771,6 +5771,7 @@ xenDaemonFormatSxpr(virConnectPtr conn, def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ + verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); if (def->vcpus < def->maxvcpus) virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 430d40b..f80e252 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2336,6 +2336,8 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) goto no_memory; + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ + verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); if (def->vcpus < def->maxvcpus && xenXMConfigSetInt(conf, "vcpu_avail", (1U << def->vcpus) - 1) < 0) goto no_memory; -- 1.7.2.3 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Nov 01, 2010 at 10:48:34AM -0600, Eric Blake wrote:
On 11/01/2010 10:35 AM, Daniel P. Berrange wrote:
On Fri, Oct 29, 2010 at 11:19:50AM -0600, Eric Blake wrote:
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 974e96b..614c036 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -5770,10 +5770,11 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); - /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is 32. */ - verify(MAX_VIRT_CPUS <= sizeof(1U) * CHAR_BIT); + /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ + verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) - virBufferVSprintf(&buf, "(vcpu_avail %u)", (1U << def->vcpus) - 1); + virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1);
if (def->cpumask) { char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen);
This chunk causes a compile error for me
CC libvirt_driver_xen_la-xend_internal.lo cc1: warnings being treated as errors xen/xend_internal.c: In function 'xenDaemonFormatSxpr': xen/xend_internal.c:5775: error: nested extern declaration of 'verify_function2' [-Wnested-externs] make[3]: *** [libvirt_driver_xen_la-xend_internal.lo] Error 1
Which version of gcc? This may be a bug in gnulib's verify module. I tested successfully on F13, gcc 4.4.4.
It fails on F12 gcc-4.4.3-4.fc12.x86_64, and on F 14 gcc-4.5.0-4.fc14.x86_64 Making it use verify_true() instead, works though Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/01/2010 11:05 AM, Daniel P. Berrange wrote:
+ verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT);
Which version of gcc? This may be a bug in gnulib's verify module. I tested successfully on F13, gcc 4.4.4.
It fails on F12 gcc-4.4.3-4.fc12.x86_64, and on F 14 gcc-4.5.0-4.fc14.x86_64
Making it use verify_true() instead, works though
The difference being that verify() is a declaration, while verify_true() is an integer constant expression. OK, I'll prepare the patch along those lines. In the meantime, I'm also seeing a failure when trying to compile in rawhide: xen/xend_internal.c: In function 'xenDaemonFormatSxpr': xen/xend_internal.c:5775:112: error: 'MAX_VIRT_CPUS' undeclared (first use in this function) which was exactly what my patch was supposed to avoid, so I have to figure out how to convince things to work with xen-devel-4.0.1-6.fc15 headers. :( -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* src/xen/xen_driver.h (includes): Include main xen headers here... * src/xen/xs_internal.c (includes): ...rather than in just one of the sub-drivers. --- I'm waiting for an ACK before pushing this, but it sure seems pretty trivial. I don't know why the xen 4.0.1 headers of rawhide are different from the xen 3.4.3 headers of Fedora 13 (translation: something used to implicitly include xen/xen.h in the older headers, but no longer does in the newer xen, but I didn't bother to figure out where the inclusion chain differs). Tested on F13 and rawhide; fixes the MAX_VIRT_CPUS undeclared issue that was occurring on rawhide. src/xen/xen_driver.h | 2 ++ src/xen/xs_internal.c | 1 - 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 16d22f1..6af6132 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -29,6 +29,8 @@ # include <winsock2.h> # endif +# include <xen/xen.h> + /* xen-unstable changeset 19788 removed MAX_VIRT_CPUS from public * headers. Its semantic was retained with XEN_LEGACY_MAX_VCPUS. * Ensure MAX_VIRT_CPUS is defined accordingly. diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index a9817b1..eba1b95 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -22,7 +22,6 @@ #include <xen/dom0_ops.h> #include <xen/version.h> -#include <xen/xen.h> #include <xs.h> -- 1.7.2.3
Eric Blake wrote:
* src/xen/xen_driver.h (includes): Include main xen headers here... * src/xen/xs_internal.c (includes): ...rather than in just one of the sub-drivers. ---
I'm waiting for an ACK before pushing this, but it sure seems pretty trivial. I don't know why the xen 4.0.1 headers of rawhide are different from the xen 3.4.3 headers of Fedora 13 (translation: something used to implicitly include xen/xen.h in the older headers, but no longer does in the newer xen, but I didn't bother to figure out where the inclusion chain differs).
Tested on F13 and rawhide; fixes the MAX_VIRT_CPUS undeclared issue that was occurring on rawhide.
src/xen/xen_driver.h | 2 ++ src/xen/xs_internal.c | 1 - 2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 16d22f1..6af6132 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -29,6 +29,8 @@ # include <winsock2.h> # endif
+# include <xen/xen.h> + /* xen-unstable changeset 19788 removed MAX_VIRT_CPUS from public * headers. Its semantic was retained with XEN_LEGACY_MAX_VCPUS. * Ensure MAX_VIRT_CPUS is defined accordingly. diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index a9817b1..eba1b95 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -22,7 +22,6 @@
#include <xen/dom0_ops.h> #include <xen/version.h> -#include <xen/xen.h>
#include <xs.h>
ACK. libvirt 0.8.5 + this patch + commit dc27e089 + commit b164db62 = successful builds against xen 3.3.1, 3.4.1, and 4.0.1. I did notice commit e8066d53 broke the build on older openSUSE containing polkit0. I'll send a patch for that shortly. Regards, Jim
On 11/01/2010 05:45 PM, Jim Fehlig wrote:
Eric Blake wrote:
* src/xen/xen_driver.h (includes): Include main xen headers here... * src/xen/xs_internal.c (includes): ...rather than in just one of the sub-drivers. ---
+++ b/src/xen/xen_driver.h @@ -29,6 +29,8 @@ # include <winsock2.h> # endif
+# include <xen/xen.h> + /* xen-unstable changeset 19788 removed MAX_VIRT_CPUS from public * headers. Its semantic was retained with XEN_LEGACY_MAX_VCPUS. * Ensure MAX_VIRT_CPUS is defined accordingly.
ACK.
libvirt 0.8.5 + this patch + commit dc27e089 + commit b164db62 = successful builds against xen 3.3.1, 3.4.1, and 4.0.1. I did notice commit e8066d53 broke the build on older openSUSE containing polkit0.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/01/2010 10:48 AM, Eric Blake wrote:
cc1: warnings being treated as errors xen/xend_internal.c: In function 'xenDaemonFormatSxpr': xen/xend_internal.c:5775: error: nested extern declaration of 'verify_function2' [-Wnested-externs] make[3]: *** [libvirt_driver_xen_la-xend_internal.lo] Error 1
Which version of gcc? This may be a bug in gnulib's verify module. I tested successfully on F13, gcc 4.4.4.
Oops - I found out (the hard way) that --enable-compiler-warnings=error is not the same as --enable-compile-warnings=error. (We only support the latter). Once I fixed my devel build to turn on -Werror, I'm also seeing this failure with my setup (which was to be expected, since Daniel's F12 and F14 tests both failed). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* src/xen/xend_internal.c (xenDaemonFormatSxpr): Hoist verify outside of function to avoid a -Wnested-externs warning. * src/xen/xm_internal.c (xenXMDomainConfigFormat): Likewise. Reported by Daniel P. Berrange. ---
Making it use verify_true() instead, works though
Not really, as that then triggers an unused expression warning. But hoisting outside of a function works. I'm pushing this now, under the build-breaker rule, and then focusing on fixing things for the xen 4 headers in a separate patch (as I still don't have that one fixed locally yet). src/xen/xend_internal.c | 5 ++++- src/xen/xm_internal.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 614c036..5c3a4bd 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -5744,6 +5744,10 @@ xenDaemonFormatSxprInput(virDomainInputDefPtr input, } +/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ +verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); + /** * xenDaemonFormatSxpr: * @conn: pointer to the hypervisor connection @@ -5772,7 +5776,6 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ - verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus) virBufferVSprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 6c5df0f..a4d1a30 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2304,6 +2304,10 @@ error: } +/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is + either 32, or 64 on a platform where long is big enough. */ +verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); + virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, virDomainDefPtr def) { virConfPtr conf = NULL; @@ -2338,7 +2342,6 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, goto no_memory; /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ - verify(MAX_VIRT_CPUS <= sizeof(1UL) * CHAR_BIT); if (def->vcpus < def->maxvcpus && xenXMConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) goto no_memory; -- 1.7.2.3
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Diego Elio Pettenò -
Eric Blake -
Jim Fehlig