Re: [PATCH v3 32/32] libxl: allow the creation of HVM domains without a device model.

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Jul 03, 2015 at 01:35:10PM +0200, Roger Pau Monne wrote:
> Replace the firmware loaded into HVM guests with an OS kernel. Since the HVM
> builder now uses the PV xc_dom_* set of functions this kernel will be parsed
> and loaded inside the guest like on PV, but the container is a pure HVM
> guest.
> 
> Also, if device_model_version is set to none unconditinally set the nic type
> to LIBXL_NIC_TYPE_VIF.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  docs/man/xl.cfg.pod.5       |  5 ++++
>  tools/libxl/libxl.c         |  7 +++---
>  tools/libxl/libxl_create.c  | 18 ++++++++++++++
>  tools/libxl/libxl_dom.c     | 60 ++++++++++++++++++++++++++++++---------------
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/libxl_x86.c     |  4 ++-
>  tools/libxl/xl_cmdimpl.c    |  2 ++
>  7 files changed, 72 insertions(+), 25 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a3e0e2e..1270605 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1631,6 +1631,11 @@ This device-model is the default for Linux dom0.
>  Use the device-model based upon the historical Xen fork of Qemu.
>  This device-model is still the default for NetBSD dom0.
>  
> +=item B<none>
> +
> +Don't use any device model. This requires a kernel capable of booting
> +in this mode.
> +
>  =back
>  
>  It is recommended to accept the default value for new guests.  If
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index e9a2d26..a6690cf 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1590,11 +1590,10 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
>  
>      switch (libxl__domain_type(gc, domid)) {
>      case LIBXL_DOMAIN_TYPE_HVM:
> -        if (!libxl_get_stubdom_id(CTX, domid))
> -            dm_present = 1;
> -        else
> +        if (libxl_get_stubdom_id(CTX, domid)) {
>              dm_present = 0;
> -        break;
> +            break;
> +        }

There is a path that dm_present contains garbage.

>      case LIBXL_DOMAIN_TYPE_PV:
>          pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/%d/image/device-model-pid", domid));
>          dm_present = (pid != NULL);
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index f366a09..1444e40 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -164,6 +164,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>                  b_info->u.hvm.bios = LIBXL_BIOS_TYPE_ROMBIOS; break;
>              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>                  b_info->u.hvm.bios = LIBXL_BIOS_TYPE_SEABIOS; break;
> +            case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +                break;
>              default:return ERROR_INVAL;
>              }
>  
> @@ -177,6 +179,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>              if (b_info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS)
>                  return ERROR_INVAL;
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            break;
>          default:abort();
>          }
>  
> @@ -278,6 +282,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>                  break;
>              }
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            b_info->video_memkb = 0;
> +            break;
>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>          default:
>              switch (b_info->u.hvm.vga.kind) {
> @@ -945,6 +952,11 @@ static void initiate_domain_create(libxl__egc *egc,
>          ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
>          if (ret) goto error_out;
>  
> +        /* HVM guests without a device model only have PV nics. */
> +        if (d_config->b_info.device_model_version ==
> +            LIBXL_DEVICE_MODEL_VERSION_NONE)
> +            d_config->nics[i].nictype = LIBXL_NIC_TYPE_VIF;
> +

Better to check if users have asked for emulated NIC and bail.

>          if (d_config->nics[i].devid > last_devid)
>              last_devid = d_config->nics[i].devid;
>      }
> @@ -1293,6 +1305,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
>          libxl__device_console_add(gc, domid, &console, state, &device);
>          libxl__device_console_dispose(&console);
>  
> +        if (d_config->b_info.device_model_version ==
> +            LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
> +            return;
> +        }
> +
>          libxl_device_vkb_init(&vkb);
>          libxl__device_vkb_add(gc, domid, &vkb);
>          libxl_device_vkb_dispose(&vkb);
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 6ce4115..59854a1 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -789,21 +789,23 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
>      uint64_t str_mfn, cons_mfn;
>      int i;
>  
> -    va_map = xc_map_foreign_range(handle, domid,
> -                                  XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
> -                                  HVM_INFO_PFN);
> -    if (va_map == NULL)
> -        return -1;
> -
> -    va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
> -    va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
> -    va_hvm->nr_vcpus = info->max_vcpus;
> -    memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
> -    memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
> -    for (i = 0, sum = 0; i < va_hvm->length; i++)
> -        sum += ((uint8_t *) va_hvm)[i];
> -    va_hvm->checksum -= sum;
> -    munmap(va_map, XC_PAGE_SIZE);
> +    if ( info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) {

Stray space.

> +        va_map = xc_map_foreign_range(handle, domid,
> +                                      XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
> +                                      HVM_INFO_PFN);
> +        if (va_map == NULL)
> +            return -1;
> +
> +        va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
> +        va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
> +        va_hvm->nr_vcpus = info->max_vcpus;
> +        memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
> +        memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
> +        for (i = 0, sum = 0; i < va_hvm->length; i++)
> +            sum += ((uint8_t *) va_hvm)[i];
> +        va_hvm->checksum -= sum;
> +        munmap(va_map, XC_PAGE_SIZE);
> +    }
>  
>      xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);
>      xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);
> @@ -884,6 +886,12 @@ static int libxl__domain_firmware(libxl__gc *gc,
>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>              firmware = "hvmloader";
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            if (info->kernel == NULL) {
> +                LOG(ERROR, "no device model requested without a kernel");
> +                return ERROR_FAIL;

goto out

Maybe fix the other return as well...

Wei.