Re: [PATCH v4 23/31] block: Fix error_append_hint/error_prepend usage

[Eric Blake]

On 10/1/19 10:53 AM, Vladimir Sementsov-Ogievskiy wrote:
> If we want to add some info to errp (by error_prepend() or
> error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
> Otherwise, this info will not be added when errp == &fatal_err
> (the program will exit prior to the error_append_hint() or
> error_prepend() call).  Fix such cases.
>
> This commit (together with its neighbors) was generated by
>
> git grep -l 'error_\(append_hint\|prepend\)(errp' | while read f; do \
> spatch --sp-file scripts/coccinelle/fix-error-add-info.cocci \
> --in-place $f; done
>
> and then
>
> ./python/commit-per-subsystem.py MAINTAINERS "$(< auto-msg)"
>
> (auto-msg was a file with this commit message)
>
> and then by hand, for not maintained changed files:
>
> git commit -m "<SUB-SYSTEM>: $(< auto-msg)" <FILES>
>
> Still, for backporting it may be more comfortable to use only the first
> command and then do one huge commit.
>
> Reported-by: Greg Kurz <address@hidden>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   include/block/nbd.h  | 1 +
>   block.c              | 3 +++
>   block/backup.c       | 1 +
>   block/dirty-bitmap.c | 1 +
>   block/file-posix.c   | 4 ++++
>   block/gluster.c      | 2 ++
>   block/qcow.c         | 1 +
>   block/qcow2-bitmap.c | 1 +
>   block/qcow2.c        | 3 +++
>   block/vdi.c          | 1 +
>   block/vhdx-log.c     | 1 +
>   block/vmdk.c         | 1 +
>   block/vpc.c          | 1 +
>   13 files changed, 21 insertions(+)

The addition of error_prepend() checking makes this patch larger than in v3.

But similar to v3, I was able to come up with a matching grep query:

$ git grep -np 'error_\(append_hint\|prepend\)(errp' \
   block.c block/ include/block/ \
  | grep '\.[ch]=' | wc -l
22

and see that grep found one more instance than coccinelle.  Looking 
closer, I see where my diff found a spot you missed:

block/qcow2-bitmap.c=1448=void 
qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
block/qcow2-bitmap.c=1671=fail:

[Ouch - there's a REASON that emacs prefers indenting labels at column 
'2, 5, 9, 13, ...' rather than '1, 5, 9, 13' - that's because anything 
at column 1 messes up determination of 'grep -p' for which function owns 
the code, showing the label at 1671 instead of the function 
qcow2_can_store_new_dirty_bitmap at 1618 - but that's a side point]


> +++ b/block/qcow2-bitmap.c
> @@ -1618,6 +1618,7 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState 
> *bs,
>                                         uint32_t granularity,
>                                         Error **errp)
>   {
> +    ERRP_AUTO_PROPAGATE();
>       BDRVQcow2State *s = bs->opaque;
>       bool found;
>       Qcow2BitmapList *bm_list;
> diff --git a/block/qcow2.c b/block/qcow2.c

Your patch is missing a patch to qcow2_store_persistent_dirty_bitmaps(), 
which calls error_prepend(errp, ...).  However, when I manually ran the 
same spatch command line, I also got the same failure to include a fix 
in that function.

I'm not sure what's wrong with the .cocci script to miss that instance; 
I've tried fiddling around with the .cocci file to see if I can spot any 
change to make (for example, using ... instead of <+...), but so far, 
with no success at getting the second function patched.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org