Re: [PATCH 10/14] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface

[Alberto Garcia]

On Wed 09 Sep 2020 08:59:26 PM CEST, Vladimir Sementsov-Ogievskiy 
<vsementsov@virtuozzo.com> wrote:
> -/* qcow2_load_dirty_bitmaps()
> - * Return value is a hint for caller: true means that the Qcow2 header was
> - * updated. (false doesn't mean that the header should be updated by the
> - * caller, it just means that updating was not needed or the image cannot be
> - * written to).
> - * On failure the function returns false.
> - */
> -bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
> +/* Return true on success, false on failure. */
> +bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated,
> +                              Error **errp)

I think that the documentation should clarify under what conditions
'header_updated' is modified.

>      if (s->nb_bitmaps == 0) {
>          /* No bitmaps - nothing to do */
> -        return false;
> +        return true;
>      }

Here is it not for example (should it be set to false?).

> -    if (bm_list == NULL) {
> +    if (!bm_list) {
>          return false;
>      }

This looks like a cosmetic change unrelated to the rest of the patch.

Berto