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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, May 14, 2019 at 10:14:52AM +0200, Olaf Hering wrote:
> Am Tue, 14 May 2019 10:05:58 +0200
> schrieb Olaf Hering <olaf@xxxxxxxxx>:
> 
> > @@ -459,7 +461,9 @@ int libxl__domain_resume(libxl__gc *gc, uint32_t domid, 
> > int suspend_cancel)
> >          goto out;
> >      }
> >  
> > -    if (type == LIBXL_DOMAIN_TYPE_HVM) {
> > +    if (type == LIBXL_DOMAIN_TYPE_HVM ||
> > +        libxl__device_model_version_running(gc, domid) ==
> > +        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> >          rc = libxl__domain_resume_device_model(gc, domid);
> >          if (rc) {
> >              LOGD(ERROR, domid, "failed to resume device model:%d", rc);
> 
> I think this could be done like that instead, so that 
> libxl__device_model_version_running
> is called just once:
> 
> --- a/tools/libxl/libxl_dom_suspend.c
> +++ b/tools/libxl/libxl_dom_suspend.c
> @@ -444,6 +444,8 @@ int libxl__domain_resume_device_model(libxl__gc *gc, 
> uint32_t domid)
>          if (libxl__qmp_resume(gc, domid))
>              return ERROR_FAIL;
>          break;
> +    case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +        break;
>      default:
>          return ERROR_INVAL;
>      }
> @@ -461,14 +463,10 @@ int libxl__domain_resume(libxl__gc *gc, uint32_t domid, 
> int suspend_cancel)
>          goto out;
>      }
>  
> -    if (type == LIBXL_DOMAIN_TYPE_HVM ||
> -        libxl__device_model_version_running(gc, domid) ==
> -        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> -        rc = libxl__domain_resume_device_model(gc, domid);
> -        if (rc) {
> -            LOGD(ERROR, domid, "failed to resume device model:%d", rc);
> -            goto out;
> -        }
> +    rc = libxl__domain_resume_device_model(gc, domid);
> +    if (rc) {
> +        LOGD(ERROR, domid, "failed to resume device model:%d", rc);
> +        goto out;
>      }
>  
>      if (xc_domain_resume(CTX->xch, domid, suspend_cancel)) {
> 

Yeah, from the look of it this is definitely better.

> 
> While it is easy for the resume path, doing the same in the suspend path
> needs more changes. libxl__domain_suspend_device_model would need to receive
> the callback and set it if a device model is active.

What do you mean here? Can't you handle the NONE case just like you do
in the resume function?

Wei.

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