[libvirt] [PATCH 0/7 RFC] RHEL-5 support, and a few preparatory fixes
This patch series attempts to make libvirt just work on RHEL-5. Right now it doesn't, mostly because libvirt relies on version number checks in a couple of places, and RHEL-5's version numbers aren't the whole truth due to various backports of later stuff. The first two patches are plain bug fixes: [PATCH 1/7] Fix network device inconsistency between xm and sxpr [PATCH 2/7] Fix graphics configuration inconsistency between xm and sxpr The next three add a few special cases for RHEL-5: [PATCH 3/7] New configure option --with-rhel5-api [PATCH 4/7] Fix HVM network device configuration for RHEL-5 [PATCH 5/7] Fix PVFB device configuration for RHEL-5 [PATCH 6/7] Enable NUMA support for RHEL-5 The last one drops a counter-productive error check: [PATCH 7/7] Don't treat missing topology information as error See the patches for more detailed descriptions. I'm not proposing this for immediate commit, as I'm still testing. But I'd appreciate review: is this the right way to do it? Thanks, Markus
xenDaemonFormatSxpr() adds (type ioemu) only if xendConfigVersion < 4. xenXMDomainConfigFormatNet() adds type=ioemu always. Change it make it consistent with xenDaemonFormatSxpr(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xm_internal.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 77aa3e1..b3be199 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1808,6 +1808,7 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn, virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; char *str; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virBufferVSprintf(&buf, "mac=%02x:%02x:%02x:%02x:%02x:%02x", net->mac[0], net->mac[1], @@ -1836,7 +1837,7 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn, goto cleanup; } - if (hvm) + if (hvm && priv->xendConfigVersion < 4) virBufferAddLit(&buf, ",type=ioemu"); if (net->model) -- 1.5.4.3
xenDaemonFormatSxpr() generates old-style HVM graphics configuration when xendConfigVersion is below 4. xenXMDomainConfigFormat() does that always. Change it to make it consistent with xenDaemonFormatSxpr(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xm_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index b3be199..7cbfc7e 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2049,7 +2049,7 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, } if (def->graphics) { - if (hvm || priv->xendConfigVersion < 3) { + if (priv->xendConfigVersion < (hvm ? 4 : 3)) { if (def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { if (xenXMConfigSetInt(conf, "sdl", 1) < 0) goto no_memory; -- 1.5.4.3
RHEL-5 has a peculiar version of Xen, which requires some special casing. Add a configure option for that. It will be used in later commits. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- configure.in | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/configure.in b/configure.in index e870004..b65333e 100644 --- a/configure.in +++ b/configure.in @@ -229,6 +229,14 @@ fi AM_CONDITIONAL([LIBVIRT_INIT_SCRIPTS_RED_HAT], test x$with_init_scripts = xredhat) AC_MSG_RESULT($with_init_scripts) +dnl RHEL-5 has a peculiar version of Xen, which requires some special casing +AC_ARG_WITH([rhel5-api], + [AC_HELP_STRING([--with-rhel5-api=[ARG]], + [build for the RHEL-5 API [default=no]])]) +if test x"$with_rhel5_api" = x"yes"; then + AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API]) +fi + dnl dnl ensure that Fedora's system-config-firewall knows dnl about libvirt's iptables rules -- 1.5.4.3
On Tue, Dec 23, 2008 at 04:05:13PM +0100, Markus Armbruster wrote:
RHEL-5 has a peculiar version of Xen, which requires some special casing. Add a configure option for that. It will be used in later commits.
Ugh, shouldn't this be staying in a private Red Hat repository? regards john
On Tue, Dec 23, 2008 at 05:50:03PM +0000, John Levon wrote:
On Tue, Dec 23, 2008 at 04:05:13PM +0100, Markus Armbruster wrote:
RHEL-5 has a peculiar version of Xen, which requires some special casing. Add a configure option for that. It will be used in later commits.
Ugh, shouldn't this be staying in a private Red Hat repository?
This has been our approach up until now. It has turned out to cause more trouble than its solves though. Various people want to try out & user the newer libvirt on RHEL-5 hosts than the version we ship in RHEL-5. The current libvirt releases don't entirely work on RHEL-5 , due to fact that RHEL-5 Xen has backported a number of newer features. Adding this new --with-rhel5-api is a compromise - we target official Xen releases by default, but if you pass this flag at build time it'll enable the extra features backported to RHEL-5 Xen. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
John Levon <levon@movementarian.org> writes:
On Tue, Dec 23, 2008 at 04:05:13PM +0100, Markus Armbruster wrote:
RHEL-5 has a peculiar version of Xen, which requires some special casing. Add a configure option for that. It will be used in later commits.
Ugh, shouldn't this be staying in a private Red Hat repository?
I don't claim it's pretty. It affects only a few places, though, and I tried to make it as invisible as I could. I'm open to any ideas on how to solve the problem more cleanly. And the problem is real: people want to play with upstream libvirt on RHEL, but it doesn't work. Whether my cure is better or worse than the disease is open for debate, that's why I posted.
Omit (type ioemu) on RHEL-5, because it breaks PV drivers on HVM there. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xend_internal.c | 8 +++++++- src/xm_internal.c | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index 39a92ff..b502508 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -59,6 +59,12 @@ #endif /* PROXY */ +#ifdef WITH_RHEL5_API +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#else +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#endif + /** * xend_connection_type: * @@ -5155,7 +5161,7 @@ xenDaemonFormatSxprNet(virConnectPtr conn, * apparently (type ioemu) breaks paravirt drivers on HVM so skip this * from Xen 3.1.0 */ - if ((hvm) && (xendConfigVersion < 4)) + if (hvm && xendConfigVersion < XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) virBufferAddLit(buf, "(type ioemu)"); if (!isAttach) diff --git a/src/xm_internal.c b/src/xm_internal.c index 7cbfc7e..36334c4 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -48,6 +48,12 @@ #include "logging.h" +#ifdef WITH_RHEL5_API +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#else +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#endif + /* The true Xen limit varies but so far is always way less than 1024, which is the Linux kernel limit according to sched.h, so we'll match that for now */ @@ -1837,7 +1843,7 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn, goto cleanup; } - if (hvm && priv->xendConfigVersion < 4) + if (hvm && priv->xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) virBufferAddLit(&buf, ",type=ioemu"); if (net->model) -- 1.5.4.3
On Tue, Dec 23, 2008 at 04:05:14PM +0100, Markus Armbruster wrote:
Omit (type ioemu) on RHEL-5, because it breaks PV drivers on HVM there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
....
+#ifdef WITH_RHEL5_API +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#else +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#endif + /** * xend_connection_type: * @@ -5155,7 +5161,7 @@ xenDaemonFormatSxprNet(virConnectPtr conn, * apparently (type ioemu) breaks paravirt drivers on HVM so skip this * from Xen 3.1.0 */ - if ((hvm) && (xendConfigVersion < 4)) + if (hvm && xendConfigVersion < XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
For this to be a no-op if WITH_RHEL5_API is not defined, then I guess we need xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU
diff --git a/src/xm_internal.c b/src/xm_internal.c
like here:
- if (hvm && priv->xendConfigVersion < 4) + if (hvm && priv->xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
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/
Daniel Veillard <veillard@redhat.com> writes:
On Tue, Dec 23, 2008 at 04:05:14PM +0100, Markus Armbruster wrote:
Omit (type ioemu) on RHEL-5, because it breaks PV drivers on HVM there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
....
+#ifdef WITH_RHEL5_API +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#else +#define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#endif + /** * xend_connection_type: * @@ -5155,7 +5161,7 @@ xenDaemonFormatSxprNet(virConnectPtr conn, * apparently (type ioemu) breaks paravirt drivers on HVM so skip this * from Xen 3.1.0 */ - if ((hvm) && (xendConfigVersion < 4)) + if (hvm && xendConfigVersion < XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
For this to be a no-op if WITH_RHEL5_API is not defined, then I guess we need xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU
diff --git a/src/xm_internal.c b/src/xm_internal.c
like here:
- if (hvm && priv->xendConfigVersion < 4) + if (hvm && priv->xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU)
Daniel
Typo, good catch!
PVFB configuration depends on the version. The version number check doesn't work for RHEL-5 because its PVFB device was backported from later versions. Special-case it. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xend_internal.c | 6 ++++-- src/xm_internal.c | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index b502508..9b9f98d 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -61,8 +61,10 @@ #ifdef WITH_RHEL5_API #define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 2 #else #define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 3 #endif /** @@ -5406,7 +5408,7 @@ xenDaemonFormatSxpr(virConnectPtr conn, /* PV graphics for xen <= 3.0.4, or HVM graphics for xen <= 3.1.0 */ - if ((!hvm && xendConfigVersion < 3) || + if ((!hvm && xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF) || (hvm && xendConfigVersion < 4)) { if (def->graphics && xenDaemonFormatSxprGraphicsOld(conn, def->graphics, &buf, xendConfigVersion) < 0) @@ -5428,7 +5430,7 @@ xenDaemonFormatSxpr(virConnectPtr conn, /* New style PV graphics config xen >= 3.0.4, * or HVM graphics config xen >= 3.0.5 */ - if ((xendConfigVersion >= 3 && !hvm) || + if ((xendConfigVersion >= XEND_CONFIG_MIN_VERS_PVFB_NEWCONF && !hvm) || (xendConfigVersion >= 4 && hvm)) { if (def->graphics && xenDaemonFormatSxprGraphicsNew(conn, def->graphics, &buf) < 0) diff --git a/src/xm_internal.c b/src/xm_internal.c index 36334c4..c29f68f 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -50,8 +50,10 @@ #ifdef WITH_RHEL5_API #define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 0 +#define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 2 #else #define XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU 3 +#define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 3 #endif /* The true Xen limit varies but so far is always way @@ -2055,7 +2057,7 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, } if (def->graphics) { - if (priv->xendConfigVersion < (hvm ? 4 : 3)) { + if (priv->xendConfigVersion < (hvm ? 4 : XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { if (def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { if (xenXMConfigSetInt(conf, "sdl", 1) < 0) goto no_memory; -- 1.5.4.3
RHEL-5 supports NUMA since 5.2. Relax the version number checks. This breaks older versions, which will be fixed in the next commit. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xen_internal.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/xen_internal.c b/src/xen_internal.c index b3a531e..aada2e7 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -89,6 +89,12 @@ typedef privcmd_hypercall_t hypercall_t; #define __HYPERVISOR_domctl 36 #endif +#ifdef WITH_RHEL5_API +#define SYS_IFACE_MIN_VERS_NUMA 3 +#else +#define SYS_IFACE_MIN_VERS_NUMA 4 +#endif + static int xen_ioctl_hypercall_cmd = 0; static int initialized = 0; static int in_init = 0; @@ -2149,7 +2155,7 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, goto no_memory; - if (sys_interface_version >= 4) { + if (sys_interface_version >= SYS_IFACE_MIN_VERS_NUMA) { if (xenDaemonNodeGetTopology(conn, caps) != 0) { virCapabilitiesFree(caps); return NULL; @@ -2949,7 +2955,7 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free /* * Support only sys_interface_version >=4 */ - if (sys_interface_version < 4) { + if (sys_interface_version < SYS_IFACE_MIN_VERS_NUMA) { virXenErrorFunc (conn, VIR_ERR_XEN_CALL, __FUNCTION__, "unsupported in sys interface < 4", 0); return -1; -- 1.5.4.3
The check for missing topology information fails if and only if the system doesn't support NUMA. We reach the check only when sys_interface_version is high enough for NUMA. In other words, the check fails when sys_interface_version incorrectly indicates NUMA support, and it turns that condition from a recoverable oddity into a fatal error. The previous commit enabled NUMA for RHEL-5 regardless of version, and thus made old versions that don't support NUMA trip this check. Because the check looks quite worthless to me, I remove it instead of adding a special case for RHEL. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- src/xend_internal.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index 9b9f98d..096e22f 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -2546,6 +2546,8 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) * Internal routine populating capability info with * NUMA node mapping details * + * Does nothing when the system doesn't support NUMA (not an error). + * * Returns 0 in case of success, -1 in case of error */ static int @@ -2562,11 +2564,8 @@ sexpr_to_xend_topology(virConnectPtr conn, int numCpus; nodeToCpu = sexpr_node(root, "node/node_to_cpu"); - if (nodeToCpu == NULL) { - virXendError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to parse topology information")); - return -1; - } + if (nodeToCpu == NULL) + return 0; /* no NUMA support */ numCpus = sexpr_int(root, "node/nr_cpus"); -- 1.5.4.3
On Tue, Dec 23, 2008 at 04:04:21PM +0100, Markus Armbruster wrote:
This patch series attempts to make libvirt just work on RHEL-5. Right now it doesn't, mostly because libvirt relies on version number checks in a couple of places, and RHEL-5's version numbers aren't the whole truth due to various backports of later stuff.
Based on the earlier discussion, I think we should apply those patches:
The first two patches are plain bug fixes:
[PATCH 1/7] Fix network device inconsistency between xm and sxpr [PATCH 2/7] Fix graphics configuration inconsistency between xm and sxpr
because those are just plain bug fixes
The next three add a few special cases for RHEL-5:
[PATCH 3/7] New configure option --with-rhel5-api [PATCH 4/7] Fix HVM network device configuration for RHEL-5 [PATCH 5/7] Fix PVFB device configuration for RHEL-5 [PATCH 6/7] Enable NUMA support for RHEL-5
because they are really no-ops if one didn't configured --with-rhel5-api (assuming the slight change I pointed out in 4/7)
The last one drops a counter-productive error check:
[PATCH 7/7] Don't treat missing topology information as error
That's really a cleanup IMHO,
See the patches for more detailed descriptions.
I'm not proposing this for immediate commit, as I'm still testing. But I'd appreciate review: is this the right way to do it?
That looks fine to me, the only point is whether the configure option should be added, I guess it benefits more than just the RHEL users but also CentOS users so those should be commited. I plan to do this today but I just want to do a bit of testing on it first and check my 4/7 suggested change. 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 Mon, Jan 19, 2009 at 03:52:02PM +0100, Daniel Veillard wrote:
On Tue, Dec 23, 2008 at 04:04:21PM +0100, Markus Armbruster wrote:
This patch series attempts to make libvirt just work on RHEL-5. Right now it doesn't, mostly because libvirt relies on version number checks in a couple of places, and RHEL-5's version numbers aren't the whole truth due to various backports of later stuff.
Based on the earlier discussion, I think we should apply those patches:
Done, of course the regression tests now fail if the rhel5 specific configure option is turned on, I guess that's normal, maybe the SEXP<->XML conversion tests need to be switched off in that case, or we should generate alternate version when running in that case. 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 Mon, Jan 19, 2009 at 09:20:57PM +0100, Daniel Veillard wrote:
On Mon, Jan 19, 2009 at 03:52:02PM +0100, Daniel Veillard wrote:
On Tue, Dec 23, 2008 at 04:04:21PM +0100, Markus Armbruster wrote:
This patch series attempts to make libvirt just work on RHEL-5. Right now it doesn't, mostly because libvirt relies on version number checks in a couple of places, and RHEL-5's version numbers aren't the whole truth due to various backports of later stuff.
Based on the earlier discussion, I think we should apply those patches:
Done, of course the regression tests now fail if the rhel5 specific configure option is turned on, I guess that's normal, maybe the SEXP<->XML conversion tests need to be switched off in that case, or we should generate alternate version when running in that case.
I'll take a look - IIRC it should be reasonably easy for me to make the tests work even when the rhel5-xen option is turned on. We definitely want to validate that this option is actually doing what we intend, so prefer not to totally disable the tests in question Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
John Levon -
Markus Armbruster