Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal

[Michael S. Tsirkin]

On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote:
> * Michael S. Tsirkin <address@hidden> [2010-11-02 14:18]:
> > On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote:
> > > > > > > I like the idea of disconnect; if part of the device_del method 
> > > > > > > was to
> > > > > > > invoke a disconnect method, we could implement that for block, 
> > > > > > > net, etc;
> > > > > > > 
> > > > > > > I'd think we'd want to send the notification, then disconnect.
> > > > > > > Struggling with whether it's worth having some reasonable timeout
> > > > > > > between notification and disconnect.  
> > > > > > 
> > > > > > The problem with this is that it has no analog in real world.
> > > > > > In real world, you can send some notifications to the guest, and 
> > > > > > you can
> > > > > > remove the card.  Tying them together is what created the problem 
> > > > > > in the
> > > > > > first place.
> > > > > > 
> > > > > > Timeouts can be implemented by management, maybe with a nice dialog
> > > > > > being shown to the user.
> > > > > 
> > > > > Very true.  I'm fine with forcing a disconnect during the removal path
> > > > > prior to notification.  Do we want a new disconnect method at the 
> > > > > device
> > > > > level (pci)? or just use the existing removal callback and call that
> > > > > during the initial hotremov event?
> > > > 
> > > > Not sure what you mean by that, but I don't see a device doing anything
> > > > differently wrt surprise or ordered removal. So probably the existing
> > > > callback should do. I don't think we need to talk about disconnect:
> > > > since we decided we are emulating device removal, let's call it
> > > > just that.
> > > 
> > > Because current the "removal" process depends on the guest actually
> > > responding.  What I'm suggesting is that, in Marcus's term, and what
> > > drive_unplug() implements, is to disconnect the host block device from
> > > the guest device to prevent any further access to it in the case the
> > > guest doesn't respond to the removal request made via ACPI.
> > > 
> > > Very specifically, what we're suggesting instead of the drive_unplug()
> > > command so to complete the device removal operation without waiting for
> > > the guest to respond; that's what's going to happen if we invoke the
> > > response callback; it will appear as if the guest responded whether it
> > > did or not.
> > > 
> > > What I was suggesting above was to instead of calling the callback for
> > > handing the guest response was to add a device function called
> > > disconnect which would remove any association of host resources from
> > > guest resources before we notified the guest.  Thinking about it again
> > > I'm not sure this is useful, but if we're going to remove the device
> > > without the guests knowledge, I'm not sure how useful sending the
> > > removal requests via ACPI is in the first place.
> > > 
> > > My feeling is that I'd like to have explicit control over the disconnect
> > > from host resources separate from the device removal *if* we're going to
> > > retain the guest notification.  If we don't care to notify the guest,
> > > then we can just do device removal without notifying the guest
> > > and be done with it.
> > 
> > I imagine management would typically want to do this:
> > 1. notify guest
> > 2. wait a bit
> > 3. remove device
> 
> Yes; but this argues for (1) being a separate command from (3)

Yes. Long term I think we will want a way to do that.

> unless we
> require (3) to include (1) and (2) in the qemu implementation.
> 
> Currently we implement:
> 
> 1. device_del (attempt to remove device)
> 2. notify guest
> 3. if guest responds, remove device
> 4. disconnect host resource from device on destruction
> 
> With my drive_unplug patch we do:
> 
> 1. disconnect host resource from device

This is what drive_unplug does, right?

> 2. device_del (attempt to remove device)
> 3. notify guest
> 4. if guest responds, remove device
> 
> I think we're suggesting to instead do (if we keep disconnect as part of
> device_del)
> 
> 1. device_del (attemp to remove device)
> 2. notify guest
> 3. invoke device destruction callback resulting in disconnect host resource 
> from device
> 4. if guest responds, invoke device destruction path a second time.

By response you mean eject?  No, this is not what I was suggesting.
I was really suggesting that your patch is fine :)
Sorry about confusion.

I was also saying that from what I hear, the pci express support
will at some point need interfaces to
- notify guest about device removal/addition
- get eject from guest
- remove device without talking to guest
- add device without talking to guest
- suppress device deletion on eject

All this can be generic and can work through express
configuration mechanisms or through acpi for pci.
But this is completely separate from unplugging
the host backend, which should be possible at any point.

> 
> -- 
> Ryan Harper
> Software Engineer; Linux Technology Center
> IBM Corp., Austin, Tx
> address@hidden