Re: [Xen-devel] [PATCH 08/19] libxl: wait for qemu to acknowledge logdirty command

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 08/19] libxl: wait for qemu to 
acknowledge logdirty command"):
> On Fri, 2012-06-08 at 18:34 +0100, Ian Jackson wrote:
> > The current migration code in libxl instructs qemu to start or stop
> > logdirty, but it does not wait for an acknowledgement from qemu before
> > continuing.  This might lead to memory corruption (!)
...
> > +static void logdirty_init(libxl__logdirty_switch *lds)
> > +{
> > +    lds->cmd_path = 0;
> > +    libxl__ev_xswatch_init(&lds->watch);
> > +    libxl__ev_time_init(&lds->timeout);
> 
> I meant to mention this once before when reviewing some patch or other
> but I'm not sure I actually did, so at the risk of repeating myself:
> 
> This watch with timeout pattern seems to be reasonably common (in fact,
> I'm not sure we have any watches without a timeout). It might be a
> candidate for a specific helper routine in the future?

Perhaps.  We should think about this.  I'm not sure it's necessary
now.  The benefit might be relatively small, as the callback function
would more complicated.  Perhaps we should integrate the single xs
read which most of these callers also have.

> > +static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch 
> > *watch,
> > +                            const char *watch_path, const char *event_path)
> > +{
...
> > + out:
> > +    /* rc < 0: error
> > +     * rc == 0: ok, we are done
> > +     * rc == +1: need to keep waiting
> > +     */
> > +    libxl__xs_transaction_abort(gc, &t);
> > +
> > +    if (!rc) {
> > +        switch_logdirty_done(egc,dss,1);
> > +    } else if (rc < 0) {
> > +        LOG(ERROR,"logdirty switch: response read failed (rc=%d)",rc);
> 
> Is it only "response read failed" which can cause us to end up here,
> looks like the read, rm etc could do it?

Oh yes.  I should fix that message.  (All of these paths have already
logged something already, but another message probably doesn't hurt.)

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel