Re: [Qemu-devel] [PATCH 3/4 v2 RESEND] block: add dirty flag status to qemu-img

[Kevin Wolf]

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 <address@hidden>
>>>>
>>>> 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 <address@hidden>
>>>> ---
>>>>  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