This change make libvirt XML with plain <cpu> element invalid for libxl,
which affect not only upcoming CPUID support, but also NUMA. In fact,
default mode 'custom' does not match what the driver actually does, so
it was a bug. Adjust xenconfig driver accordingly.
But nevertheless this commit break some configurations that were working
before.
---
Changes since v3:
- fix vnuma tests (nested HVM implicitly enabled there)
Changes since v2:
- change separated from 'libxl: add support for CPUID features policy'
---
src/libxl/libxl_conf.c | 10 ++++++++--
src/xenconfig/xen_xl.c | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +-
tests/xlconfigdata/test-fullvirt-vnuma.cfg | 1 +-
tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +-
10 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8cced29..66956a7 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -355,11 +355,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
def->features[VIR_DOMAIN_FEATURE_ACPI] ==
VIR_TRISTATE_SWITCH_ON);
- if (caps &&
- def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ if (caps && def->cpu) {
bool hasHwVirt = false;
bool svm = false, vmx = false;
+ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu mode '%s'"),
+ virCPUModeTypeToString(def->cpu->mode));
+ return -1;
+ }
+
if (ARCH_IS_X86(def->os.arch)) {
vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm");
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 2ef80eb..6cda305 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -494,6 +494,7 @@ xenParseXLVnuma(virConfPtr conf,
goto cleanup;
}
+ cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
cpu->type = VIR_CPU_TYPE_GUEST;
def->cpu = cpu;
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
index edba69a..2c9de44 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg
@@ -22,5 +22,6 @@ parallel = "none"
serial = "none"
builder = "hvm"
boot = "d"
+nestedhvm = 1
vnuma = [ [ "pnode=0", "size=2048", "vcpus=0,11", "vdistances=10,21,31,41,51,61" ], [ "pnode=1", "size=2048", "vcpus=1,10", "vdistances=21,10,21,31,41,51" ], [ "pnode=2", "size=2048", "vcpus=2,9", "vdistances=31,21,10,21,31,41" ], [ "pnode=3", "size=2048", "vcpus=3,8", "vdistances=41,31,21,10,21,31" ], [ "pnode=4", "size=2048", "vcpus=4,7", "vdistances=51,41,31,21,10,21" ], [ "pnode=5", "size=2048", "vcpus=5-6", "vdistances=61,51,41,31,21,10" ] ]
disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
index e3639eb..3c486ad 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0,11' memory='2097152' unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
index 8186edf..ca8e5b0 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg
@@ -22,5 +22,6 @@ parallel = "none"
serial = "none"
builder = "hvm"
boot = "d"
+nestedhvm = 1
vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,20,20,20" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=20,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=20,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=20,20,20,10" ] ]
disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
index 9cab3ca..17c9ca5 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152' unit='KiB'/>
<cell id='1' cpus='2-3' memory='2097152' unit='KiB'/>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
index 861a50e..46d5f90 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg
@@ -22,5 +22,6 @@ parallel = "none"
serial = "none"
builder = "hvm"
boot = "d"
+nestedhvm = 1
vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,20,20,10" ] ]
disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
index 084b889..291fc37 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
<distances>
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.cfg b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
index 91e233a..813ad7f 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.cfg
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.cfg
@@ -22,5 +22,6 @@ parallel = "none"
serial = "none"
builder = "hvm"
boot = "d"
+nestedhvm = 1
vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,21,31" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,21,10,21" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,31,21,10" ] ]
disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml b/tests/xlconfigdata/test-fullvirt-vnuma.xml
index 5368b0d..9a9f495 100644
--- a/tests/xlconfigdata/test-fullvirt-vnuma.xml
+++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml
@@ -14,7 +14,7 @@
<apic/>
<pae/>
</features>
- <cpu>
+ <cpu mode='host-passthrough'>
<numa>
<cell id='0' cpus='0-1' memory='2097152' unit='KiB'>
<distances>
--
git-series 0.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote: > This change make libvirt XML with plain <cpu> element invalid for libxl, > which affect not only upcoming CPUID support, but also NUMA. In fact, > default mode 'custom' does not match what the driver actually does, so > it was a bug. Adjust xenconfig driver accordingly. > > But nevertheless this commit break some configurations that were working > before. > > --- > Changes since v3: > - fix vnuma tests (nested HVM implicitly enabled there) > Changes since v2: > - change separated from 'libxl: add support for CPUID features policy' > --- > src/libxl/libxl_conf.c | 10 ++++++++-- > src/xenconfig/xen_xl.c | 1 +- > tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg | 1 +- > tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +- > tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg | 1 +- > tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +- > tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg | 1 +- > tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +- > tests/xlconfigdata/test-fullvirt-vnuma.cfg | 1 +- > tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +- > 10 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 8cced29..66956a7 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -355,11 +355,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > def->features[VIR_DOMAIN_FEATURE_ACPI] == > VIR_TRISTATE_SWITCH_ON); > > - if (caps && > - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { > + if (caps && def->cpu) { > bool hasHwVirt = false; > bool svm = false, vmx = false; > > + if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unsupported cpu mode '%s'"), > + virCPUModeTypeToString(def->cpu->mode)); > + return -1; > + } It looks like we never answered my question from V3, i.e. should we change the default mode in the post-parse callback if NUMA or CPU features are defined within <cpu>? It sure feels like such configuration tweaks and error checks should be handled in the parsing phase, similar to qemuDomainDefVcpusPostParse() and qemuDomainDefCPUPostParse() in the qemu driver. I think danpb is vaguely familiar with this series and can help answer that question. Daniel, do you have an opinion? Or a general comment about guidelines for checking/tweaking config in parse phase vs domain start phase? Regards, Jim > + > if (ARCH_IS_X86(def->os.arch)) { > vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx"); > svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm"); > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > index 2ef80eb..6cda305 100644 > --- a/src/xenconfig/xen_xl.c > +++ b/src/xenconfig/xen_xl.c > @@ -494,6 +494,7 @@ xenParseXLVnuma(virConfPtr conf, > goto cleanup; > } > > + cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH; > cpu->type = VIR_CPU_TYPE_GUEST; > def->cpu = cpu; > > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg > index edba69a..2c9de44 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg > @@ -22,5 +22,6 @@ parallel = "none" > serial = "none" > builder = "hvm" > boot = "d" > +nestedhvm = 1 > vnuma = [ [ "pnode=0", "size=2048", "vcpus=0,11", "vdistances=10,21,31,41,51,61" ], [ "pnode=1", "size=2048", "vcpus=1,10", "vdistances=21,10,21,31,41,51" ], [ "pnode=2", "size=2048", "vcpus=2,9", "vdistances=31,21,10,21,31,41" ], [ "pnode=3", "size=2048", "vcpus=3,8", "vdistances=41,31,21,10,21,31" ], [ "pnode=4", "size=2048", "vcpus=4,7", "vdistances=51,41,31,21,10,21" ], [ "pnode=5", "size=2048", "vcpus=5-6", "vdistances=61,51,41,31,21,10" ] ] > disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml > index e3639eb..3c486ad 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml > @@ -14,7 +14,7 @@ > <apic/> > <pae/> > </features> > - <cpu> > + <cpu mode='host-passthrough'> > <numa> > <cell id='0' cpus='0,11' memory='2097152' unit='KiB'> > <distances> > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg > index 8186edf..ca8e5b0 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg > @@ -22,5 +22,6 @@ parallel = "none" > serial = "none" > builder = "hvm" > boot = "d" > +nestedhvm = 1 > vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,20,20,20" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=20,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=20,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=20,20,20,10" ] ] > disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml > index 9cab3ca..17c9ca5 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml > @@ -14,7 +14,7 @@ > <apic/> > <pae/> > </features> > - <cpu> > + <cpu mode='host-passthrough'> > <numa> > <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/> > <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/> > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg > index 861a50e..46d5f90 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg > @@ -22,5 +22,6 @@ parallel = "none" > serial = "none" > builder = "hvm" > boot = "d" > +nestedhvm = 1 > vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,20,20,10" ] ] > disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml > index 084b889..291fc37 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml > +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml > @@ -14,7 +14,7 @@ > <apic/> > <pae/> > </features> > - <cpu> > + <cpu mode='host-passthrough'> > <numa> > <cell id='0' cpus='0-1' memory='2097152' unit='KiB'> > <distances> > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.cfg b/tests/xlconfigdata/test-fullvirt-vnuma.cfg > index 91e233a..813ad7f 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma.cfg > +++ b/tests/xlconfigdata/test-fullvirt-vnuma.cfg > @@ -22,5 +22,6 @@ parallel = "none" > serial = "none" > builder = "hvm" > boot = "d" > +nestedhvm = 1 > vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,21,31" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,21,10,21" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,31,21,10" ] ] > disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] > diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml b/tests/xlconfigdata/test-fullvirt-vnuma.xml > index 5368b0d..9a9f495 100644 > --- a/tests/xlconfigdata/test-fullvirt-vnuma.xml > +++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml > @@ -14,7 +14,7 @@ > <apic/> > <pae/> > </features> > - <cpu> > + <cpu mode='host-passthrough'> > <numa> > <cell id='0' cpus='0-1' memory='2097152' unit='KiB'> > <distances> > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote: > On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote: > > This change make libvirt XML with plain <cpu> element invalid for libxl, > > which affect not only upcoming CPUID support, but also NUMA. In fact, > > default mode 'custom' does not match what the driver actually does, so > > it was a bug. Adjust xenconfig driver accordingly. > > > > But nevertheless this commit break some configurations that were working > > before. > > > > --- > > Changes since v3: > > - fix vnuma tests (nested HVM implicitly enabled there) > > Changes since v2: > > - change separated from 'libxl: add support for CPUID features policy' > > --- > > src/libxl/libxl_conf.c | 10 ++++++++-- > > src/xenconfig/xen_xl.c | 1 +- > > tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg | 1 +- > > tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +- > > tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg | 1 +- > > tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +- > > tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg | 1 +- > > tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +- > > tests/xlconfigdata/test-fullvirt-vnuma.cfg | 1 +- > > tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +- > > 10 files changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > > index 8cced29..66956a7 100644 > > --- a/src/libxl/libxl_conf.c > > +++ b/src/libxl/libxl_conf.c > > @@ -355,11 +355,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > def->features[VIR_DOMAIN_FEATURE_ACPI] == > > VIR_TRISTATE_SWITCH_ON); > > - if (caps && > > - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { > > + if (caps && def->cpu) { > > bool hasHwVirt = false; > > bool svm = false, vmx = false; > > + if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > + _("unsupported cpu mode '%s'"), > > + virCPUModeTypeToString(def->cpu->mode)); > > + return -1; > > + } > > It looks like we never answered my question from V3, i.e. should we change > the default mode in the post-parse callback if NUMA or CPU features are > defined within <cpu>? Hmm, but this means changing the config behind user's back, no? You have disabled nested HVM, upgrade, now you have enabled. Global switch is some consolation here, but is it enough? > It sure feels like such configuration tweaks and error checks should be > handled in the parsing phase, similar to qemuDomainDefVcpusPostParse() and > qemuDomainDefCPUPostParse() in the qemu driver. I think danpb is vaguely > familiar with this series and can help answer that question. Daniel, do you > have an opinion? Or a general comment about guidelines for checking/tweaking > config in parse phase vs domain start phase? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/15/2018 02:47 PM, Marek Marczykowski-Górecki wrote: > On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote: >> On 02/08/2018 03:58 PM, Marek Marczykowski-Górecki wrote: >>> This change make libvirt XML with plain <cpu> element invalid for libxl, >>> which affect not only upcoming CPUID support, but also NUMA. In fact, >>> default mode 'custom' does not match what the driver actually does, so >>> it was a bug. Adjust xenconfig driver accordingly. >>> >>> But nevertheless this commit break some configurations that were working >>> before. >>> >>> --- >>> Changes since v3: >>> - fix vnuma tests (nested HVM implicitly enabled there) >>> Changes since v2: >>> - change separated from 'libxl: add support for CPUID features policy' >>> --- >>> src/libxl/libxl_conf.c | 10 ++++++++-- >>> src/xenconfig/xen_xl.c | 1 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg | 1 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml | 2 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg | 1 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml | 2 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg | 1 +- >>> tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml | 2 +- >>> tests/xlconfigdata/test-fullvirt-vnuma.cfg | 1 +- >>> tests/xlconfigdata/test-fullvirt-vnuma.xml | 2 +- >>> 10 files changed, 17 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >>> index 8cced29..66956a7 100644 >>> --- a/src/libxl/libxl_conf.c >>> +++ b/src/libxl/libxl_conf.c >>> @@ -355,11 +355,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >>> def->features[VIR_DOMAIN_FEATURE_ACPI] == >>> VIR_TRISTATE_SWITCH_ON); >>> - if (caps && >>> - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { >>> + if (caps && def->cpu) { >>> bool hasHwVirt = false; >>> bool svm = false, vmx = false; >>> + if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { >>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >>> + _("unsupported cpu mode '%s'"), >>> + virCPUModeTypeToString(def->cpu->mode)); >>> + return -1; >>> + } >> >> It looks like we never answered my question from V3, i.e. should we change >> the default mode in the post-parse callback if NUMA or CPU features are >> defined within <cpu>? > > Hmm, but this means changing the config behind user's back, no? Well, sort of. But it is really just making the implicit explicit. > You have disabled nested HVM, upgrade, now you have enabled. > Global switch is some consolation here, but is it enough? I _think_ so. With a global default that disables nesting, are there any scenarios you envision where a previously disabled nesting becomes enabled after upgrade? BTW, I'm fine with this patch, just playing devil's advocate with handling such things post-parse vs domain start. It's not the only place where default xen config is not reflected in libvirt :-/. Regards, Jim > >> It sure feels like such configuration tweaks and error checks should be >> handled in the parsing phase, similar to qemuDomainDefVcpusPostParse() and >> qemuDomainDefCPUPostParse() in the qemu driver. I think danpb is vaguely >> familiar with this series and can help answer that question. Daniel, do you >> have an opinion? Or a general comment about guidelines for checking/tweaking >> config in parse phase vs domain start phase? > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 26, 2018 at 01:20:49PM -0700, Jim Fehlig wrote: > On 02/15/2018 02:47 PM, Marek Marczykowski-Górecki wrote: > > On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote: > > > It looks like we never answered my question from V3, i.e. should we change > > > the default mode in the post-parse callback if NUMA or CPU features are > > > defined within <cpu>? > > > > Hmm, but this means changing the config behind user's back, no? > > Well, sort of. But it is really just making the implicit explicit. > > > You have disabled nested HVM, upgrade, now you have enabled. > > Global switch is some consolation here, but is it enough? > > I _think_ so. With a global default that disables nesting, are there any > scenarios you envision where a previously disabled nesting becomes enabled > after upgrade? Probably also importing VM from older libvirt. Not sure about migration? Any other opinion about this? Daniel? Since introduction of global switch I'm fine with either. > BTW, I'm fine with this patch, just playing devil's advocate with handling > such things post-parse vs domain start. It's not the only place where > default xen config is not reflected in libvirt :-/. > > Regards, > Jim > > > > > > It sure feels like such configuration tweaks and error checks should be > > > handled in the parsing phase, similar to qemuDomainDefVcpusPostParse() and > > > qemuDomainDefCPUPostParse() in the qemu driver. I think danpb is vaguely > > > familiar with this series and can help answer that question. Daniel, do you > > > have an opinion? Or a general comment about guidelines for checking/tweaking > > > config in parse phase vs domain start phase? > > > -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2018 01:48 PM, Marek Marczykowski-Górecki wrote: > On Mon, Feb 26, 2018 at 01:20:49PM -0700, Jim Fehlig wrote: >> On 02/15/2018 02:47 PM, Marek Marczykowski-Górecki wrote: >>> On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote: >>>> It looks like we never answered my question from V3, i.e. should we change >>>> the default mode in the post-parse callback if NUMA or CPU features are >>>> defined within <cpu>? >>> >>> Hmm, but this means changing the config behind user's back, no? >> >> Well, sort of. But it is really just making the implicit explicit. >> >>> You have disabled nested HVM, upgrade, now you have enabled. >>> Global switch is some consolation here, but is it enough? >> >> I _think_ so. With a global default that disables nesting, are there any >> scenarios you envision where a previously disabled nesting becomes enabled >> after upgrade? > > Probably also importing VM from older libvirt. Not sure about migration? If importing from an older libvirt, the newer libvirt would have nesting disabled by default. I don't see a problem in that case. The same should be true when migrating from older to new libvirt. The only interesting case is migrating from libvirt containing this work to one without it. And then, only when the config contains <cpu mode='host-passthrough'> and svm|vmx are not disabled. Support for settings under <cpu> are quite new to the libxl driver, so in practice encountering that case is unlikely. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 26, 2018 at 09:48:14PM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Feb 26, 2018 at 01:20:49PM -0700, Jim Fehlig wrote: > > On 02/15/2018 02:47 PM, Marek Marczykowski-Górecki wrote: > > > On Tue, Feb 13, 2018 at 09:02:35AM -0700, Jim Fehlig wrote: > > > > It looks like we never answered my question from V3, i.e. should we change > > > > the default mode in the post-parse callback if NUMA or CPU features are > > > > defined within <cpu>? > > > > > > Hmm, but this means changing the config behind user's back, no? > > > > Well, sort of. But it is really just making the implicit explicit. > > > > > You have disabled nested HVM, upgrade, now you have enabled. > > > Global switch is some consolation here, but is it enough? > > > > I _think_ so. With a global default that disables nesting, are there any > > scenarios you envision where a previously disabled nesting becomes enabled > > after upgrade? > > Probably also importing VM from older libvirt. Not sure about migration? > > Any other opinion about this? Daniel? Since introduction of global > switch I'm fine with either. As a general rule in libvirt we aim to preserve compatibility such that upgrading either libvirt or the hypervisor stack will not change guest visible ABI. This ensures that migration from old to new libvirt and/or old to new hypervisor will succeed without breaking guests. The caveat is that for QEMU driver this is only guaranteed if QEMU is using a versioned machine type - which means x86, aarch64 and ppc only at this time. If previous versions of the libxl driver defaulted to having svm/vmx enabled by default, and enw versions of libvirt libxl defualt to disabled, that would be a potentially guest visible change. I think you can probably argue that this is none the less OK, because there is a global config parameter to change the default behaviour to regain strict compatibilty Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.