From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VPUiw-0006Wn-RA for qemu-devel@nongnu.org; Fri, 27 Sep 2013 05:52:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VPUir-0007mz-QW for qemu-devel@nongnu.org; Fri, 27 Sep 2013 05:52:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60124) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VPUir-0007mv-HZ for qemu-devel@nongnu.org; Fri, 27 Sep 2013 05:52:29 -0400 Date: Fri, 27 Sep 2013 11:52:20 +0200 From: Kevin Wolf Message-ID: <20130927095220.GG2440@dhcp-200-207.str.redhat.com> References: <1380206029-9740-1-git-send-email-benoit@irqsave.net> <1380206029-9740-2-git-send-email-benoit@irqsave.net> <20130926164813.GA29688@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20130926164813.GA29688@localhost.localdomain> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2] block: Add BlockDriver.bdrv_check_ext_snapshot. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , stefanha@redhat.com, qemu-devel@nongnu.org Am 26.09.2013 um 18:48 hat Jeff Cody geschrieben: > On Thu, Sep 26, 2013 at 04:33:49PM +0200, Beno=EEt Canet wrote: > > This field is used by blkverify to disable external snapshots creatio= n. > > I will also be used by block filters like quorum to disable external = snapshots > > creation. > >=20 > > Signed-off-by: Benoit Canet > > --- > > block.c | 14 ++++++++++++++ > > block/blkverify.c | 2 ++ > > blockdev.c | 5 +++++ > > include/block/block.h | 7 +++++++ > > include/block/block_int.h | 8 ++++++++ > > 5 files changed, 36 insertions(+) > >=20 > > diff --git a/block.c b/block.c > > index 4833b37..4da6fd9 100644 > > --- a/block.c > > +++ b/block.c > > @@ -4632,3 +4632,17 @@ int bdrv_amend_options(BlockDriverState *bs, Q= EMUOptionParameter *options) > > } > > return bs->drv->bdrv_amend_options(bs, options); > > } > > + > > +bool bdrv_check_ext_snapshot(BlockDriverState *bs) > > +{ > > + /* external snashots are enabled by defaults */ > > + if (!bs->drv->bdrv_check_ext_snapshot) { > > + return true; > > + } > > + return bs->drv->bdrv_check_ext_snapshot(bs); > > +} > > + > > +bool bdrv_forbid_ext_snapshot(BlockDriverState *bs) > > +{ > > + return false; > > +} >=20 > The only problem I have with this now, is that > "bdrv_forbid_ext_snapshot()" returns false, to indicate that "forbid > ext snapshot" is true. Looking at the function above, I would come to > the opposite conclusion as to what it does. >=20 > I understand why - you want the function name assigned to > .bdrv_check_ext_snapshot to reflect the action, but then that causes > the boolean return to be misleading. Maybe returning an enum would be > more natural? >=20 > I apologize if this seems too pedantic. :) Perhaps rename the function to bdrv_check_ext_snapshot_forbidden() or something like that? Kevin