Re: [Xen-devel] [PATCH 15/19] libxl: suspend: Async xenstore pvcontrol wait

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH 15/19] libxl: suspend: Async xenstore 
pvcontrol wait"):
> On Tue, 2014-03-04 at 14:56 +0000, Ian Jackson wrote:
> > When negotiating guest suspend via the xenstore pvcontrol protocol
> > (ie when the guest does NOT support the evtchn fast suspend protocol):
...
> > -    /*
> > -     * Final check for guest acknowledgement. The guest may have
> > -     * acknowledged while we were cancelling the request in which
> > -     * case we lost the race while cancelling and should continue.
> 
> This behaviour has gone completely now?

No, this possibility still exists.  This is handled by the rc ==
ERROR_TIMEDOUT branch in domain_suspend_common_pvcontrol_suspending.
The comment there (now) reads:

        /*
         * Guest appears to not be responding. Cancel the suspend
         * request.
         *
         * We re-read the suspend node and clear it within a
         * transaction in order to handle the case where we race
         * against the guest catching up and acknowledging the request
         * at the last minute.
         */

> > +    assert(domain_suspend_pvcontrol_acked(state));
> >      LOG(DEBUG, "guest acknowledged suspend request");
> > +
> > +    libxl__xs_transaction_abort(gc, &t);
> 
> Is this right/proper in the success case?

Yes.  Because it has then already been committed.  This is exactly the
pattern from the usage comment for libxl__xs_transaction_* in
libxl_internal.h.

> If rc != TIMEOUT t may not have been started, although it is initialised
> so I suppose this is intended to be ok.

Yes.  Precisely.

Thanks for the careful review :-).

Ian.

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