Re: [Xen-devel] [PATCH 3 of 6 V2] libxl: QMP stop/resume & refactor QEMU suspend/resume/save

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 1 Feb 2012, Shriram Rajagopalan wrote:
> On 2012-02-01, at 6:14 AM, Stefano Stabellini 
> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> 
> > On Wed, 1 Feb 2012, rshriram@xxxxxxxxx wrote:
> >> # HG changeset patch
> >> # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> >> # Date 1328070813 28800
> >> # Node ID 3ca830009da79443bb445d983a34f5f78664cdf4
> >> # Parent  9f0a67bd54db89a23078913db578df72c5dba2e3
> >> libxl: QMP stop/resume & refactor QEMU suspend/resume/save
> >> 
> >> Implement QMP stop and resume functionality and split
> >> device model save into 3 parts:
> >> suspend_dm(domid)
> >> save_dm(domid, fd)
> >> resume_dm(domid)
> >> 
> >> Integrate Device model suspend into suspend_common_callback
> >> 
> >> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
> >> 
> >> diff -r 9f0a67bd54db -r 3ca830009da7 tools/libxl/libxl_dom.c
> >> --- a/tools/libxl/libxl_dom.c    Tue Jan 31 20:33:33 2012 -0800
> >> +++ b/tools/libxl/libxl_dom.c    Tue Jan 31 20:33:33 2012 -0800
> >> @@ -425,6 +425,61 @@ static int libxl__domain_suspend_common_
> >>     return rc ? 0 : 1;
> >> }
> >> 
> >> +int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid)
> >> +{
> >> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> >> +    int ret = 0, fd2 = -1;
> >> +    const char *filename = libxl__device_model_savefile(gc, domid);
> >> +
> >> +    switch (libxl__device_model_version_running(gc, domid)) {
> >> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> >> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> >> +                   "Saving device model state to %s", filename);
> >> +        libxl__qemu_traditional_cmd(gc, domid, "save");
> >> +        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, 
> >> NULL);
> >> +        break;
> >> +    }
> >> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> >> +        if (libxl__qmp_stop(gc, domid))
> >> +            return ERROR_FAIL;
> >> +        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | 
> >> S_IWUSR);
> >> +        if (fd2 < 0) {
> >> +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >> +                             "Unable to create a QEMU save file\n");
> >> +            return ERROR_FAIL;
> >> +        }
> >> +        /* Save DM state into fd2 */
> >> +        ret = libxl__qmp_migrate(gc, domid, fd2);
> >> +        if (ret)
> >> +            unlink(filename);
> >> +        close(fd2);
> >> +        fd2 = -1;
> >> +        break;
> >> +    default:
> >> +        return ERROR_INVAL;
> >> +    }
> >> +
> > 
> > Why do you need to introduce libxl__qmp_stop and libxl__qmp_resume?
> > 
> > Also keep in mind that I am about to change the command sent to save the
> > state of devices to "save_devices" because "migrate" is not working
> > properly with Xen.
> > 
> 
> I was modeling the qmp stuff similar to the old style qemu. 
> 
> Please correct me if I am wrong. Until qmp_migrate, qemu could still be 
> writing data to the guest's memory right? 
> 
> So, I thought qmp_stop was a sure shot way to ensure such stuff is not 
> happening under the hood. 

That is correct, however qemu is synchronous, so if the VM is paused qemu
is not going to do anything because it is not receiving any ioreqs from
xen. Moreover during the device state save we temporarily stop qemu to
make sure that the device state we save is consistent.

That said, explicitly stopping and resuming qemu should be harmless.


> With qmp_stop came qmp_resume.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel