Since this is something between PV and HVM, it makes sense to put the
setting in place where domain type is specified.
To enable it, use <os><type machine="xenpvh">...</type></os>. It is
also included in capabilities.xml, for every supported HVM guest type - it
doesn't seems to be any other requirement (besides new enough Xen).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
docs/formatcaps.html.in | 4 +--
docs/schemas/domaincommon.rng | 1 +-
src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++--
src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++-----
src/libxl/libxl_driver.c | 6 +++--
5 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 34a74a9..1f17aa9 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -104,8 +104,8 @@
<dt><code>machine</code></dt><dd>Machine type, for use in
<a href="formatdomain.html#attributeOSTypeMachine">machine</a>
attribute of os/type element in domain XML. For example Xen
- supports <code>xenfv</code> for HVM or <code>xenpv</code> for
- PV.</dd>
+ supports <code>xenfv</code> for HVM, <code>xenpv</code> for
+ PV, or <code>xenpvh</code> for PVHv2.</dd>
<dt><code>domain</code></dt><dd>Supported domain type, named by the
<code>type</code> attribute.</dd>
</dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index eded1ca..d32b0cb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -341,6 +341,7 @@
<choice>
<value>xenpv</value>
<value>xenfv</value>
+ <value>xenpvh</value>
</choice>
</attribute>
</optional>
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 18596c7..e7b26f1 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -57,6 +57,7 @@ struct guest_arch {
virArch arch;
int bits;
int hvm;
+ int pvh;
int pae;
int nonpae;
int ia64_be;
@@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
guest_archs[i].nonpae = nonpae;
if (ia64_be)
guest_archs[i].ia64_be = ia64_be;
+
+ /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */
+ if ((ver_info->xen_version_major > 4 ||
+ (ver_info->xen_version_major == 4 &&
+ ver_info->xen_version_minor >= 9)) &&
+ hvm && i == nr_guest_archs-1) {
+ i = nr_guest_archs;
+ /* Too many arch flavours - highly unlikely ! */
+ if (i >= ARRAY_CARDINALITY(guest_archs))
+ continue;
+ nr_guest_archs++;
+ guest_archs[i].arch = arch;
+ guest_archs[i].pvh = 1;
+ }
}
}
regfree(®ex);
for (i = 0; i < nr_guest_archs; ++i) {
virCapsGuestPtr guest;
- char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"};
+ char const *const xen_machines[] = {
+ guest_archs[i].hvm ? "xenfv" :
+ (guest_archs[i].pvh ? "xenpvh" : "xenpv")};
virCapsGuestMachinePtr *machines;
if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL)
@@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
1,
0) == NULL)
return -1;
+ }
+ if (guest_archs[i].hvm || guest_archs[i].pvh) {
if (virCapabilitiesAddGuestFeature(guest,
"hap",
1,
@@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine,
os->supported = true;
- if (STREQ(machine, "xenpv"))
+ if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh"))
return 0;
capsLoader->supported = true;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f3da0ed..2df40ec 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
libxl_domain_create_info_init(c_info);
- if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
- c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+ if (def->os.type == VIR_DOMAIN_OSTYPE_HVM ||
+ (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
+ STREQ(def->os.machine, "xenpvh"))) {
+ c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ?
+ LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH;
switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) {
case VIR_TRISTATE_SWITCH_OFF:
libxl_defbool_set(&c_info->hap, false);
@@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
virDomainClockDef clock = def->clock;
libxl_ctx *ctx = cfg->ctx;
libxl_domain_build_info *b_info = &d_config->b_info;
- int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
+ bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
+ bool pvh = STREQ(def->os.machine, "xenpvh");
size_t i;
size_t nusbdevice = 0;
@@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
if (hvm)
libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM);
+ else if (pvh)
+ libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
else
libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
@@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024);
b_info->max_memkb = virDomainDefGetMemoryInitial(def);
b_info->target_memkb = def->mem.cur_balloon;
- if (hvm) {
+ if (hvm || pvh) {
if (caps &&
def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
bool hasHwVirt = false;
@@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
return -1;
}
#endif
+ } else if (pvh) {
+#ifdef LIBXL_HAVE_BUILDINFO_KERNEL
+ if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0)
+ return -1;
+ if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0)
+ return -1;
+ if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0)
+ return -1;
+#else
+ /*
+ * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen
+ * 4.5, but PVHv2 since 4.9.
+ */
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PVH guest type not supported"));
+#endif
+#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER
+ if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0)
+ return -1;
+ if (def->os.bootloaderArgs) {
+ if (!(b_info->bootloader_args =
+ virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
+ return -1;
+ }
+#endif
} else {
/*
* For compatibility with the legacy xen toolstack, default to pygrub
@@ -1230,7 +1261,7 @@ libxlMakeNic(virDomainDefPtr def,
STRNEQ(l_nic->model, "netfront")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("only model 'netfront' is supported for "
- "Xen PV domains"));
+ "Xen PV(H) domains"));
return -1;
}
if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 5a5e792..052a0da 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6271,9 +6271,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn,
emulatorbin = "/usr/bin/qemu-system-x86_64";
if (machine) {
- if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) {
+ if (STRNEQ(machine, "xenpv") &&
+ STRNEQ(machine, "xenpvh") &&
+ STRNEQ(machine, "xenfv")) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("Xen only supports 'xenpv' and 'xenfv' machines"));
+ _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines"));
goto cleanup;
}
} else {
--
git-series 0.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote: > Since this is something between PV and HVM, it makes sense to put the > setting in place where domain type is specified. > To enable it, use <os><type machine="xenpvh">...</type></os>. It is > also included in capabilities.xml, for every supported HVM guest type - it > doesn't seems to be any other requirement (besides new enough Xen). > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > docs/formatcaps.html.in | 4 +-- > docs/schemas/domaincommon.rng | 1 +- > src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++-- > src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++----- > src/libxl/libxl_driver.c | 6 +++-- > 5 files changed, 64 insertions(+), 11 deletions(-) > > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in > index 34a74a9..1f17aa9 100644 > --- a/docs/formatcaps.html.in > +++ b/docs/formatcaps.html.in > @@ -104,8 +104,8 @@ > <dt><code>machine</code></dt><dd>Machine type, for use in > <a href="formatdomain.html#attributeOSTypeMachine">machine</a> > attribute of os/type element in domain XML. For example Xen > - supports <code>xenfv</code> for HVM or <code>xenpv</code> for > - PV.</dd> > + supports <code>xenfv</code> for HVM, <code>xenpv</code> for > + PV, or <code>xenpvh</code> for PVHv2.</dd> > <dt><code>domain</code></dt><dd>Supported domain type, named by the > <code>type</code> attribute.</dd> > </dl> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index eded1ca..d32b0cb 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -341,6 +341,7 @@ > <choice> > <value>xenpv</value> > <value>xenfv</value> > + <value>xenpvh</value> > </choice> > </attribute> > </optional> > diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c > index 18596c7..e7b26f1 100644 > --- a/src/libxl/libxl_capabilities.c > +++ b/src/libxl/libxl_capabilities.c > @@ -57,6 +57,7 @@ struct guest_arch { > virArch arch; > int bits; > int hvm; > + int pvh; > int pae; > int nonpae; > int ia64_be; > @@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > guest_archs[i].nonpae = nonpae; > if (ia64_be) > guest_archs[i].ia64_be = ia64_be; > + > + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ I'm having problems understanding this. Do you mean add a PVH for each supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64 contains xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64 Given these caps, should a PVH be added that corresponds to the hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the hvm-3.0-x86_32p cap excluded? > + if ((ver_info->xen_version_major > 4 || > + (ver_info->xen_version_major == 4 && > + ver_info->xen_version_minor >= 9)) && > + hvm && i == nr_guest_archs-1) { > + i = nr_guest_archs; > + /* Too many arch flavours - highly unlikely ! */ > + if (i >= ARRAY_CARDINALITY(guest_archs)) > + continue; > + nr_guest_archs++; > + guest_archs[i].arch = arch; > + guest_archs[i].pvh = 1; > + } Without answers to the above questions, I can't really comment on this code. Regardless, since PVH is not advertised in xen_caps shouldn't it be added to guest_archs outside of the loop parsing xen_caps? > } > } > regfree(®ex); > > for (i = 0; i < nr_guest_archs; ++i) { > virCapsGuestPtr guest; > - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; > + char const *const xen_machines[] = { > + guest_archs[i].hvm ? "xenfv" : > + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; > virCapsGuestMachinePtr *machines; > > if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) > @@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > 1, > 0) == NULL) > return -1; > + } > > + if (guest_archs[i].hvm || guest_archs[i].pvh) { > if (virCapabilitiesAddGuestFeature(guest, > "hap", > 1, > @@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine, > > os->supported = true; > > - if (STREQ(machine, "xenpv")) > + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) > return 0; > > capsLoader->supported = true; > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index f3da0ed..2df40ec 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, > > libxl_domain_create_info_init(c_info); > > - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { > - c_info->type = LIBXL_DOMAIN_TYPE_HVM; > + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || > + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && > + STREQ(def->os.machine, "xenpvh"))) { > + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? > + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; > switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { > case VIR_TRISTATE_SWITCH_OFF: > libxl_defbool_set(&c_info->hap, false); > @@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > virDomainClockDef clock = def->clock; > libxl_ctx *ctx = cfg->ctx; > libxl_domain_build_info *b_info = &d_config->b_info; > - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > + bool pvh = STREQ(def->os.machine, "xenpvh"); > size_t i; > size_t nusbdevice = 0; > > @@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > if (hvm) > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); > + else if (pvh) > + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); > else > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); > > @@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); > b_info->max_memkb = virDomainDefGetMemoryInitial(def); > b_info->target_memkb = def->mem.cur_balloon; > - if (hvm) { > + if (hvm || pvh) { > if (caps && > def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { > bool hasHwVirt = false; > @@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > return -1; > } > #endif > + } else if (pvh) { > +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL > + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) > + return -1; > + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) > + return -1; > + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) > + return -1; > +#else > + /* > + * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen > + * 4.5, but PVHv2 since 4.9. > + */ > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("PVH guest type not supported")); > +#endif I guess this is needed else the build will fail on Xen < 4.5? Maybe it is time to bump the minimum supported Xen version to 4.6 :-). I say that a bit jokingly, but I did propose it a few months back. Regards, Jim > +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER > + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) > + return -1; > + if (def->os.bootloaderArgs) { > + if (!(b_info->bootloader_args = > + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) > + return -1; > + } > +#endif > } else { > /* > * For compatibility with the legacy xen toolstack, default to pygrub > @@ -1230,7 +1261,7 @@ libxlMakeNic(virDomainDefPtr def, > STRNEQ(l_nic->model, "netfront")) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("only model 'netfront' is supported for " > - "Xen PV domains")); > + "Xen PV(H) domains")); > return -1; > } > if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 5a5e792..052a0da 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -6271,9 +6271,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn, > emulatorbin = "/usr/bin/qemu-system-x86_64"; > > if (machine) { > - if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) { > + if (STRNEQ(machine, "xenpv") && > + STRNEQ(machine, "xenpvh") && > + STRNEQ(machine, "xenfv")) { > virReportError(VIR_ERR_INVALID_ARG, "%s", > - _("Xen only supports 'xenpv' and 'xenfv' machines")); > + _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines")); > goto cleanup; > } > } else { > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote: > On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote: > > Since this is something between PV and HVM, it makes sense to put the > > setting in place where domain type is specified. > > To enable it, use <os><type machine="xenpvh">...</type></os>. It is > > also included in capabilities.xml, for every supported HVM guest type - it > > doesn't seems to be any other requirement (besides new enough Xen). > > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > --- > > docs/formatcaps.html.in | 4 +-- > > docs/schemas/domaincommon.rng | 1 +- > > src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++-- > > src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++----- > > src/libxl/libxl_driver.c | 6 +++-- > > 5 files changed, 64 insertions(+), 11 deletions(-) > > > > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in > > index 34a74a9..1f17aa9 100644 > > --- a/docs/formatcaps.html.in > > +++ b/docs/formatcaps.html.in > > @@ -104,8 +104,8 @@ > > <dt><code>machine</code></dt><dd>Machine type, for use in > > <a href="formatdomain.html#attributeOSTypeMachine">machine</a> > > attribute of os/type element in domain XML. For example Xen > > - supports <code>xenfv</code> for HVM or <code>xenpv</code> for > > - PV.</dd> > > + supports <code>xenfv</code> for HVM, <code>xenpv</code> for > > + PV, or <code>xenpvh</code> for PVHv2.</dd> > > <dt><code>domain</code></dt><dd>Supported domain type, named by the > > <code>type</code> attribute.</dd> > > </dl> > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > > index eded1ca..d32b0cb 100644 > > --- a/docs/schemas/domaincommon.rng > > +++ b/docs/schemas/domaincommon.rng > > @@ -341,6 +341,7 @@ > > <choice> > > <value>xenpv</value> > > <value>xenfv</value> > > + <value>xenpvh</value> > > </choice> > > </attribute> > > </optional> > > diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c > > index 18596c7..e7b26f1 100644 > > --- a/src/libxl/libxl_capabilities.c > > +++ b/src/libxl/libxl_capabilities.c > > @@ -57,6 +57,7 @@ struct guest_arch { > > virArch arch; > > int bits; > > int hvm; > > + int pvh; > > int pae; > > int nonpae; > > int ia64_be; > > @@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > > guest_archs[i].nonpae = nonpae; > > if (ia64_be) > > guest_archs[i].ia64_be = ia64_be; > > + > > + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ > > I'm having problems understanding this. Do you mean add a PVH for each > supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64 > contains > > xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64 > > Given these caps, should a PVH be added that corresponds to the > hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the > hvm-3.0-x86_32p cap excluded? Yes, exactly. Setting PAE (or not) is possible only for HVM, but not PVH. It would be much better if Xen would report support for PVH explicitly... > > + if ((ver_info->xen_version_major > 4 || > > + (ver_info->xen_version_major == 4 && > > + ver_info->xen_version_minor >= 9)) && > > + hvm && i == nr_guest_archs-1) { > > + i = nr_guest_archs; > > + /* Too many arch flavours - highly unlikely ! */ > > + if (i >= ARRAY_CARDINALITY(guest_archs)) > > + continue; > > + nr_guest_archs++; > > + guest_archs[i].arch = arch; > > + guest_archs[i].pvh = 1; > > + } > > Without answers to the above questions, I can't really comment on this code. > Regardless, since PVH is not advertised in xen_caps shouldn't it be added to > guest_archs outside of the loop parsing xen_caps? This works on assumption that if you have HVM and new enough Xen, then you have PVH. Just having new Xen isn't enough - for example the hardware may lack VT-x. > > } > > } > > regfree(®ex); > > for (i = 0; i < nr_guest_archs; ++i) { > > virCapsGuestPtr guest; > > - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; > > + char const *const xen_machines[] = { > > + guest_archs[i].hvm ? "xenfv" : > > + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; > > virCapsGuestMachinePtr *machines; > > if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) > > @@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > > 1, > > 0) == NULL) > > return -1; > > + } > > + if (guest_archs[i].hvm || guest_archs[i].pvh) { > > if (virCapabilitiesAddGuestFeature(guest, > > "hap", > > 1, > > @@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine, > > os->supported = true; > > - if (STREQ(machine, "xenpv")) > > + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) > > return 0; > > capsLoader->supported = true; > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > > index f3da0ed..2df40ec 100644 > > --- a/src/libxl/libxl_conf.c > > +++ b/src/libxl/libxl_conf.c > > @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, > > libxl_domain_create_info_init(c_info); > > - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { > > - c_info->type = LIBXL_DOMAIN_TYPE_HVM; > > + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || > > + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && > > + STREQ(def->os.machine, "xenpvh"))) { > > + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? > > + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; > > switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { > > case VIR_TRISTATE_SWITCH_OFF: > > libxl_defbool_set(&c_info->hap, false); > > @@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > virDomainClockDef clock = def->clock; > > libxl_ctx *ctx = cfg->ctx; > > libxl_domain_build_info *b_info = &d_config->b_info; > > - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > > + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > > + bool pvh = STREQ(def->os.machine, "xenpvh"); > > size_t i; > > size_t nusbdevice = 0; > > @@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > if (hvm) > > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); > > + else if (pvh) > > + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); > > else > > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); > > @@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); > > b_info->max_memkb = virDomainDefGetMemoryInitial(def); > > b_info->target_memkb = def->mem.cur_balloon; > > - if (hvm) { > > + if (hvm || pvh) { > > if (caps && > > def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { > > bool hasHwVirt = false; > > @@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > return -1; > > } > > #endif > > + } else if (pvh) { > > +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL > > + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) > > + return -1; > > + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) > > + return -1; > > + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) > > + return -1; > > +#else > > + /* > > + * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen > > + * 4.5, but PVHv2 since 4.9. > > + */ > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > > + _("PVH guest type not supported")); > > +#endif > > I guess this is needed else the build will fail on Xen < 4.5? Yes, exactly. > Maybe it is > time to bump the minimum supported Xen version to 4.6 :-). I say that a bit > jokingly, but I did propose it a few months back. IMO good idea, since Xen < 4.6 is not supported anymore. > > +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER > > + if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0) > > + return -1; > > + if (def->os.bootloaderArgs) { > > + if (!(b_info->bootloader_args = > > + virStringSplit(def->os.bootloaderArgs, " \t\n", 0))) > > + return -1; > > + } > > +#endif > > } else { > > /* > > * For compatibility with the legacy xen toolstack, default to pygrub > > @@ -1230,7 +1261,7 @@ libxlMakeNic(virDomainDefPtr def, > > STRNEQ(l_nic->model, "netfront")) { > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > > _("only model 'netfront' is supported for " > > - "Xen PV domains")); > > + "Xen PV(H) domains")); > > return -1; > > } > > if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > > index 5a5e792..052a0da 100644 > > --- a/src/libxl/libxl_driver.c > > +++ b/src/libxl/libxl_driver.c > > @@ -6271,9 +6271,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn, > > emulatorbin = "/usr/bin/qemu-system-x86_64"; > > if (machine) { > > - if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) { > > + if (STRNEQ(machine, "xenpv") && > > + STRNEQ(machine, "xenpvh") && > > + STRNEQ(machine, "xenfv")) { > > virReportError(VIR_ERR_INVALID_ARG, "%s", > > - _("Xen only supports 'xenpv' and 'xenfv' machines")); > > + _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines")); > > goto cleanup; > > } > > } else { > > > -- 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 09/10/2018 04:02 PM, Marek Marczykowski-Górecki wrote: > On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote: >> On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote: >>> Since this is something between PV and HVM, it makes sense to put the >>> setting in place where domain type is specified. >>> To enable it, use <os><type machine="xenpvh">...</type></os>. It is >>> also included in capabilities.xml, for every supported HVM guest type - it >>> doesn't seems to be any other requirement (besides new enough Xen). >>> >>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >>> --- >>> docs/formatcaps.html.in | 4 +-- >>> docs/schemas/domaincommon.rng | 1 +- >>> src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++-- >>> src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++----- >>> src/libxl/libxl_driver.c | 6 +++-- >>> 5 files changed, 64 insertions(+), 11 deletions(-) >>> >>> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in >>> index 34a74a9..1f17aa9 100644 >>> --- a/docs/formatcaps.html.in >>> +++ b/docs/formatcaps.html.in >>> @@ -104,8 +104,8 @@ >>> <dt><code>machine</code></dt><dd>Machine type, for use in >>> <a href="formatdomain.html#attributeOSTypeMachine">machine</a> >>> attribute of os/type element in domain XML. For example Xen >>> - supports <code>xenfv</code> for HVM or <code>xenpv</code> for >>> - PV.</dd> >>> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for >>> + PV, or <code>xenpvh</code> for PVHv2.</dd> >>> <dt><code>domain</code></dt><dd>Supported domain type, named by the >>> <code>type</code> attribute.</dd> >>> </dl> >>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >>> index eded1ca..d32b0cb 100644 >>> --- a/docs/schemas/domaincommon.rng >>> +++ b/docs/schemas/domaincommon.rng >>> @@ -341,6 +341,7 @@ >>> <choice> >>> <value>xenpv</value> >>> <value>xenfv</value> >>> + <value>xenpvh</value> >>> </choice> >>> </attribute> >>> </optional> >>> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c >>> index 18596c7..e7b26f1 100644 >>> --- a/src/libxl/libxl_capabilities.c >>> +++ b/src/libxl/libxl_capabilities.c >>> @@ -57,6 +57,7 @@ struct guest_arch { >>> virArch arch; >>> int bits; >>> int hvm; >>> + int pvh; >>> int pae; >>> int nonpae; >>> int ia64_be; >>> @@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) >>> guest_archs[i].nonpae = nonpae; >>> if (ia64_be) >>> guest_archs[i].ia64_be = ia64_be; >>> + >>> + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ >> >> I'm having problems understanding this. Do you mean add a PVH for each >> supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64 >> contains >> >> xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64 >> >> Given these caps, should a PVH be added that corresponds to the >> hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the >> hvm-3.0-x86_32p cap excluded? > > Yes, exactly. Setting PAE (or not) is possible only for HVM, but not > PVH. > > It would be much better if Xen would report support for PVH > explicitly... > >>> + if ((ver_info->xen_version_major > 4 || >>> + (ver_info->xen_version_major == 4 && >>> + ver_info->xen_version_minor >= 9)) && >>> + hvm && i == nr_guest_archs-1) { How about checking for hvm && !pae instead of i == nr_guest_archs-1? >>> + i = nr_guest_archs; >>> + /* Too many arch flavours - highly unlikely ! */ >>> + if (i >= ARRAY_CARDINALITY(guest_archs)) >>> + continue; >>> + nr_guest_archs++; >>> + guest_archs[i].arch = arch; >>> + guest_archs[i].pvh = 1; >>> + } >> >> Without answers to the above questions, I can't really comment on this code. >> Regardless, since PVH is not advertised in xen_caps shouldn't it be added to >> guest_archs outside of the loop parsing xen_caps? > > This works on assumption that if you have HVM and new enough Xen, then > you have PVH. Just having new Xen isn't enough - for example the > hardware may lack VT-x. > >>> } >>> } >>> regfree(®ex); >>> for (i = 0; i < nr_guest_archs; ++i) { >>> virCapsGuestPtr guest; >>> - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; >>> + char const *const xen_machines[] = { >>> + guest_archs[i].hvm ? "xenfv" : >>> + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; >>> virCapsGuestMachinePtr *machines; >>> if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) >>> @@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) >>> 1, >>> 0) == NULL) >>> return -1; >>> + } >>> + if (guest_archs[i].hvm || guest_archs[i].pvh) { >>> if (virCapabilitiesAddGuestFeature(guest, >>> "hap", >>> 1, >>> @@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine, >>> os->supported = true; >>> - if (STREQ(machine, "xenpv")) >>> + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) >>> return 0; >>> capsLoader->supported = true; >>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >>> index f3da0ed..2df40ec 100644 >>> --- a/src/libxl/libxl_conf.c >>> +++ b/src/libxl/libxl_conf.c >>> @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, >>> libxl_domain_create_info_init(c_info); >>> - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { >>> - c_info->type = LIBXL_DOMAIN_TYPE_HVM; >>> + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || >>> + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && >>> + STREQ(def->os.machine, "xenpvh"))) { >>> + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? >>> + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; >>> switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { >>> case VIR_TRISTATE_SWITCH_OFF: >>> libxl_defbool_set(&c_info->hap, false); >>> @@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >>> virDomainClockDef clock = def->clock; >>> libxl_ctx *ctx = cfg->ctx; >>> libxl_domain_build_info *b_info = &d_config->b_info; >>> - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; >>> + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; >>> + bool pvh = STREQ(def->os.machine, "xenpvh"); >>> size_t i; >>> size_t nusbdevice = 0; >>> @@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >>> if (hvm) >>> libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); >>> + else if (pvh) >>> + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); >>> else >>> libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); >>> @@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >>> def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); >>> b_info->max_memkb = virDomainDefGetMemoryInitial(def); >>> b_info->target_memkb = def->mem.cur_balloon; >>> - if (hvm) { >>> + if (hvm || pvh) { >>> if (caps && >>> def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { >>> bool hasHwVirt = false; >>> @@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, >>> return -1; >>> } >>> #endif >>> + } else if (pvh) { >>> +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL >>> + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) >>> + return -1; >>> + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) >>> + return -1; >>> + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) >>> + return -1; >>> +#else >>> + /* >>> + * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen >>> + * 4.5, but PVHv2 since 4.9. >>> + */ >>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >>> + _("PVH guest type not supported")); >>> +#endif >> >> I guess this is needed else the build will fail on Xen < 4.5? > > Yes, exactly. > >> Maybe it is >> time to bump the minimum supported Xen version to 4.6 :-). I say that a bit >> jokingly, but I did propose it a few months back. > > IMO good idea, since Xen < 4.6 is not supported anymore. BTW, here is a link to my earlier series to drop support for 4.4 and 4.5 https://www.redhat.com/archives/libvir-list/2018-March/msg01704.html I'll rebase and resend that soon. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Sep 10, 2018 at 04:45:50PM -0600, Jim Fehlig wrote: > On 09/10/2018 04:02 PM, Marek Marczykowski-Górecki wrote: > > On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote: > > > On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote: > > > > Since this is something between PV and HVM, it makes sense to put the > > > > setting in place where domain type is specified. > > > > To enable it, use <os><type machine="xenpvh">...</type></os>. It is > > > > also included in capabilities.xml, for every supported HVM guest type - it > > > > doesn't seems to be any other requirement (besides new enough Xen). > > > > > > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > > > --- > > > > docs/formatcaps.html.in | 4 +-- > > > > docs/schemas/domaincommon.rng | 1 +- > > > > src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++-- > > > > src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++----- > > > > src/libxl/libxl_driver.c | 6 +++-- > > > > 5 files changed, 64 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in > > > > index 34a74a9..1f17aa9 100644 > > > > --- a/docs/formatcaps.html.in > > > > +++ b/docs/formatcaps.html.in > > > > @@ -104,8 +104,8 @@ > > > > <dt><code>machine</code></dt><dd>Machine type, for use in > > > > <a href="formatdomain.html#attributeOSTypeMachine">machine</a> > > > > attribute of os/type element in domain XML. For example Xen > > > > - supports <code>xenfv</code> for HVM or <code>xenpv</code> for > > > > - PV.</dd> > > > > + supports <code>xenfv</code> for HVM, <code>xenpv</code> for > > > > + PV, or <code>xenpvh</code> for PVHv2.</dd> > > > > <dt><code>domain</code></dt><dd>Supported domain type, named by the > > > > <code>type</code> attribute.</dd> > > > > </dl> > > > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > > > > index eded1ca..d32b0cb 100644 > > > > --- a/docs/schemas/domaincommon.rng > > > > +++ b/docs/schemas/domaincommon.rng > > > > @@ -341,6 +341,7 @@ > > > > <choice> > > > > <value>xenpv</value> > > > > <value>xenfv</value> > > > > + <value>xenpvh</value> > > > > </choice> > > > > </attribute> > > > > </optional> > > > > diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c > > > > index 18596c7..e7b26f1 100644 > > > > --- a/src/libxl/libxl_capabilities.c > > > > +++ b/src/libxl/libxl_capabilities.c > > > > @@ -57,6 +57,7 @@ struct guest_arch { > > > > virArch arch; > > > > int bits; > > > > int hvm; > > > > + int pvh; > > > > int pae; > > > > int nonpae; > > > > int ia64_be; > > > > @@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > > > > guest_archs[i].nonpae = nonpae; > > > > if (ia64_be) > > > > guest_archs[i].ia64_be = ia64_be; > > > > + > > > > + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ > > > > > > I'm having problems understanding this. Do you mean add a PVH for each > > > supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64 > > > contains > > > > > > xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64 > > > > > > Given these caps, should a PVH be added that corresponds to the > > > hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the > > > hvm-3.0-x86_32p cap excluded? > > > > Yes, exactly. Setting PAE (or not) is possible only for HVM, but not > > PVH. > > > > It would be much better if Xen would report support for PVH > > explicitly... > > > > > > + if ((ver_info->xen_version_major > 4 || > > > > + (ver_info->xen_version_major == 4 && > > > > + ver_info->xen_version_minor >= 9)) && > > > > + hvm && i == nr_guest_archs-1) { > > How about checking for hvm && !pae instead of i == nr_guest_archs-1? At this time it should be ok. The i == nr_guest_archs-1 is based directly on if (i == nr_guest_archs) nr_guest_archs++ earlier up. So, it is indeed added only when given arch was added to guest_archs, regardless of what conditions were used for that. Maybe, use something like this instead: bool new_arch_added; if ((new_arch_added = (i == nr_guest_archs))) nr_guest_archs++ ... if (... hvm && new_arch_added) ? > > > > > + i = nr_guest_archs; > > > > + /* Too many arch flavours - highly unlikely ! */ > > > > + if (i >= ARRAY_CARDINALITY(guest_archs)) > > > > + continue; > > > > + nr_guest_archs++; > > > > + guest_archs[i].arch = arch; > > > > + guest_archs[i].pvh = 1; > > > > + } > > > > > > Without answers to the above questions, I can't really comment on this code. > > > Regardless, since PVH is not advertised in xen_caps shouldn't it be added to > > > guest_archs outside of the loop parsing xen_caps? > > > > This works on assumption that if you have HVM and new enough Xen, then > > you have PVH. Just having new Xen isn't enough - for example the > > hardware may lack VT-x. > > > > > > } > > > > } > > > > regfree(®ex); > > > > for (i = 0; i < nr_guest_archs; ++i) { > > > > virCapsGuestPtr guest; > > > > - char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"}; > > > > + char const *const xen_machines[] = { > > > > + guest_archs[i].hvm ? "xenfv" : > > > > + (guest_archs[i].pvh ? "xenpvh" : "xenpv")}; > > > > virCapsGuestMachinePtr *machines; > > > > if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) > > > > @@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) > > > > 1, > > > > 0) == NULL) > > > > return -1; > > > > + } > > > > + if (guest_archs[i].hvm || guest_archs[i].pvh) { > > > > if (virCapabilitiesAddGuestFeature(guest, > > > > "hap", > > > > 1, > > > > @@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine, > > > > os->supported = true; > > > > - if (STREQ(machine, "xenpv")) > > > > + if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh")) > > > > return 0; > > > > capsLoader->supported = true; > > > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > > > > index f3da0ed..2df40ec 100644 > > > > --- a/src/libxl/libxl_conf.c > > > > +++ b/src/libxl/libxl_conf.c > > > > @@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, > > > > libxl_domain_create_info_init(c_info); > > > > - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { > > > > - c_info->type = LIBXL_DOMAIN_TYPE_HVM; > > > > + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM || > > > > + (def->os.type == VIR_DOMAIN_OSTYPE_XEN && > > > > + STREQ(def->os.machine, "xenpvh"))) { > > > > + c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ? > > > > + LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH; > > > > switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) { > > > > case VIR_TRISTATE_SWITCH_OFF: > > > > libxl_defbool_set(&c_info->hap, false); > > > > @@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > > > virDomainClockDef clock = def->clock; > > > > libxl_ctx *ctx = cfg->ctx; > > > > libxl_domain_build_info *b_info = &d_config->b_info; > > > > - int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > > > > + bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; > > > > + bool pvh = STREQ(def->os.machine, "xenpvh"); > > > > size_t i; > > > > size_t nusbdevice = 0; > > > > @@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > > > if (hvm) > > > > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); > > > > + else if (pvh) > > > > + libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); > > > > else > > > > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); > > > > @@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > > > def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024); > > > > b_info->max_memkb = virDomainDefGetMemoryInitial(def); > > > > b_info->target_memkb = def->mem.cur_balloon; > > > > - if (hvm) { > > > > + if (hvm || pvh) { > > > > if (caps && > > > > def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) { > > > > bool hasHwVirt = false; > > > > @@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > > > > return -1; > > > > } > > > > #endif > > > > + } else if (pvh) { > > > > +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL > > > > + if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0) > > > > + return -1; > > > > + if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0) > > > > + return -1; > > > > + if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0) > > > > + return -1; > > > > +#else > > > > + /* > > > > + * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen > > > > + * 4.5, but PVHv2 since 4.9. > > > > + */ > > > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > > > > + _("PVH guest type not supported")); > > > > +#endif > > > > > > I guess this is needed else the build will fail on Xen < 4.5? > > > > Yes, exactly. > > > > > Maybe it is > > > time to bump the minimum supported Xen version to 4.6 :-). I say that a bit > > > jokingly, but I did propose it a few months back. > > > > IMO good idea, since Xen < 4.6 is not supported anymore. > > BTW, here is a link to my earlier series to drop support for 4.4 and 4.5 > > https://www.redhat.com/archives/libvir-list/2018-March/msg01704.html > > I'll rebase and resend that soon. > > Regards, > Jim -- 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 9/10/18 7:10 PM, Marek Marczykowski-Górecki wrote: > On Mon, Sep 10, 2018 at 04:45:50PM -0600, Jim Fehlig wrote: >> On 09/10/2018 04:02 PM, Marek Marczykowski-Górecki wrote: >>> On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote: >>>> On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote: >>>>> Since this is something between PV and HVM, it makes sense to put the >>>>> setting in place where domain type is specified. >>>>> To enable it, use <os><type machine="xenpvh">...</type></os>. It is >>>>> also included in capabilities.xml, for every supported HVM guest type - it >>>>> doesn't seems to be any other requirement (besides new enough Xen). >>>>> >>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >>>>> --- >>>>> docs/formatcaps.html.in | 4 +-- >>>>> docs/schemas/domaincommon.rng | 1 +- >>>>> src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++-- >>>>> src/libxl/libxl_conf.c | 41 ++++++++++++++++++++++++++++++----- >>>>> src/libxl/libxl_driver.c | 6 +++-- >>>>> 5 files changed, 64 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in >>>>> index 34a74a9..1f17aa9 100644 >>>>> --- a/docs/formatcaps.html.in >>>>> +++ b/docs/formatcaps.html.in >>>>> @@ -104,8 +104,8 @@ >>>>> <dt><code>machine</code></dt><dd>Machine type, for use in >>>>> <a href="formatdomain.html#attributeOSTypeMachine">machine</a> >>>>> attribute of os/type element in domain XML. For example Xen >>>>> - supports <code>xenfv</code> for HVM or <code>xenpv</code> for >>>>> - PV.</dd> >>>>> + supports <code>xenfv</code> for HVM, <code>xenpv</code> for >>>>> + PV, or <code>xenpvh</code> for PVHv2.</dd> >>>>> <dt><code>domain</code></dt><dd>Supported domain type, named by the >>>>> <code>type</code> attribute.</dd> >>>>> </dl> >>>>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >>>>> index eded1ca..d32b0cb 100644 >>>>> --- a/docs/schemas/domaincommon.rng >>>>> +++ b/docs/schemas/domaincommon.rng >>>>> @@ -341,6 +341,7 @@ >>>>> <choice> >>>>> <value>xenpv</value> >>>>> <value>xenfv</value> >>>>> + <value>xenpvh</value> >>>>> </choice> >>>>> </attribute> >>>>> </optional> >>>>> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c >>>>> index 18596c7..e7b26f1 100644 >>>>> --- a/src/libxl/libxl_capabilities.c >>>>> +++ b/src/libxl/libxl_capabilities.c >>>>> @@ -57,6 +57,7 @@ struct guest_arch { >>>>> virArch arch; >>>>> int bits; >>>>> int hvm; >>>>> + int pvh; >>>>> int pae; >>>>> int nonpae; >>>>> int ia64_be; >>>>> @@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) >>>>> guest_archs[i].nonpae = nonpae; >>>>> if (ia64_be) >>>>> guest_archs[i].ia64_be = ia64_be; >>>>> + >>>>> + /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ >>>> >>>> I'm having problems understanding this. Do you mean add a PVH for each >>>> supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64 >>>> contains >>>> >>>> xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64 >>>> >>>> Given these caps, should a PVH be added that corresponds to the >>>> hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the >>>> hvm-3.0-x86_32p cap excluded? >>> >>> Yes, exactly. Setting PAE (or not) is possible only for HVM, but not >>> PVH. >>> >>> It would be much better if Xen would report support for PVH >>> explicitly... >>> >>>>> + if ((ver_info->xen_version_major > 4 || >>>>> + (ver_info->xen_version_major == 4 && >>>>> + ver_info->xen_version_minor >= 9)) && >>>>> + hvm && i == nr_guest_archs-1) { >> >> How about checking for hvm && !pae instead of i == nr_guest_archs-1? > > At this time it should be ok. The i == nr_guest_archs-1 is based > directly on > > if (i == nr_guest_archs) > nr_guest_archs++ > > earlier up. So, it is indeed added only when given arch was added to > guest_archs, regardless of what conditions were used for that. Right. This function is a little intense. I had to look at it a third time to see i == nr_guest_archs-1 also means we are adding a new entry in guest_archs :-). > Maybe, use something like this instead: > > > bool new_arch_added; > if ((new_arch_added = (i == nr_guest_archs))) > nr_guest_archs++ > ... > if (... hvm && new_arch_added) Along with an improved comment, I think this will help me remember what's going on in this code in the event I ever find time to refactor it. What do you think of the below diff? Regards, Jim diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index e7b26f12d8..446fa4ccf9 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -440,6 +440,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; + bool new_arch_added = false; if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { arch = VIR_ARCH_I686; @@ -476,8 +477,10 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (i >= ARRAY_CARDINALITY(guest_archs)) continue; /* Didn't find a match, so create a new one */ - if (i == nr_guest_archs) + if (i == nr_guest_archs) { nr_guest_archs++; + new_arch_added = true; + } guest_archs[i].arch = arch; guest_archs[i].hvm = hvm; @@ -493,18 +496,23 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (ia64_be) guest_archs[i].ia64_be = ia64_be; - /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */ + /* + * Xen 4.9 introduced support for the PVH guest type, which + * requires hardware virtualization support similar to the + * HVM guest type. Add a PVH guest type for each new HVM + * guest type. + */ if ((ver_info->xen_version_major > 4 || (ver_info->xen_version_major == 4 && ver_info->xen_version_minor >= 9)) && - hvm && i == nr_guest_archs-1) { + hvm && new_arch_added) { i = nr_guest_archs; - /* Too many arch flavours - highly unlikely ! */ - if (i >= ARRAY_CARDINALITY(guest_archs)) + /* Ensure we have not exhausted the guest_archs array */ + if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs)) continue; + guest_archs[nr_guest_archs].arch = arch; + guest_archs[nr_guest_archs].pvh = 1; nr_guest_archs++; - guest_archs[i].arch = arch; - guest_archs[i].pvh = 1; } } } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.