qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Harper <ryanh@us.ibm.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Anthony Liguori <aliguori@linux.vnet.ibm.com>,
	Ryan Harper <ryanh@us.ibm.com>,
	Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>,
	yamahata@valinux.co.jp
Subject: Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Date: Mon, 8 Nov 2010 12:41:04 -0600	[thread overview]
Message-ID: <20101108184104.GC22381@us.ibm.com> (raw)
In-Reply-To: <20101108170437.GB26714@redhat.com>

* Daniel P. Berrange <berrange@redhat.com> [2010-11-08 11:05]:
> On Mon, Nov 08, 2010 at 06:56:02PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote:
> > > * Markus Armbruster <armbru@redhat.com> [2010-11-08 06:04]:
> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > >> Here's how the various objects are connected to each other:
> > > > >> 
> > > > >>                contains
> > > > >> drivelist    -----------> DriveInfo
> > > > >>                                 |
> > > > >>                                 | .bdrv
> > > > >>                                 | .id == .bdrv->device_name
> > > > >>                                 |
> > > > >>                contains         V
> > > > >> bdrv_states  -----------> BlockDriverState
> > > > >>                              |   ^
> > > > >>                        .peer |   |
> > > > >>                              |   |                          host part
> > > > >> -----------------------------|---|-----------------------------------
> > > > >>                              |   |                         guest part
> > > > >>                              |   | property "drive"
> > > > >>                              v   |
> > > > >>                           DeviceState
> > > > >> 
> > > > >> To disconnect host from guest part, you need to cut both pointers.  To
> > > > >> delete the host part, you need to delete both objects, BlockDriverState
> > > > >> and DriveInfo.
> > > > >
> > > > >
> > > > > If we remove DriveInfo, how can management later detect that guest part
> > > > > was deleted?
> > > > 
> > > > Directly: check whether the qdev is gone.
> > > > 
> > > > I don't know how to check that indirectly, via DriveInfo.
> > > > 
> > > > >              If you want symmetry with netdev, it's possible to keep a
> > > > > shell of BlockDriverState/DriveInfo around (solving dangling pointer
> > > > > problems).
> > > > 
> > > > netdev_del deletes the host network part:
> > > > 
> > > >     (qemu) info network
> > > >     Devices not on any VLAN:
> > > >       net.0: net=10.0.2.0, restricted=n peer=nic.0
> > > >       nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0
> > > >     (qemu) netdev_del net.0
> > > >     (qemu) info network
> > > >     Devices not on any VLAN:
> > > >       nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0
> > > > 
> > > > It leaves around the VLAN object.  Since qdev property points to that,
> > > > it doesn't dangle.
> > > > 
> > > > In my opinion, drive_del should make the drive vanish from "info block",
> > > 
> > > Yeah; that's the right thing to do here.  Let me respin the patch with
> > > the name change and the additional work to fix up the pointers and
> > > ensure that we don't see the drive in info block.
> > 
> > Daniel, I'd like your input here: can you live with
> > device diappearing from info block and parsing
> > qdev tree info to figure out whether device is really gone?
> 
> We don't use info block for anything. Having to parse the full qdev tree
> to determine if a single device is gone seems rather tedious. It would
> be better if query-qdev took an optional argument, which is the name
> of the device to root the tree at. Then checking whether a device
> named 'foo' is gone just means running 'query-qdev foo' and seeing if
> that returns an error about the device not existing, then we know it
> has gone. No need to parse anything. Being able to query the qdev data
> for a single device, or sub-tree of devices seems useful in its own
> right.

Since I'm not looking forward to parsing info block (easy) nor parsing
all of qdev tree (much harder) I really like the query approach.  

That makes it easy to put a query in the netdev_del/drive_del commands
to skip invoking them if the guest has already responded.

> 
> Regards,
> Daniel
> -- 
> |: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
> |: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
> |: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

  reply	other threads:[~2010-11-08 18:41 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
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 [this message]
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=20101108184104.GC22381@us.ibm.com \
    --to=ryanh@us.ibm.com \
    --cc=aliguori@linux.vnet.ibm.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefan.hajnoczi@uk.ibm.com \
    --cc=yamahata@valinux.co.jp \
    /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).