Re: [Xen-devel] [PATCH V4 23/24] libxl: consider force removal of device successful

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, May 07, 2014 at 11:18:51AM +0100, Ian Campbell wrote:
[...]
> > The commit message is written like this due to the current behavior.
> > 
> > If you remove a device, and the backend times out, libxl will try to
> > force-remove (destroy) this device. Then it returns "fail to remove a
> > device" even if the force removal is successful.
> > 
> > This is rather confusing because libxl tells you the removal fails, but
> > actually the device is long gone from guest's PoV.
> 
> I see. I'm not sure how sensible I think this automatic fallback to
> forcing is, but given that is the behaviour I think your change does
> make sense. I'd be in favour of pasting most of your explanation above
> into the commit message though and/or comments.
> 

NP.

> > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > ---
> > > >  tools/libxl/libxl_device.c   |   12 ++++++++++--
> > > >  tools/libxl/libxl_internal.h |    4 ++++
> > > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > > > index fa99f77..7a37778 100644
> > > > --- a/tools/libxl/libxl_device.c
> > > > +++ b/tools/libxl/libxl_device.c
> > > > @@ -845,6 +845,11 @@ void libxl__initiate_device_remove(libxl__egc *egc,
> > > >          if (rc < 0) goto out;
> > > >      }
> > > >  
> > > > +    /* At this point the XS transaction is commited. So check if we
> > > > +     * force removal of the device.
> > > 
> > > "check if we were force removing the device". I think committed has two
> > > t-s too (although I confess I'm not 100% sure...).
> > > 
> > 
> > You're right. I have lots of typos in this series. :-(
> 
> FWIW I only spot other people's, not my own ;-)
> 
> > 
> > > > +     */
> > > > +    aodev->force_removed = aodev->force;
> > > 
> > > How and when can aodev->force_removed be distinct from aodev->force?
> > > 
> > 
> > The removal of xenstore entries can fail. So we are only sure if the
> > device is force removed if the xenstore transaction is committed.
> 
> If the xenstore commit fails the force_removed and force can differ?
> 

If xenstore transaction fails (commit failure being one of them), AO
ends with error code from the failed operation. In that case the
change is not committed and device entries are still there.

Wei.

> Ian.

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