Re: [PATCH 2/2] libxl: do not automatically force detach of block devices

[Wei Liu <wl@xxxxxxx>]

On Mon, Sep 14, 2020 at 12:41:09PM +0200, Roger Pau Monné wrote:
> On Tue, Sep 08, 2020 at 02:19:03PM +0000, Wei Liu wrote:
> > On Thu, Sep 03, 2020 at 11:05:37AM +0100, Paul Durrant wrote:
> > > From: Paul Durrant <pdurrant@xxxxxxxxxx>
> > > 
> > > The manpage for 'xl' documents that guest co-operation is required for a 
> > > (non-
> > > forced) block-detach operation and that it may consequently fail. 
> > > Currently,
> > > however, the implementation of generic device removal means that a 
> > > time-out
> > > of a block-detach is being automatically re-tried with the force flag set
> > > rather than failing. This patch stops such behaviour.
> > > 
> > > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> > 
> > I'm two-minded here.
> > 
> > On the one hand, special-casing VBD in libxl to conform to xl's
> > behaviour seems wrong to me.
> > 
> > On the other hand, if we want to treat all devices the same in libxl,
> > libxl should drop its force-removal behaviour all together, and the
> > retry behaviour would need to be implemented in xl for all devices
> > excepts for VBD. This requires a bit of code churn and, more
> > importantly, changes how those device removal APIs behave. In the end
> > this effort may not worth it.
> 
> I would be worried about those changes, as we would likely have to
> also change libvirt or any other downstreams?
> 
> > If we go with the patch here, we should document this special case on
> > libxl level somehow.
> > 
> > Anthony and Ian, do you have any opinion on this?
> 
> Maybe a new function should be introduced instead, that attempts to
> remove a device gracefully and fail otherwise?
> 
> Then none of the current APIs would change, and xl could use this new
> function to handle VBD removal?

This sounds fine to me.

Wei.

> 
> Roger.