Re: [Qemu-devel] [PATCH 3/5] block: added check image option and callback bdrv_is_opened_unclean

[Denis V. Lunev]

On 12/23/2015 12:09 PM, Fam Zheng wrote:
> On Wed, 12/23 10:46, Denis V. Lunev wrote:
> > From: Olga Krishtal <address@hidden>
> >
> > If image is opened for writing and it was not closed correctly
> > (the image is dirty) we have to check and repair it. By default
> > the option is off.
> >
> > bdrv_is_opened_unclean - cheks if the image is dirty
> > This callbsck will be used to ensure that image was
> > closed correctly, and if not - to check and repair it.
> >
> > Signed-off-by: Olga Krishtal <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > CC: Max Reitz <address@hidden>
> > CC: Eric Blake <address@hidden>
> > CC: Fam Zheng <address@hidden>
> > ---
> >   block.c                   | 32 ++++++++++++++++++++++++++++++++
> >   include/block/block.h     |  1 +
> >   include/block/block_int.h |  1 +
> >   3 files changed, 34 insertions(+)
> >
> > diff --git a/block.c b/block.c
> > index 74228b8..1f704f5 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -903,6 +903,12 @@ static QemuOptsList bdrv_runtime_opts = {
> >               .help = "How to lock the image: none or lockfile",
> >               .def_value_str = "none",
> >           },
> > +        {
> > +            .name = "check",
> > +            .type = QEMU_OPT_BOOL,
> > +            .help = "Check and repair the image if it is unclean",
> > +            .def_value_str = "off",
> > +        },
> >           { /* end of list */ }
> >       },
> >   };
> > @@ -1042,6 +1048,16 @@ static int bdrv_open_common(BlockDriverState *bs, 
> > BdrvChild *file,
> >           }
> >       }
> >   
> > +    if (!bs->read_only && qemu_opt_get_bool_del(opts, "check", true) &&
> > +                bdrv_is_opened_unclean(bs) && !(flags | BDRV_O_CHECK)) {
> > +        BdrvCheckResult result = {0};
> > +        ret = bdrv_check(bs, &result, BDRV_FIX_ERRORS);
> > +        if (ret < 0) {
> > +            error_setg_errno(errp, -ret, "Could not repair dirty image");
> > +            bdrv_close(bs);
> > +            goto fail_opts;
> > +        }
> > +    }
> >       if (bs->encrypted) {
> >           error_report("Encrypted images are deprecated");
> >           error_printf("Support for them will be removed in a future release.\n"
> > @@ -4361,3 +4377,19 @@ int bdrv_lock_image(BlockDriverState *bs, BdrvLockImage 
> > lock)
> >       }
> >       return -EOPNOTSUPP;
> >   }
> > +
> > +bool bdrv_is_opened_unclean(BlockDriverState *bs)
> > +{
> > +    BlockDriver *drv = bs->drv;
> > +    if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
> > +        return drv->bdrv_is_opened_unclean(bs);
> > +    }
> > +    if (bs->file == NULL) {
> > +        return false;
> > +    }
> > +    drv = bs->file->bs->drv;
> > +    if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
> > +        return drv->bdrv_is_opened_unclean;
> Should this be
>
>             return drv->bdrv_is_opened_unclean(bs);
>
> ?
>
> (well, I may be wrong in saying this doesn't compile :)
>
> Fam

exactly :)

> > +    }
> > +    return false;
> > +}
> > diff --git a/include/block/block.h b/include/block/block.h
> > index 27fc434..c366990 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -507,6 +507,7 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct 
> > HBitmapIter *hbi);
> >   void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
> >   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> >   int bdrv_lock_image(BlockDriverState *bs, BdrvLockImage lock);
> > +bool bdrv_is_opened_unclean(BlockDriverState *bs);
> >   
> >   void bdrv_enable_copy_on_read(BlockDriverState *bs);
> >   void bdrv_disable_copy_on_read(BlockDriverState *bs);
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 755f342..fc3f4a6 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -137,6 +137,7 @@ struct BlockDriver {
> >       int (*bdrv_make_empty)(BlockDriverState *bs);
> >       int (*bdrv_lock_image)(BlockDriverState *bs, BdrvLockImage lock);
> >   
> > +    bool (*bdrv_is_opened_unclean)(BlockDriverState *bs);
> >       void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
> >   
> >       /* aio */
> > --
> > 2.1.4