[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.10 5/9] block: introduce bdrv_try_set_read
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH for-2.10 5/9] block: introduce bdrv_try_set_read_only() |
Date: |
Wed, 5 Apr 2017 20:23:12 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Apr 05, 2017 at 04:28:44PM -0400, John Snow wrote:
>
>
> On 04/05/2017 02:28 PM, Jeff Cody wrote:
> > Introduce try function for setting read_only flags. Will return < 0 on
> > error, with appropriate Error value set. Does not alter any flags.
> >
> > Signed-off-by: Jeff Cody <address@hidden>
> > ---
> > block.c | 14 +++++++++++++-
> > include/block/block.h | 1 +
> > 2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/block.c b/block.c
> > index 8bfe7f4..ad958b9 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -197,7 +197,7 @@ bool bdrv_is_read_only(BlockDriverState *bs)
> > return bs->read_only;
> > }
> >
> > -int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
> > +int bdrv_try_set_read_only(BlockDriverState *bs, bool read_only, Error
> > **errp)
> > {
> > /* Do not set read_only if copy_on_read is enabled */
> > if (bs->copy_on_read && read_only) {
> > @@ -213,6 +213,18 @@ int bdrv_set_read_only(BlockDriverState *bs, bool
> > read_only, Error **errp)
> > return -EPERM;
> > }
> >
> > + return 0;
> > +}
> > +
> > +int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
> > +{
> > + int ret = 0;
> > +
> > + ret = bdrv_try_set_read_only(bs, read_only, errp);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > +
> > bs->read_only = read_only;
> > return 0;
> > }
> > diff --git a/include/block/block.h b/include/block/block.h
> > index beb563a..0049b57 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top,
> > BlockDriverState *base,
> > int64_t sector_num, int nb_sectors, int *pnum);
> >
> > bool bdrv_is_read_only(BlockDriverState *bs);
> > +int bdrv_try_set_read_only(BlockDriverState *bs, bool read_only, Error
> > **errp);
> > int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
> > bool bdrv_is_sg(BlockDriverState *bs);
> > bool bdrv_is_inserted(BlockDriverState *bs);
> >
>
> Maybe I'm just green, but I find it weird that the function is named
> try_x when we aren't actually trying to do anything, we're checking to
> see if we can.
Yeah, not a good name. Maybe bdrv_set_read_only_test() or
bdrv_test_set_read_only()?