| Message ID | 1331112179-12726-3-git-send-email-wdongxu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 07, 2012 at 05:22:58PM +0800, Dong Xu Wang wrote: > From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > > Some block drivers can verify their image files are clean or not. So we can show > it while using "qemu-img info. > > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > --- > block.c | 14 ++++++++++++++ > block.h | 2 ++ > block_int.h | 1 + > qemu-img.c | 3 +++ > 4 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/block.c b/block.c > index 947607b..17e9ba8 100644 > --- a/block.c > +++ b/block.c > @@ -193,6 +193,20 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, > qemu_co_queue_next(&bs->throttled_reqs); > } > > +/* check if the image was cleanly shut down */ > +bool bdrv_not_cleanly_down(BlockDriverState *bs) The name is a little cryptic to me and I suggest avoiding 'not' in function names because it easily leads to double-negatives (!not_foo()). How about: bool bdrv_was_shutdown_cleanly() if (!bdrv_was_shutdown_cleanly(bs)) { printf(...); } This patch and the QED patch look fine otherwise. Stefan
Am 12.03.2012 19:18, schrieb Stefan Hajnoczi: > On Wed, Mar 07, 2012 at 05:22:58PM +0800, Dong Xu Wang wrote: >> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >> >> Some block drivers can verify their image files are clean or not. So we can show >> it while using "qemu-img info. >> >> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >> --- >> block.c | 14 ++++++++++++++ >> block.h | 2 ++ >> block_int.h | 1 + >> qemu-img.c | 3 +++ >> 4 files changed, 20 insertions(+), 0 deletions(-) >> >> diff --git a/block.c b/block.c >> index 947607b..17e9ba8 100644 >> --- a/block.c >> +++ b/block.c >> @@ -193,6 +193,20 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, >> qemu_co_queue_next(&bs->throttled_reqs); >> } >> >> +/* check if the image was cleanly shut down */ >> +bool bdrv_not_cleanly_down(BlockDriverState *bs) > > The name is a little cryptic to me and I suggest avoiding 'not' in > function names because it easily leads to double-negatives (!not_foo()). > > How about: > > bool bdrv_was_shutdown_cleanly() > > if (!bdrv_was_shutdown_cleanly(bs)) { > printf(...); > } > > This patch and the QED patch look fine otherwise. Should we rather add a new field to BlockDriverInfo and use the existing bdrv_get_info() function? Kevin
On Tue, Mar 13, 2012 at 10:03:51AM +0100, Kevin Wolf wrote: > Am 12.03.2012 19:18, schrieb Stefan Hajnoczi: > > On Wed, Mar 07, 2012 at 05:22:58PM +0800, Dong Xu Wang wrote: > >> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > >> > >> Some block drivers can verify their image files are clean or not. So we can show > >> it while using "qemu-img info. > >> > >> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > >> --- > >> block.c | 14 ++++++++++++++ > >> block.h | 2 ++ > >> block_int.h | 1 + > >> qemu-img.c | 3 +++ > >> 4 files changed, 20 insertions(+), 0 deletions(-) > >> > >> diff --git a/block.c b/block.c > >> index 947607b..17e9ba8 100644 > >> --- a/block.c > >> +++ b/block.c > >> @@ -193,6 +193,20 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, > >> qemu_co_queue_next(&bs->throttled_reqs); > >> } > >> > >> +/* check if the image was cleanly shut down */ > >> +bool bdrv_not_cleanly_down(BlockDriverState *bs) > > > > The name is a little cryptic to me and I suggest avoiding 'not' in > > function names because it easily leads to double-negatives (!not_foo()). > > > > How about: > > > > bool bdrv_was_shutdown_cleanly() > > > > if (!bdrv_was_shutdown_cleanly(bs)) { > > printf(...); > > } > > > > This patch and the QED patch look fine otherwise. > > Should we rather add a new field to BlockDriverInfo and use the existing > bdrv_get_info() function? Yeah, that sounds good. In that case it's best to make the "clean" value false and the "dirty" value true, so that block drivers that don't care about this feature automatically report "clean". struct BlockDriverInfo { bool is_dirty; } Stefan
Am 13.03.2012 10:33, schrieb Stefan Hajnoczi: > On Tue, Mar 13, 2012 at 10:03:51AM +0100, Kevin Wolf wrote: >> Am 12.03.2012 19:18, schrieb Stefan Hajnoczi: >>> On Wed, Mar 07, 2012 at 05:22:58PM +0800, Dong Xu Wang wrote: >>>> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >>>> >>>> Some block drivers can verify their image files are clean or not. So we can show >>>> it while using "qemu-img info. >>>> >>>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >>>> --- >>>> block.c | 14 ++++++++++++++ >>>> block.h | 2 ++ >>>> block_int.h | 1 + >>>> qemu-img.c | 3 +++ >>>> 4 files changed, 20 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index 947607b..17e9ba8 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -193,6 +193,20 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, >>>> qemu_co_queue_next(&bs->throttled_reqs); >>>> } >>>> >>>> +/* check if the image was cleanly shut down */ >>>> +bool bdrv_not_cleanly_down(BlockDriverState *bs) >>> >>> The name is a little cryptic to me and I suggest avoiding 'not' in >>> function names because it easily leads to double-negatives (!not_foo()). >>> >>> How about: >>> >>> bool bdrv_was_shutdown_cleanly() >>> >>> if (!bdrv_was_shutdown_cleanly(bs)) { >>> printf(...); >>> } >>> >>> This patch and the QED patch look fine otherwise. >> >> Should we rather add a new field to BlockDriverInfo and use the existing >> bdrv_get_info() function? > > Yeah, that sounds good. In that case it's best to make the "clean" > value false and the "dirty" value true, so that block drivers that don't > care about this feature automatically report "clean". > > struct BlockDriverInfo { > bool is_dirty; > } Yes, I agree. Kevin
diff --git a/block.c b/block.c index 947607b..17e9ba8 100644 --- a/block.c +++ b/block.c @@ -193,6 +193,20 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, qemu_co_queue_next(&bs->throttled_reqs); } +/* check if the image was cleanly shut down */ +bool bdrv_not_cleanly_down(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + + if (!drv) { + return 0; + } + if (!drv->bdrv_not_cleanly_down) { + return 0; + } + return drv->bdrv_not_cleanly_down(bs); +} + /* check if the path starts with "<protocol>:" */ static int path_has_protocol(const char *path) { diff --git a/block.h b/block.h index d76d386..00dc2a5 100644 --- a/block.h +++ b/block.h @@ -110,6 +110,8 @@ void bdrv_io_limits_enable(BlockDriverState *bs); void bdrv_io_limits_disable(BlockDriverState *bs); bool bdrv_io_limits_enabled(BlockDriverState *bs); +bool bdrv_not_cleanly_down(BlockDriverState *bs); + void bdrv_init(void); void bdrv_init_with_whitelist(void); BlockDriver *bdrv_find_protocol(const char *filename); diff --git a/block_int.h b/block_int.h index 339a5ac..e28787c 100644 --- a/block_int.h +++ b/block_int.h @@ -114,6 +114,7 @@ struct BlockDriver { int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); int (*bdrv_set_key)(BlockDriverState *bs, const char *key); int (*bdrv_make_empty)(BlockDriverState *bs); + bool (*bdrv_not_cleanly_down)(BlockDriverState *bs); /* aio */ BlockDriverAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, diff --git a/qemu-img.c b/qemu-img.c index 17731a9..c84527b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1122,6 +1122,9 @@ static int img_info(int argc, char **argv) if (bdrv_is_encrypted(bs)) { printf("encrypted: yes\n"); } + if (bdrv_not_cleanly_down(bs)) { + printf("cleanly shut down: no\n"); + } if (bdrv_get_info(bs, &bdi) >= 0) { if (bdi.cluster_size != 0) { printf("cluster_size: %d\n", bdi.cluster_size);