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-listOn 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 - 2026 Red Hat, Inc.