Re: [Xen-devel] [PATCH v2] libxl: fix migration of PV and PVH domUs with and without qemu

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, May 09, 2019 at 05:54:38PM +0200, Olaf Hering wrote:
> Am Fri, 3 May 2019 17:29:53 +0200
> schrieb Roger Pau Monné <roger.pau@xxxxxxxxxx>:
> 
> > On Fri, May 03, 2019 at 04:11:32PM +0200, Olaf Hering wrote:
> > > Am Fri, 3 May 2019 13:04:11 +0200
> > > schrieb Roger Pau Monné <roger.pau@xxxxxxxxxx>:
> > >   
> > > > Wouldn't it be easier to leave libxl__need_xenpv_qemu alone and just
> > > > use the contents of the migration stream to decide whether to launch a
> > > > QEMU for the PV backends or not? ie: just parsing the domain config on
> > > > the migration stream should be enough for the destination side to
> > > > decide whether a QEMU is needed in order to handle the PV backends?  
> > > 
> > > I think that is done anyway. How would the receiving side know what to 
> > > do?  
> > 
> > Hm, OK. I will wait for v3 with the domid stuff fixed.
> 
> I think it will be as simple as this. In the end b_info.device_model_version
> must be set prior the call to store_libxl_entry.

The patch below looks sensible to me, and it's more like what I was
expecting would be needed to solve this issue.

> 
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -92,9 +92,6 @@ int libxl__domain_build_info_setdefault(
>              } else {
>                  b_info->device_model_version = 
> libxl__default_device_model(gc);
>              }
> -        } else {
> -            b_info->device_model_version =
> -                LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
>          }
>          if (b_info->device_model_version
>                  == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> @@ -969,6 +966,18 @@ static void initiate_domain_create(libxl
>      dcs->sdss.dm.guest_domid = 0; /* means we haven't spawned */
>  
>      /*
> +     * libxl__domain_build_info_setdefault sets this only for HVM
> +     * set it before store_libxl_entry()
> +     */
> +    if (d_config->b_info.device_model_version
> +        == LIBXL_DEVICE_MODEL_VERSION_UNKNOWN) {
> +        ret = libxl__need_xenpv_qemu(gc, d_config);
> +        if (ret > 0)

Since libxl__need_xenpv_qemu can fail, what about doing something
like:

switch ( (ret = libxl__need_xenpv_qemu(gc, d_config)) ) {
case 1:
    d_config->b_info.device_model_version =
    LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
    break;

case 0:
    break;

default:
    LOGEVD(ERROR, ret, domid, "Unable to determine QEMU requisite");
    goto error_out;
}

Note the error message wording is not very good, probably an English
speaker would come up with a better message.

Thanks for looking into this!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel