[libvirt] [PATCH 1/2] libxl: add support for PVH
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> --- src/libxl/libxl_capabilities.c | 40 +++++++++++++++++++++++++++++++--------- src/libxl/libxl_conf.c | 2 ++ src/libxl/libxl_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 0145116..c443353 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -45,11 +45,16 @@ VIR_LOG_INIT("libxl.libxl_capabilities"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40 +enum machine_type { + machine_hvm, + machine_pvh, + machine_pv, +}; struct guest_arch { virArch arch; int bits; - int hvm; + enum machine_type machine; int pae; int nonpae; int ia64_be; @@ -296,7 +301,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) /* Search for existing matching (model,hvm) tuple */ for (i = 0; i < nr_guest_archs; i++) { if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) + guest_archs[i].machine == (hvm ? machine_hvm : machine_pv)) break; } @@ -308,7 +313,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) nr_guest_archs++; guest_archs[i].arch = arch; - guest_archs[i].hvm = hvm; + guest_archs[i].machine = hvm ? machine_hvm : machine_pv; /* Careful not to overwrite a previous positive setting with a negative one here - some archs @@ -320,23 +325,40 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* On Xen >= 4.4 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 >= 4)) && + 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].machine = machine_pvh; + } } } 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].machine == machine_hvm ? "xenfv" : + (guest_archs[i].machine == machine_pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines; if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) return -1; if ((guest = virCapabilitiesAddGuest(caps, - guest_archs[i].hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, + guest_archs[i].machine == machine_hvm ? + VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, guest_archs[i].arch, LIBXL_EXECBIN_DIR "/qemu-system-i386", - (guest_archs[i].hvm ? + (guest_archs[i].machine == machine_hvm ? LIBXL_FIRMWARE_DIR "/hvmloader" : NULL), 1, @@ -375,7 +397,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0) == NULL) return -1; - if (guest_archs[i].hvm) { + if (guest_archs[i].machine != machine_pv) { if (virCapabilitiesAddGuestFeature(guest, "acpi", 1, @@ -390,7 +412,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (virCapabilitiesAddGuestFeature(guest, "hap", 1, - 1) == NULL) + guest_archs[i].machine == machine_hvm) == NULL) return -1; } } @@ -409,7 +431,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 5202ca1..aa06586 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -173,6 +173,8 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, } } else { c_info->type = LIBXL_DOMAIN_TYPE_PV; + if (STREQ(def->os.machine, "xenpvh")) + libxl_defbool_set(&c_info->pvh, true); } if (VIR_STRDUP(c_info->name, def->name) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4957072..fa58346 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6321,9 +6321,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 { -- 2.5.5
From marmarek@invisiblethingslab.com Fri Aug 5 20:06:26 2016 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= <marmarek@invisiblethingslab.com> To: libvir-list@redhat.com Cc: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= <marmarek@invisiblethingslab.com> Subject: [PATCH 2/2] libxl: set shadow memory for any guest type, not only HVM Date: Fri, 5 Aug 2016 20:05:44 +0200 Message-Id: <1470420344-7693-2-git-send-email-marmarek@invisiblethingslab.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1470420344-7693-1-git-send-email-marmarek@invisiblethingslab.com> References: <1470420344-7693-1-git-send-email-marmarek@invisiblethingslab.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Organization: Invisible Things Lab Content-Transfer-Encoding: 8bit
Otherwise starting PVH guest will result in "arch_setup_bootlate: mapping shared_info failed (pfn=..., rc=-1, errno: 12): Internal error". After this change the behaviour is the same as in `xl`. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa06586..155934c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -494,11 +494,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } } - - /* Allow libxl to calculate shadow memory requirements */ - b_info->shadow_memkb = - libxl_get_required_shadow_memory(b_info->max_memkb, - b_info->max_vcpus); } else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -528,6 +523,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } + /* Allow libxl to calculate shadow memory requirements */ + b_info->shadow_memkb = + libxl_get_required_shadow_memory(b_info->max_memkb, + b_info->max_vcpus); + return 0; } -- 2.5.5
Otherwise starting PVH guest will result in "arch_setup_bootlate: mapping shared_info failed (pfn=..., rc=-1, errno: 12): Internal error". After this change the behaviour is the same as in `xl`. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa06586..155934c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -494,11 +494,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } } - - /* Allow libxl to calculate shadow memory requirements */ - b_info->shadow_memkb = - libxl_get_required_shadow_memory(b_info->max_memkb, - b_info->max_vcpus); } else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -528,6 +523,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; } + /* Allow libxl to calculate shadow memory requirements */ + b_info->shadow_memkb = + libxl_get_required_shadow_memory(b_info->max_memkb, + b_info->max_vcpus); + return 0; } -- 2.5.5
On 08/05/2016 12:05 PM, Marek Marczykowski-Górecki wrote:
Otherwise starting PVH guest will result in "arch_setup_bootlate: mapping shared_info failed (pfn=..., rc=-1, errno: 12): Internal error".
After this change the behaviour is the same as in `xl`.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
This patch looks good and could nearly be committed independent of 1/2, but I'm afraid "PVH" in the commit message could trick people into thinking the driver supports creating pvh domains :-). Regards, Jim
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa06586..155934c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -494,11 +494,6 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } } - - /* Allow libxl to calculate shadow memory requirements */ - b_info->shadow_memkb = - libxl_get_required_shadow_memory(b_info->max_memkb, - b_info->max_vcpus); } else { /* * For compatibility with the legacy xen toolstack, default to pygrub @@ -528,6 +523,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, return -1; }
+ /* Allow libxl to calculate shadow memory requirements */ + b_info->shadow_memkb = + libxl_get_required_shadow_memory(b_info->max_memkb, + b_info->max_vcpus); + return 0; }
On 08/05/2016 12:05 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> --- src/libxl/libxl_capabilities.c | 40 +++++++++++++++++++++++++++++++--------- src/libxl/libxl_conf.c | 2 ++ src/libxl/libxl_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 11 deletions(-)
I didn't investigate, but this patch did not apply cleanly. Does 'xenpvh' need to be added to docs/schema/domaincommon.rng? The schema looks dated anyhow since it currently contains 'xenpv' and 'xenner'. And perhaps this value should be added to docs/formatdomain.html.in, along with a sentence about the possible values for Xen machines.
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 0145116..c443353 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -45,11 +45,16 @@ VIR_LOG_INIT("libxl.libxl_capabilities"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum machine_type { + machine_hvm, + machine_pvh, + machine_pv, +};
struct guest_arch { virArch arch; int bits; - int hvm; + enum machine_type machine; int pae; int nonpae; int ia64_be; @@ -296,7 +301,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) /* Search for existing matching (model,hvm) tuple */ for (i = 0; i < nr_guest_archs; i++) { if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) + guest_archs[i].machine == (hvm ? machine_hvm : machine_pv)) break; }
@@ -308,7 +313,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) nr_guest_archs++;
guest_archs[i].arch = arch; - guest_archs[i].hvm = hvm; + guest_archs[i].machine = hvm ? machine_hvm : machine_pv;
/* Careful not to overwrite a previous positive setting with a negative one here - some archs @@ -320,23 +325,40 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* On Xen >= 4.4 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 >= 4)) && + 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].machine = machine_pvh; + } } } 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].machine == machine_hvm ? "xenfv" : + (guest_archs[i].machine == machine_pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines;
if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) return -1;
if ((guest = virCapabilitiesAddGuest(caps, - guest_archs[i].hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, + guest_archs[i].machine == machine_hvm ? + VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN,
Is a new VIR_DOMAIN_OSTYPE_XENPVH needed?
guest_archs[i].arch, LIBXL_EXECBIN_DIR "/qemu-system-i386", - (guest_archs[i].hvm ? + (guest_archs[i].machine == machine_hvm ? LIBXL_FIRMWARE_DIR "/hvmloader" : NULL), 1, @@ -375,7 +397,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0) == NULL) return -1;
- if (guest_archs[i].hvm) { + if (guest_archs[i].machine != machine_pv) { if (virCapabilitiesAddGuestFeature(guest, "acpi", 1, @@ -390,7 +412,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (virCapabilitiesAddGuestFeature(guest, "hap", 1, - 1) == NULL) + guest_archs[i].machine == machine_hvm) == NULL) return -1; } } @@ -409,7 +431,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 5202ca1..aa06586 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -173,6 +173,8 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, } } else { c_info->type = LIBXL_DOMAIN_TYPE_PV; + if (STREQ(def->os.machine, "xenpvh")) + libxl_defbool_set(&c_info->pvh, true);
I assume this won't change with HVMlite, aka pvh2?
}
if (VIR_STRDUP(c_info->name, def->name) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4957072..fa58346 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6321,9 +6321,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 {
WRT domain capabilities, should pvh be treated like pv? I.e. do they both have the same max vcpus, etc? Also, supporting a new knob in the XML usually means supporting conversion of that knob to xl.cfg. Can you add domXML <-> xl.cfg conversion for pvh? And a test case for the conversion too please? Thanks for your patience. Regards, Jim
On Fri, Sep 16, 2016 at 04:39:23PM -0600, Jim Fehlig wrote:
On 08/05/2016 12:05 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> --- src/libxl/libxl_capabilities.c | 40 +++++++++++++++++++++++++++++++--------- src/libxl/libxl_conf.c | 2 ++ src/libxl/libxl_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 11 deletions(-)
I didn't investigate, but this patch did not apply cleanly.
Does 'xenpvh' need to be added to docs/schema/domaincommon.rng? The schema looks dated anyhow since it currently contains 'xenpv' and 'xenner'. And perhaps this value should be added to docs/formatdomain.html.in, along with a sentence about the possible values for Xen machines.
After further evaluation[1], PVHv1 is not the thing I wanted here. And PVHv2 is going to be significantly different. While this patch do work for me, I'm not going to spend more time on PVHv1.
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 0145116..c443353 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -45,11 +45,16 @@ VIR_LOG_INIT("libxl.libxl_capabilities"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum machine_type { + machine_hvm, + machine_pvh, + machine_pv, +};
struct guest_arch { virArch arch; int bits; - int hvm; + enum machine_type machine; int pae; int nonpae; int ia64_be; @@ -296,7 +301,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) /* Search for existing matching (model,hvm) tuple */ for (i = 0; i < nr_guest_archs; i++) { if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) + guest_archs[i].machine == (hvm ? machine_hvm : machine_pv)) break; }
@@ -308,7 +313,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) nr_guest_archs++;
guest_archs[i].arch = arch; - guest_archs[i].hvm = hvm; + guest_archs[i].machine = hvm ? machine_hvm : machine_pv;
/* Careful not to overwrite a previous positive setting with a negative one here - some archs @@ -320,23 +325,40 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* On Xen >= 4.4 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 >= 4)) && + 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].machine = machine_pvh; + } } } 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].machine == machine_hvm ? "xenfv" : + (guest_archs[i].machine == machine_pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines;
if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) return -1;
if ((guest = virCapabilitiesAddGuest(caps, - guest_archs[i].hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, + guest_archs[i].machine == machine_hvm ? + VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN,
Is a new VIR_DOMAIN_OSTYPE_XENPVH needed?
Not sure about this. Wouldn't that require adding `os.type == VIR_DOMAIN_OSTYPE_XEN || os.type == VIR_DOMAIN_OSTYPE_XENPVH` in a lot of places? If actual settings are mostly the same, I don't see any reason for introducing such value.
guest_archs[i].arch, LIBXL_EXECBIN_DIR "/qemu-system-i386", - (guest_archs[i].hvm ? + (guest_archs[i].machine == machine_hvm ? LIBXL_FIRMWARE_DIR "/hvmloader" : NULL), 1, @@ -375,7 +397,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0) == NULL) return -1;
- if (guest_archs[i].hvm) { + if (guest_archs[i].machine != machine_pv) { if (virCapabilitiesAddGuestFeature(guest, "acpi", 1, @@ -390,7 +412,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (virCapabilitiesAddGuestFeature(guest, "hap", 1, - 1) == NULL) + guest_archs[i].machine == machine_hvm) == NULL) return -1; } } @@ -409,7 +431,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 5202ca1..aa06586 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -173,6 +173,8 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, } } else { c_info->type = LIBXL_DOMAIN_TYPE_PV; + if (STREQ(def->os.machine, "xenpvh")) + libxl_defbool_set(&c_info->pvh, true);
I assume this won't change with HVMlite, aka pvh2?
It will, unfortunately. HVMlite is enabled by setting device model to none.
}
if (VIR_STRDUP(c_info->name, def->name) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4957072..fa58346 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6321,9 +6321,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 {
WRT domain capabilities, should pvh be treated like pv? I.e. do they both have the same max vcpus, etc?
Yes, PVH behave like PV. But PVHv2 like HVM.
Also, supporting a new knob in the XML usually means supporting conversion of that knob to xl.cfg. Can you add domXML <-> xl.cfg conversion for pvh? And a test case for the conversion too please?
I'll add this for PVHv2... [1] http://markmail.org/message/c7o7qsc3chkigdzv -- 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?
On 09/16/2016 05:14 PM, Marek Marczykowski-Górecki wrote:
On Fri, Sep 16, 2016 at 04:39:23PM -0600, Jim Fehlig wrote:
On 08/05/2016 12:05 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> --- src/libxl/libxl_capabilities.c | 40 +++++++++++++++++++++++++++++++--------- src/libxl/libxl_conf.c | 2 ++ src/libxl/libxl_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 11 deletions(-) I didn't investigate, but this patch did not apply cleanly.
Does 'xenpvh' need to be added to docs/schema/domaincommon.rng? The schema looks dated anyhow since it currently contains 'xenpv' and 'xenner'. And perhaps this value should be added to docs/formatdomain.html.in, along with a sentence about the possible values for Xen machines. After further evaluation[1], PVHv1 is not the thing I wanted here. And PVHv2 is going to be significantly different. While this patch do work for me, I'm not going to spend more time on PVHv1.
Ok. I'm not a fan of supporting PVHv1 in libvirt anyhow. It came and went before anyone even used it :-).
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 0145116..c443353 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -45,11 +45,16 @@ VIR_LOG_INIT("libxl.libxl_capabilities"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum machine_type { + machine_hvm, + machine_pvh, + machine_pv, +};
struct guest_arch { virArch arch; int bits; - int hvm; + enum machine_type machine; int pae; int nonpae; int ia64_be; @@ -296,7 +301,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) /* Search for existing matching (model,hvm) tuple */ for (i = 0; i < nr_guest_archs; i++) { if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) + guest_archs[i].machine == (hvm ? machine_hvm : machine_pv)) break; }
@@ -308,7 +313,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) nr_guest_archs++;
guest_archs[i].arch = arch; - guest_archs[i].hvm = hvm; + guest_archs[i].machine = hvm ? machine_hvm : machine_pv;
/* Careful not to overwrite a previous positive setting with a negative one here - some archs @@ -320,23 +325,40 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) guest_archs[i].nonpae = nonpae; if (ia64_be) guest_archs[i].ia64_be = ia64_be; + + /* On Xen >= 4.4 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 >= 4)) && + 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].machine = machine_pvh; + } } } 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].machine == machine_hvm ? "xenfv" : + (guest_archs[i].machine == machine_pvh ? "xenpvh" : "xenpv")}; virCapsGuestMachinePtr *machines;
if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL) return -1;
if ((guest = virCapabilitiesAddGuest(caps, - guest_archs[i].hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, + guest_archs[i].machine == machine_hvm ? + VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN, Is a new VIR_DOMAIN_OSTYPE_XENPVH needed? Not sure about this. Wouldn't that require adding `os.type == VIR_DOMAIN_OSTYPE_XEN || os.type == VIR_DOMAIN_OSTYPE_XENPVH` in a lot of places? If actual settings are mostly the same, I don't see any reason for introducing such value.
As long as PVHv2/HVMlite is just another impl of an existing machine type, I agree.
guest_archs[i].arch, LIBXL_EXECBIN_DIR "/qemu-system-i386", - (guest_archs[i].hvm ? + (guest_archs[i].machine == machine_hvm ? LIBXL_FIRMWARE_DIR "/hvmloader" : NULL), 1, @@ -375,7 +397,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) 0) == NULL) return -1;
- if (guest_archs[i].hvm) { + if (guest_archs[i].machine != machine_pv) { if (virCapabilitiesAddGuestFeature(guest, "acpi", 1, @@ -390,7 +412,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) if (virCapabilitiesAddGuestFeature(guest, "hap", 1, - 1) == NULL) + guest_archs[i].machine == machine_hvm) == NULL) return -1; } } @@ -409,7 +431,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 5202ca1..aa06586 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -173,6 +173,8 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx, } } else { c_info->type = LIBXL_DOMAIN_TYPE_PV; + if (STREQ(def->os.machine, "xenpvh")) + libxl_defbool_set(&c_info->pvh, true);
I assume this won't change with HVMlite, aka pvh2? It will, unfortunately. HVMlite is enabled by setting device model to none.
}
if (VIR_STRDUP(c_info->name, def->name) < 0) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4957072..fa58346 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6321,9 +6321,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 {
WRT domain capabilities, should pvh be treated like pv? I.e. do they both have the same max vcpus, etc? Yes, PVH behave like PV. But PVHv2 like HVM.
Ok, that matches my understanding of PVHv2. It is an alternative HVM impl. Regards, Jim
Also, supporting a new knob in the XML usually means supporting conversion of that knob to xl.cfg. Can you add domXML <-> xl.cfg conversion for pvh? And a test case for the conversion too please? I'll add this for PVHv2...
participants (2)
-
Jim Fehlig -
Marek Marczykowski-Górecki