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

[Wei Liu <wei.liu2@citrix.com>]

On Tue, May 06, 2014 at 04:48:40PM +0100, Ian Campbell wrote:
> On Thu, 2014-05-01 at 13:58 +0100, Wei Liu wrote:
> > If a device is force removed and the force removal successes, from
> 
> "succeeds".
> 
> > guest's PoV this device is gone. So the toolstack should consider this a
> > successful case as well.
> 
> This seems rather self evident (consider success as success), were you
> perhaps trying to say something opposite, which is that if a force
> remove fails for some reason that we should (e.g. for the purposes of a
> later migration etc) consider the device to have actually been removed?
> 

No.

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.

> 
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  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. :-(

> > +     */
> > +    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.

Wei.

> Ian.