From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1US9SS-000392-MR for qemu-devel@nongnu.org; Tue, 16 Apr 2013 13:14:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1US9SR-0007Kp-3B for qemu-devel@nongnu.org; Tue, 16 Apr 2013 13:14:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48330) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1US9SQ-0007Ki-RG for qemu-devel@nongnu.org; Tue, 16 Apr 2013 13:14:15 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3GHEEN7027290 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 16 Apr 2013 13:14:14 -0400 Message-ID: <516D86E4.5030000@redhat.com> Date: Tue, 16 Apr 2013 11:14:12 -0600 From: Eric Blake MIME-Version: 1.0 References: <6e22db8528d4a801af11b37d42a350eab9633636.1366127809.git.phrdina@redhat.com> In-Reply-To: <6e22db8528d4a801af11b37d42a350eab9633636.1366127809.git.phrdina@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2WTOPGBARQHFLDCLNDXPN" Subject: Re: [Qemu-devel] [PATCH 02/11] block: update error reporting for bdrv_snapshot_delete() and related functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2WTOPGBARQHFLDCLNDXPN Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/16/2013 10:05 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > block.c | 22 ++++++++++++++-------- > block/qcow2-snapshot.c | 21 +++++++++++++++------ > block/qcow2.h | 4 +++- > block/rbd.c | 11 ++++++++--- > block/sheepdog.c | 6 ++++-- > include/block/block.h | 4 +++- > include/block/block_int.h | 4 +++- > qemu-img.c | 9 ++++----- > savevm.c | 26 ++++++++++---------------- > 9 files changed, 64 insertions(+), 43 deletions(-) >=20 > @@ -539,7 +541,9 @@ int qcow2_snapshot_delete(BlockDriverState *bs, con= st char *snapshot_id) > /* Search the snapshot */ > snapshot_index =3D find_snapshot_by_id_or_name(bs, snapshot_id); > if (snapshot_index < 0) { > - return -ENOENT; > + error_setg(errp, "qcow2: failed to find snapshot '%s' by id or= name", > + snapshot_id); > + return; I haven't looked if you changed this later in the series to have find_snapshot_by_id_or_name take an errp parameter, at which point you could refactor the error reporting to that point in the stack. That might be a nice followup, but this patch is okay as-is. > +++ b/block/sheepdog.c > @@ -1908,10 +1908,12 @@ out: > return ret; > } > =20 > -static int sd_snapshot_delete(BlockDriverState *bs, const char *snapsh= ot_id) > +static void sd_snapshot_delete(BlockDriverState *bs, > + const char *snapshot_id, > + Error **errp) > { > /* FIXME: Delete specified snapshot id. */ > - return 0; > + return; No semantic change; but a trailing return; in a void function is not necessary, and an empty body would suffice. On the other hand, claiming success when we didn't delete anything feels wrong. Should we change this function to call error_setg() and warn the user that deletion is not supported at this time? If so, that's probably better done as a separate commit. As my findings are best addressed in other patches, I'm okay with this patch as-is, and you can use: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2WTOPGBARQHFLDCLNDXPN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRbYbkAAoJEKeha0olJ0NqYtEH/0/8jl3bFzRB6puxQ8h3fMK7 +cp8anoSr1FCmux1CKH+wYUbtTbu5Yfp9/JGICWUt+TumDOFaGd4xCyP4YmU1rvO Ku1GMFyrfN8z8bRecL5V2ntqmL5jl+RByi+5lExsuYbMdVeOyrUaht8e5VYdfsjD +AJdwTsHOOiFV5OvnJE2CfmDrnDhFPjJ+jTqbVTjtALllsD8exGo6Bkw9pg19W4J 19S1x4c32yyDlDLA5nJ5xMgRMPvOKl+9IsxqzMpVTI0XQMC/bWcVgeAqo838qgtl WrwBQkxjOrZ21VXQtvPaA6ZgmAFk1hDfqC/BBPj6d6+74TMb/7J7p8H/kxhaJRA= =2/4/ -----END PGP SIGNATURE----- ------enig2WTOPGBARQHFLDCLNDXPN--