Re: [PATCH] xen-block: Fix removal of backend instance via xenstore

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Mar 08, 2021 at 03:38:49PM +0100, Paolo Bonzini wrote:
> On 08/03/21 15:32, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > 
> > Whenever a Xen block device is detach via xenstore, the image
> > associated with it remained open by the backend QEMU and an error is
> > logged:
> >      qemu-system-i386: failed to destroy drive: Node xvdz-qcow2 is in use
> > 
> > This happened since object_unparent() doesn't immediately frees the
> > object and thus keep a reference to the node we are trying to free.
> > The reference is hold by the "drive" property and the call
> > xen_block_drive_destroy() fails.
> > 
> > In order to fix that, we call drain_call_rcu() to run the callback
> > setup by bus_remove_child() via object_unparent().
> > 
> > Fixes: 2d24a6466154 ("device-core: use RCU for list of children of a bus")
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> > CCing people whom introduced/reviewed the change to use RCU to give
> > them a chance to say if the change is fine.
> 
> If nothing else works then I guess it's okay, but why can't you do the
> xen_block_drive_destroy from e.g. an unrealize callback?

I'm not sure if that's possible.

xen_block_device_create/xen_block_device_destroy() is supposed to be
equivalent to do those qmp commands:
    blockdev-add node-name=xvdz-qcow2 driver=qcow2 
file={"driver":"file","filename":"disk.qcow2","locking":"off"}
    device_add id=xvdz driver=xen-disk vdev=xvdz drive=xvdz-qcow2

But I tried to add a call xen_block_drive_destroy from
xen_block_unrealize, but that still is called too early, it's called
before object_property_del_all() which would delete "drive" and call
release_drive() which would free the node.

So, no, I don't think we can use an unrealized callback.

I though of trying to delete the "drive" property ahead of calling
object_unparent() but I didn't figure out how to do so and it's maybe
not possible.

So either drain_call_rcu or adding call_rcu(xen_block_drive_destroy)
seems to be the way, but since xen_block_drive_destroy uses
qmp_blockdev_del, it seems better to drain_call_rcu.

Cheers,

-- 
Anthony PERARD