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 <marma...@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(&regex);
>  
>      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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to