From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32823) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGVJM-0003zg-EA for qemu-devel@nongnu.org; Fri, 24 Jun 2016 13:54:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGVJK-0004Zm-9k for qemu-devel@nongnu.org; Fri, 24 Jun 2016 13:54:35 -0400 Date: Fri, 24 Jun 2016 19:54:22 +0200 From: Kevin Wolf Message-ID: <20160624175422.GG5422@noname.redhat.com> References: <1466692592-9551-1-git-send-email-kwolf@redhat.com> <1466692592-9551-2-git-send-email-kwolf@redhat.com> <576D6F7F.6020904@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JYK4vJDZwFMowpUq" Content-Disposition: inline In-Reply-To: <576D6F7F.6020904@redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH 1/7] block/qdev: Allow node name for drive properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-block@nongnu.org, armbru@redhat.com, stefanha@redhat.com, mreitz@redhat.com, famz@redhat.com, qemu-devel@nongnu.org --JYK4vJDZwFMowpUq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 24.06.2016 um 19:35 hat Eric Blake geschrieben: > On 06/23/2016 08:36 AM, Kevin Wolf wrote: > > If a node name instead of a BlockBackend name is specified as the driver > > for a guest device, an anonymous BlockBackend is created now. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > hw/core/qdev-properties-system.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > >=20 > > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties= -system.c > > index df38b8a..c5cc9cf 100644 > > --- a/hw/core/qdev-properties-system.c > > +++ b/hw/core/qdev-properties-system.c > > @@ -72,9 +72,18 @@ static void parse_drive(DeviceState *dev, const char= *str, void **ptr, > > const char *propname, Error **errp) > > { > > BlockBackend *blk; > > + bool blk_created =3D false; > > =20 > > blk =3D blk_by_name(str); > > if (!blk) { > > + BlockDriverState *bs =3D bdrv_lookup_bs(NULL, str, NULL); >=20 > So BB name takes priority, but if one is not found, you try a BDS lookup > (node name) and create an anonymous BB to match. Seems okay. Well, there's no real priority here because BBs and BDSes share the same namespace, so it can only be one or the other. I just can't pass both to bdrv_lookup_bs() because I need the BB and not just its root BDS if it's a BB name, so I have to check one after the other. > > + if (bs) { > > + blk =3D blk_new(); > > + blk_insert_bs(blk, bs); > > + blk_created =3D true; > > + } > > + } > > + if (!blk) { > > error_setg(errp, "Property '%s.%s' can't find value '%s'", > > object_get_typename(OBJECT(dev)), propname, str); > > return; >=20 > This return is safe, but looks a bit odd... >=20 > > @@ -91,9 +100,16 @@ static void parse_drive(DeviceState *dev, const cha= r *str, void **ptr, > > error_setg(errp, "Drive '%s' is already in use by another = device", > > str); > > } > > - return; > > + goto fail; >=20 > ...given that you had to convert this return to a goto. Hm, okay. I think it's pretty common to have early error paths return directly and later ones use goto. But blk_unref(NULL) is allowed, so in theory I could change that. Kevin > > } > > + > > *ptr =3D blk; > > + > > +fail: > > + if (blk_created) { > > + /* If we need to keep a reference, blk_attach_dev() took it */ > > + blk_unref(blk); > > + } > > } > > =20 > > static void release_drive(Object *obj, const char *name, void *opaque) --JYK4vJDZwFMowpUq Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXbXPOAAoJEH8JsnLIjy/WBmsP/R8Hq9srJ+vOmNqVHzvEJByC l/Q/qre+RmbCqtf60tpxLwtuil7qLsN9cYDQZzB2ITvK42vFD5c1hFz1ZuzQ9Esn TPymkiRUAgjy6sUaL0MshmB57qHijuGriaV80ez/L8Wh6YcO/IIGnkxn14qvfwom n8N1IViKeTgrK+LuBG+zn8xaddO5gcGWMXm30himoIyBbi10ccyl7vxgLa9ZvISm 9ZPsKeKeAHRb2okVsQB36EQLAB340x7CVtcVH5y/kR+YNtMt6tBmglpN/IfXc89L LHt4g3zfoYR4rO6hfthzQRHjmzQsGucYNxHAuVuUrP/2YblyvBumKW8NmSSZ1OX3 wkd9DkwO0+s+W4fRpD7xoM9Fpcb+SJsYkndIQLsmO/MgjVu8ciUwDwJPieGej4IV z7ECotA06Rz2cSRnFPd3RFWVYinkWC0qQ9zHO8gpyyFYaNm1Egzpc3hiZKIp6qqP dAGpA2GBBqsFAX2HilhPOylCOTs7Zk3XFgw8FBXpGTeZsduyk4LlNjB92RJeSQVl RA10bPI9plzd2oLENb63E/F/T/I+0WhYFC8fA38ZTG7nAv1hTdkt3SjxmrEDwL0x dc4gMW+oO4f222uUpWAI3JBTiPU/wkGBDgLZu0b4LtUW6/c9YacQOMMqmNCnr8fd PUGKDBA2hq6OH3kKdgDT =y3oC -----END PGP SIGNATURE----- --JYK4vJDZwFMowpUq--