[libvirt] [PATCH] libxl: always enable pae for x86_64 HVM

For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1; + /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } + return 0; } -- 2.9.2

On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/> I would check that it's not VIR_TRISTATE_SWITCH_OFF, then enable it and during startup just fail if it is VIR_TRISTATE_SWITCH_ON.
return 0; }
-- 2.9.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
I would check that it's not VIR_TRISTATE_SWITCH_OFF, then enable it and during startup just fail if it is VIR_TRISTATE_SWITCH_ON.
I suppose you mean fail if it is *not* VIR_TRISTATE_SWITCH_ON? Regards, Jim
return 0; }
-- 2.9.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
Heh, the schema is wrong then, since the code treats all features the same way, as tri-states :-) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

On Thu, Jan 12, 2017 at 02:34:37PM +0000, Daniel P. Berrange wrote:
On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
Heh, the schema is wrong then, since the code treats all features the same way, as tri-states :-)
It does not, pae is explicitly just: <element name="pae"> <empty/> </element> Martin
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jan 12, 2017 at 03:51:43PM +0100, Martin Kletzander wrote:
On Thu, Jan 12, 2017 at 02:34:37PM +0000, Daniel P. Berrange wrote:
On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
Heh, the schema is wrong then, since the code treats all features the same way, as tri-states :-)
It does not, pae is explicitly just:
<element name="pae"> <empty/> </element>
I've hit 'send' too soon. It does not, we just use virSwitchTristate for saving the values, but it parses and formats them separately.
Martin
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 01/12/2017 07:34 AM, Daniel P. Berrange wrote:
On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
Heh, the schema is wrong then, since the code treats all features the same way, as tri-states :-)
src/conf/domain_conf.c treats them differently. E.g. see the features switch statement in the parse function http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/conf/domain_conf.c;h=52ae... # cat test.xml | grep pae <pae state='off'/> # virt-xml-validate test.xml Relax-NG validity error : Extra element features in interleave test.xml:19: element features: Relax-NG validity error : Element domain failed to validate content test.xml fails to validate # virsh define test.xml Domain test-hvm defined from test.xml # virsh dumpxml test-hvm | grep pae <pae/> Regards, Jim

On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
I saw that, but I somehow thought it changed since, I haven't realized it's less than a week ago.
I would check that it's not VIR_TRISTATE_SWITCH_OFF, then enable it and during startup just fail if it is VIR_TRISTATE_SWITCH_ON.
I suppose you mean fail if it is *not* VIR_TRISTATE_SWITCH_ON?
No, we add features by default normally, I just thought it's a tristate. Since it doesn't, this patch makes sense as it is. ACK, safe for freeze if you want to.
Regards, Jim
return 0; }
-- 2.9.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 01/12/2017 07:50 AM, Martin Kletzander wrote:
On Thu, Jan 12, 2017 at 07:31:29AM -0700, Jim Fehlig wrote:
On 01/12/2017 02:06 AM, Martin Kletzander wrote:
On Wed, Jan 11, 2017 at 05:45:27PM -0700, Jim Fehlig wrote:
For HVM domains, pae is only set in libxl_domain_build_info when explicitly specified in the hypervisor <features> config. This is fine for i686 machines, but is incorrect behavior for x86_64 machines where pae must always be enabled. See the following discussion for additional details
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index fbe7ee5..a5314b0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -410,6 +410,12 @@ libxlDomainDefPostParse(virDomainDefPtr def, if (xenDomainDefAddImplicitInputDevice(def) < 0) return -1;
+ /* For x86_64 HVM, always enable pae */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && + def->os.arch == VIR_ARCH_X86_64) { + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; + } +
This will even rewrite explicit <pae enabled='no/>
According to docs/schemas/domaincommon.rng, pae is not a tristate :-). One of the options for solving this problem is to make it one
https://www.redhat.com/archives/libvir-list/2017-January/msg00254.html
I saw that, but I somehow thought it changed since, I haven't realized it's less than a week ago.
I would check that it's not VIR_TRISTATE_SWITCH_OFF, then enable it and during startup just fail if it is VIR_TRISTATE_SWITCH_ON.
I suppose you mean fail if it is *not* VIR_TRISTATE_SWITCH_ON?
No, we add features by default normally, I just thought it's a tristate. Since it doesn't, this patch makes sense as it is.
ACK, safe for freeze if you want to.
Thanks. Since I think the bug fix is noteworthy, I squashed in the below news entry and pushed the patch. Regards, Jim --- a/docs/news.xml +++ b/docs/news.xml @@ -214,6 +214,17 @@ default to the general working scenario. </description> </change> + <change> + <summary> + libxl: always enable pae for x86_64 HVM + </summary> + <description> + By default pae is disabled in libxl. Without an explicit <pae/> + setting in the domain <features> configuration, an x86_64 HVM + domain would be get an i686 environment. pae should always be enabled + for x86_64 HVM domains. + </description> + </change>
participants (3)
-
Daniel P. Berrange
-
Jim Fehlig
-
Martin Kletzander