All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Ryan Harper <ryanh@us.ibm.com>
Cc: Stefan Hajnoczi <stefan.hajnoczi@uk.ibm.com>,
	Anthony Liguori <aliguori@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/3] v2 Add drive_get_by_id
Date: Fri, 29 Oct 2010 15:18:42 +0200	[thread overview]
Message-ID: <m31v79m91p.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <1288030956-28383-2-git-send-email-ryanh@us.ibm.com> (Ryan Harper's message of "Mon, 25 Oct 2010 13:22:34 -0500")

Ryan Harper <ryanh@us.ibm.com> writes:

> Add a function to find a drive by id string.
>
> Changes since v1:
> -Coding Style fix

Recommend to put patch history below the --- line, so it doesn't get
included in the commit message.

> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
>  blockdev.c |   13 +++++++++++++
>  blockdev.h |    1 +
>  2 files changed, 14 insertions(+), 0 deletions(-)
[...]

This effectively reverts commit dfb0acd8, which cleans up after commit
f8b6cc00:

    qdev: Decouple qdev_prop_drive from DriveInfo
    
    Make the property point to BlockDriverState, cutting out the DriveInfo
    middleman.  This prepares the ground for block devices that don't have
    a DriveInfo.

    Currently all user-defined ones have a DriveInfo, because the only way
    to define one is -drive & friends (they go through drive_init()).
    DriveInfo is closely tied to -drive, and like -drive, it mixes
    information about host and guest part of the block device.  I'm
    working towards a new way to define block devices, with clean
    host/guest separation, and I need to get DriveInfo out of the way for
    that.

    Fortunately, the device models are perfectly happy with
    BlockDriverState, except for two places: ide_drive_initfn() and
    scsi_disk_initfn() need to check the DriveInfo for a serial number set
    with legacy -drive serial=...  Use drive_get_by_blockdev() there.
    
    Device model code should now use DriveInfo only when explicitly
    dealing with drives defined the old way, i.e. without -device.

I think your do_drive_unplug() could use bdrv_find() instead.  More on
that in my review of your PATCH 2/3.

  reply	other threads:[~2010-10-29 13:19 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 [this message]
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
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=m31v79m91p.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=aliguori@linux.vnet.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=ryanh@us.ibm.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.