qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Harper <ryanh@us.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org,
	Anthony Liguori <aliguori@linux.vnet.ibm.com>,
	Ryan Harper <ryanh@us.ibm.com>,
	Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>
Subject: Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Date: Tue, 2 Nov 2010 09:22:01 -0500	[thread overview]
Message-ID: <20101102142201.GH22904@us.ibm.com> (raw)
In-Reply-To: <20101102135827.GB31007@redhat.com>

* Michael S. Tsirkin <mst@redhat.com> [2010-11-02 08:59]:
> On Tue, Nov 02, 2010 at 08:46:22AM -0500, Ryan Harper wrote:
> > * Markus Armbruster <armbru@redhat.com> [2010-11-02 04:40]:

> > > >> >> I'd like to have some consistency among net, block and char device
> > > >> >> commands, i.e. a common set of operations that work the same for all of
> > > >> >> them.  Can we agree on such a set?
> > > >> >
> > > >> > Yeah; the current trouble (or at least what I perceive to be trouble) is
> > > >> > that in the case where the guest responds to device_del induced ACPI
> > > >> > removal event; the current qdev code already does the host-side device
> > > >> > tear down.  Not sure if it is OK to do a blockdev_del() immediately
> > > >> > after the device_del.  What happens when we do:
> > > >> >
> > > >> > device_del
> > > >> > ACPI to guest
> > > >> > blockdev_del /* removes host-side device */
> > > >> 
> > > >> Fails in my tree, because the blockdev's still in use.  See below.
> > > >> 
> > > >> > guest responds to ACPI
> > > >> > qdev calls pci device removal code
> > > >> > qemu attempts to destroy the associated host-side block
> > > >> >
> > > >> > That may just work today; and if not, it shouldn't be hard to fix up the
> > > >> > code to check for NULLs
> > > >> 
> > > >> I hate the automatic deletion of host part along with the guest part.
> > > >> device_del should undo device_add.  {block,net,char}dev_{add,del} should
> > > >> be similarly paired.
> > > >
> > > > Agreed.
> > > >> 
> > > >> In my blockdev branch, I keep the automatic delete only for backwards
> > > >> compatibility: if you create the drive with drive_add, it gets
> > > >> auto-deleted, but if you use blockdev_add, it stays around.
> > > >
> > > > But what to do about the case where we're doing drive_add and then a
> > > > device_del()  That's the urgent situation that needs to be resolved.
> > > 
> > > What's the exact problem we need to solve urgently?
> > > 
> > > Is it "provide means to cut the connection to the host part immediately,
> > > even with an uncooperative guest"?
> > 
> > Yes, need to ensure that if the mgmt layer (libvirt) has done what it
> > believes should have disassociated the host block device from the guest,
> > we want to ensure that the host block device is no longer accessible
> > from the guest.
> > 
> > > 
> > > Does this need to be separate from device_del?
> > 
> > no, it doesn't have to be.  Honestly, I didn't see a clear way to do
> > something like unplug early in the device_del because that's all pci
> > device code which has no knowledge of host block devices; having it
> > disconnect seemed like a layering violation.
> 
> We invoke the cleanup callback, isn't that enough?

Won't that look a bit strange?  on device_del, call the cleanup callback
first;, then notify the guest, if the guest responds, I suppose as long
as the cleanup callback can handle being called a second time that'd
work.

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.  




-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

  reply	other threads:[~2010-11-02 14:22 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-25 18:22 [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal Ryan Harper
2010-10-25 18:22 ` [Qemu-devel] [PATCH 1/3] v2 Add drive_get_by_id Ryan Harper
2010-10-29 13:18   ` Markus Armbruster
2010-10-25 18:22 ` [Qemu-devel] [PATCH 2/3] v2 Fix Block Hotplug race with drive_unplug() Ryan Harper
2010-10-29 14:01   ` Markus Armbruster
2010-10-29 14:15     ` Anthony Liguori
2010-10-29 14:29       ` Kevin Wolf
2010-10-29 14:40         ` Anthony Liguori
2010-10-29 14:57           ` Kevin Wolf
2010-10-29 15:28             ` Anthony Liguori
2010-10-29 16:08               ` Kevin Wolf
2010-10-30 13:25                 ` Christoph Hellwig
2010-10-29 15:28       ` Markus Armbruster
2010-11-01 21:06     ` Ryan Harper
2010-10-25 18:22 ` [Qemu-devel] [PATCH 3/3] Add qmp version of drive_unplug Ryan Harper
2010-10-29 14:12 ` [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal Markus Armbruster
2010-10-29 15:03   ` Ryan Harper
2010-10-29 16:10     ` Markus Armbruster
2010-10-29 16:50       ` Ryan Harper
2010-11-02  9:40         ` Markus Armbruster
2010-11-02 13:22           ` Michael S. Tsirkin
2010-11-02 13:41           ` Kevin Wolf
2010-11-02 13:46           ` Ryan Harper
2010-11-02 13:58             ` Michael S. Tsirkin
2010-11-02 14:22               ` Ryan Harper [this message]
2010-11-02 15:46                 ` Michael S. Tsirkin
2010-11-02 16:53                   ` Ryan Harper
2010-11-02 17:59                     ` Michael S. Tsirkin
2010-11-02 19:01                       ` Ryan Harper
2010-11-02 19:17                         ` Michael S. Tsirkin
2010-11-02 20:23                           ` Ryan Harper
2010-11-03  7:21                             ` Michael S. Tsirkin
2010-11-03 12:04                               ` Ryan Harper
2010-11-03 16:41                                 ` Markus Armbruster
2010-11-03 17:29                                   ` Ryan Harper
2010-11-03 18:02                                     ` Michael S. Tsirkin
2010-11-03 20:59                                       ` Ryan Harper
2010-11-03 21:26                                         ` Michael S. Tsirkin
2010-11-04 16:45                                           ` Ryan Harper
2010-11-04 17:04                                             ` Michael S. Tsirkin
2010-11-05 13:27                                             ` Markus Armbruster
2010-11-05 14:17                                               ` Michael S. Tsirkin
2010-11-05 14:29                                                 ` Ryan Harper
2010-11-05 16:01                                                 ` Markus Armbruster
2010-11-08 21:02                                                   ` Michael S. Tsirkin
2010-11-05 14:25                                               ` Ryan Harper
2010-11-05 16:10                                                 ` Markus Armbruster
2010-11-05 16:22                                                   ` Ryan Harper
2010-11-06  8:18                                                     ` Markus Armbruster
2010-11-08  2:19                                                       ` Ryan Harper
2010-11-08 10:32                                                         ` Markus Armbruster
2010-11-08 10:49                                                           ` Michael S. Tsirkin
2010-11-08 12:03                                                             ` Markus Armbruster
2010-11-08 14:02                                                               ` Ryan Harper
2010-11-08 16:56                                                                 ` Michael S. Tsirkin
2010-11-08 17:04                                                                   ` Daniel P. Berrange
2010-11-08 18:41                                                                     ` Ryan Harper
2010-11-08 18:39                                                                   ` Ryan Harper
2010-11-08 19:06                                                                     ` Daniel P. Berrange
2010-11-08 16:34                                                               ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101102142201.GH22904@us.ibm.com \
    --to=ryanh@us.ibm.com \
    --cc=aliguori@linux.vnet.ibm.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefan.hajnoczi@uk.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).