From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfspK-0008Q6-IY for qemu-devel@nongnu.org; Wed, 08 Apr 2015 12:27:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfspJ-0001H8-F1 for qemu-devel@nongnu.org; Wed, 08 Apr 2015 12:27:42 -0400 Message-ID: <552556F6.2000104@redhat.com> Date: Wed, 08 Apr 2015 10:27:34 -0600 From: Eric Blake MIME-Version: 1.0 References: <9823a1f0514fdb0692e92868661c38a9e00a12d6.1428485266.git.berto@igalia.com> In-Reply-To: <9823a1f0514fdb0692e92868661c38a9e00a12d6.1428485266.git.berto@igalia.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HPweP9NSNDie524OXhnGCaeAB0jAR9jaU" Subject: Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HPweP9NSNDie524OXhnGCaeAB0jAR9jaU Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/08/2015 03:29 AM, Alberto Garcia wrote: > There are several error messages that identify a BlockDriverState by > its device name. However those errors can be produced in nodes that > don't have a device name associated. >=20 > In those cases we should use bdrv_get_device_or_node_name() to fall > back to the node name and produce a more meaningful message. The > messages are also updated to use the more generic term 'node' instead > of 'device'. >=20 > Signed-off-by: Alberto Garcia > Reviewed-by: Max Reitz > --- > +++ b/block/snapshot.c > @@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs, > if (bs->file) { > return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp)= ; > } > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, > - drv->format_name, bdrv_get_device_name(bs), > - "internal snapshot deletion"); > + error_setg(errp, "Block format '%s' used by device '%s' " > + "does not support internal snapshot deletion", > + drv->format_name, bdrv_get_device_name(bs)); > return -ENOTSUP; > } > =20 > @@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, > if (drv->bdrv_snapshot_load_tmp) { > return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp= ); > } > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, > - drv->format_name, bdrv_get_device_name(bs), > - "temporarily load internal snapshot"); > + error_setg(errp, "Block format '%s' used by device '%s' " > + "does not support temporarily loading internal snapshot= s", > + drv->format_name, bdrv_get_device_name(bs)); Should these two messages use 'node' instead of 'device'? After all, a format is tied to a node (as a backing chain can involve multiple nodes using different formats) > +++ b/blockdev.c > @@ -1248,13 +1248,14 @@ static void internal_snapshot_prepare(BlkTransa= ctionState *common, > } > =20 > if (bdrv_is_read_only(bs)) { > - error_set(errp, QERR_DEVICE_IS_READ_ONLY, device); > + error_setg(errp, "Device '%s' is read only", device); > return; > } This one is probably fine as device; > =20 > if (!bdrv_can_snapshot(bs)) { > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, > - bs->drv->format_name, device, "internal snapshot"); > + error_setg(errp, "Block format '%s' used by device '%s' " > + "does not support internal snapshots", > + bs->drv->format_name, device); but this is probably another one where node may be better. But it's already a strict improvement, so I can live with: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --HPweP9NSNDie524OXhnGCaeAB0jAR9jaU 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVJVb2AAoJEKeha0olJ0Nqu5wH/i697bUSLrgMqt9okKfc1asC QL30VifE8pGQwiHab50tNiLL09m2DP/311LSDXRePx4CIgZfmerD/liuzOPvlse2 fE/hQJNpNF0X6CLq/ta+soYzG/n/111V+M6vTqvr0MzAlUWVPfDczfEL415qGsfV gL/JaY2MAIWehpLPHo2H7K3HDQmftKTNdW4z5Sjt2JgxRV95taySBS0M935ncl36 K3REHNjKEALonUsNOSlpASKRP+Z68Pn4Dl8wYQ5vm8b8POiRvQZgQHWoilUNq73P HM78mkcJjYqFN3uU3XU4vy/2TRlKOUDwDbxO7DRsSEE4vbiaC8lPwllOl1G1dys= =niW5 -----END PGP SIGNATURE----- --HPweP9NSNDie524OXhnGCaeAB0jAR9jaU--