From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40708) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmQQu-0003UD-OU for qemu-devel@nongnu.org; Wed, 14 Oct 2015 14:05:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZmQQt-00024V-5N for qemu-devel@nongnu.org; Wed, 14 Oct 2015 14:05:48 -0400 References: From: Max Reitz Message-ID: <561E996E.4040608@redhat.com> Date: Wed, 14 Oct 2015 20:05:34 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gaTIVOgnvrhK5CRUE2XIdvE5j2V7rOAfA" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 3/3] block: use bdrv_lookup_bs() over blk_by_name() for BDS only results List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody , qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, qemu-block@nongnu.org, quintela@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gaTIVOgnvrhK5CRUE2XIdvE5j2V7rOAfA Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 14.10.2015 15:16, Jeff Cody wrote: > To find a BlockDriverState interface, it can be done via blk_by_name(),= > bdrv_find_node(), and bdrv_lookup_bs(). The latter can take the place > of the other two, in the instances where we are only concerned with the= > BlockDriverState. >=20 > In much of the usage of blk_by_name(), we can simplify the code by > calling bdrv_lookup_bs() instead of blk_by_name(), when what we need is= > just the BDS, and not the BB. >=20 > Signed-off-by: Jeff Cody > --- > blockdev.c | 84 ++++++++++++++++++++---------------------------= -------- > migration/block.c | 6 ++-- > 2 files changed, 32 insertions(+), 58 deletions(-) Again, if this series is based on Berto's blockdev-snapshot series, it should be based on my BB series as well. But with that series applied, this patch has some (non-trivial) rebase conflicts on it. Also, it has a fundamental conflict with that series: If a BB can be found by bdrv_lookup_bs() but it doesn't have a BDS, it will return NULL and set *errp accordingly ("No medium in device"). However, this patch discards that error message and just keeps the one of the former blk_by_name() caller, which is generally "Device not found". That is wrong, however, if there is simply no medium in the device. In some cases, that doesn't matter (since we just assume that if there is a BB, it will have a BDS, like for hmp_commit), but it most probably does matter for all the places which conflict with my BB series, the reason being that they generally conflict because I added a blk_is_available() check. Note that this is a "design conflict", too: bdrv_lookup_bs() will return NULL only if blk_bs() is NULL. blk_is_available() may return false even if there is a BDS (if the BDS is unavailable, e.g. one of the BDSs in the tree not being inserted or the guest device's tray is open). But if you already have a BDS (because you used bdrv_lookup_bs()), calling blk_is_available() on bs->blk afterwards looks kind of strange... Max --gaTIVOgnvrhK5CRUE2XIdvE5j2V7rOAfA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWHpluAAoJEDuxQgLoOKytnYcIAKuDWvkTbV4BMObYvlXVFm74 xudffcqm4i5xidrN/uEkRAunY6kYOsuzSvb+AsCEwuKz5gy7Jhrly/NVqCVj0ed/ LfcX2l2qc+2zqlfei2zxf9s1p4dtVLMa132aYg0zTy9kg5l7jHFzjCGpTTZqtEYl jnFW/joDMyWCVlwcmMmBlJLjrR/XekPWsZisjs5M8cJjjOQY1upvlutrVg3M0XRB n0XzCqqwdzq7bYyQ1Y24/nP2ja4wAoXaMruiDZ1jUto4t+8zkXXD5PAu5Zf9Ji4F Xhbjwd9n40NAY1odj0jpn/PSdda3v41jkCav18xmnR3wjzx6h9rGPn0fuYelcuQ= =9hh8 -----END PGP SIGNATURE----- --gaTIVOgnvrhK5CRUE2XIdvE5j2V7rOAfA--