RE: [PATCH] qcow2: Move error check of local_err near its assignment

[Tuguoyi <tu.guoyi@h3c.com>]

On 18.12.2019 17:33 Kevin Wolf wrote:
> 
> Am 18.12.2019 um 03:26 hat Tuguoyi geschrieben:
> >
> > Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
> 
> Empty commit messages are rarely acceptable. You should explain here why
> you are making the change and why it's correct (for example by comparing
> with when it was introduced).
> 
> In this case, the local_err check outside of the if block was necessary when it
> was introduced in commit d1258dd0c87 because it needed to be executed
> even if qcow2_load_autoloading_dirty_bitmaps() returned false.
> 
> After some modifications that all required the error check to remain where it
> is, commit 9c98f145dfb finally moved the
> qcow2_load_dirty_bitmaps() call into the if block, so now the error check
> should be there, too.
> 
> This is information that should be in the commit message rather than
> requiring each reader to do the research.
> 
> Please also make sure to CC the author of the code that you're modifying, in
> this case Vladimir.
> 
> > diff --git a/block/qcow2.c b/block/qcow2.c index 7c18721..ce3db29
> > 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1705,14 +1705,14 @@ static int coroutine_fn
> qcow2_do_open(BlockDriverState *bs, QDict *options,
> >      if (!(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) {
> >          /* It's case 1, 2 or 3.2. Or 3.1 which is BUG in management layer.
> */
> >          bool header_updated = qcow2_load_dirty_bitmaps(bs,
> > &local_err);
> > +        if (local_err != NULL) {
> > +            error_propagate(errp, local_err);
> > +            ret = -EINVAL;
> > +            goto fail;
> > +        }
> >
> >          update_header = update_header && !header_updated;
> >      }
> > -    if (local_err != NULL) {
> > -        error_propagate(errp, local_err);
> > -        ret = -EINVAL;
> > -        goto fail;
> > -    }
> >
> >      if (update_header) {
> >          ret = qcow2_update_header(bs);
> 
> The change itself looks good to me, but I'll let Vladimir have a look as well. If
> there are no more comments, I'm looking forward to a v2 patch with a
> non-empty commit message.
> 
> Kevin

Thanks for pointing it out, I will send the v2 patch

--
Best regards,
Guoyi