Re: [PATCH v1] libxl: always call qemus xen-save-devices-state in suspend/resume

[Anthony PERARD <anthony.perard@citrix.com>]

On Fri, May 18, 2018 at 06:01:42PM +0100, Wei Liu wrote:
> Cc Anthony.
> 
> On Thu, May 17, 2018 at 05:51:08PM +0200, Olaf Hering wrote:
> > If a domU has a qemu-xen instance attached, it is required to call qemus
> > "xen-save-devices-state" method. Without it, the receiving side of a PV
> > migration may be unable to lock the image:
> > 
> > xen be: qdisk-51712: xen be: qdisk-51712: error: Failed to get "write" lock
> > error: Failed to get "write" lock
> > xen be: qdisk-51712: xen be: qdisk-51712: initialise() failed
> > initialise() failed
> > 
> 
> The commit message can use a bit of improvement. Strictly speaking, this
> is only a problem when qdisk is in use.
> 
> But I think it should be harmless to issue that command anyway.
> 
> > The proper way to decide if a PV or PVH domU has a qemu-xen running is
> > to use the libxl__need_xenpv_qemu API. But since there is no copy of a
> > libxl_domain_config available in these places, it should be enough to
> > check if xenstore contains the relevant info.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > ---
> >  tools/libxl/libxl_dom_suspend.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
> > index 1e904bae8a..0d769eefd8 100644
> > --- a/tools/libxl/libxl_dom_suspend.c
> > +++ b/tools/libxl/libxl_dom_suspend.c
> > @@ -377,7 +377,9 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
> >      libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
> >      libxl__ev_time_deregister(gc, &dsps->guest_timeout);
> >  
> > -    if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) {
> > +    if (dsps->type == LIBXL_DOMAIN_TYPE_HVM ||
> > +        libxl__device_model_version_running(gc, dsps->domid) ==
> > +        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> 
> You don't need to test if the guest is HVM anymore. You only need to
> know if QEMU upstream is running.
> 
> Anthony, please check if I've said anything wrong.

I think the patch is fine.

The resume side is not needed for PV because that only resume emulation,
and qemu start with emulation running, but it is armless to call the
command.

Thanks,

-- 
Anthony PERARD

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