[Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I

Vladimir Sementsov-Ogievskiy posted 11 patches 4 years, 2 months ago
Only 4 patches received!
There is a newer version of this series
include/block/nbd.h                           |   1 +
include/qapi/error.h                          | 112 ++++++++++++-
block/nbd.c                                   |  49 +++---
hw/9pfs/9p-local.c                            |  12 +-
hw/9pfs/9p.c                                  |   1 +
hw/block/dataplane/xen-block.c                |  17 +-
hw/block/pflash_cfi01.c                       |   7 +-
hw/block/pflash_cfi02.c                       |   7 +-
hw/block/xen-block.c                          | 125 ++++++--------
hw/nvram/fw_cfg.c                             |  14 +-
hw/pci-host/xen_igd_pt.c                      |   7 +-
hw/sd/sdhci-pci.c                             |   7 +-
hw/sd/sdhci.c                                 |  21 +--
hw/sd/ssi-sd.c                                |  26 ++-
hw/tpm/tpm_util.c                             |   7 +-
hw/xen/xen-backend.c                          |   7 +-
hw/xen/xen-bus.c                              | 100 +++++------
hw/xen/xen-host-pci-device.c                  |  27 ++-
hw/xen/xen_pt.c                               |  25 ++-
hw/xen/xen_pt_config_init.c                   |  20 +--
nbd/client.c                                  |   5 +
nbd/server.c                                  |   5 +
tpm.c                                         |   7 +-
scripts/coccinelle/auto-propagated-errp.cocci | 158 ++++++++++++++++++
24 files changed, 500 insertions(+), 267 deletions(-)
create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
[Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Vladimir Sementsov-Ogievskiy 4 years, 2 months ago
Hi all!

v7 is available at
 https://src.openvz.org/scm/~vsementsov/qemu.git #tag up-auto-local-err-partI-v7

Changes v6->v7:

01: - improve commit message
    - fix typo in comment [Eric]
    - add Eric's and Greg's r-b
02: - grammar/wording [Eric]
    - add Eric's and Greg's r-b
03: - improve commit message
    - grammar [Eric]
    - improve script to rename unusual (Error **) parameters
      to errp, and after it switch errp back to be "symbol"
      instead of "identifier"
04: - add Eric's r-b
08: - add Greg's a-b
11: - add Paul's a-b


v6 is available at
 https://src.openvz.org/scm/~vsementsov/qemu.git #tag up-auto-local-err-partI-v6 

Changes v5->v6:
01: use errp name for the parameter, add assertion
02: add a lot of text information, drop Eric's r-b.
    no semantic changes.
03: add more comments
    skip functions with pattern error_append_.*_hint in name
    make errp identifier, to match any name of Error ** paramter
    some other improvements
04: only commit message changed,
    keep Philippe's r-b
05: new, manual update for hw/sd/ssi-sd
06: only commit message changed,
    keep Philippe's r-b
07: only commit message changed,
    keep Philippe's r-b
08: local_parse_opts() changed, so patch changed in this
    function, drop a-b mark
    also, indentation fixed, by improvement in coccinelle script
09: only commit message changed,
    keep Stefan's r-b
10: commit message and a bit of context changed, still seems
    valid to keep Eric's r-b
11: add new hunk: hw/pci-host/xen_igd_pt.c, so, drop r-b
    also, indentation fixed, by improvement in coccinelle script

In these series, there is no commit-per-subsystem script, each generated
commit is generated in separate.

Still, generating commands are very similar, and looks like

    sed -n '/^<Subsystem name>$/,/^$/{s/^F: //p}' MAINTAINERS | \
    xargs git ls-files | grep '\.[hc]$' | \
    xargs spatch \
        --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
        --macro-file scripts/cocci-macro-file.h \
        --in-place --no-show-diff --max-width 80

Note, that in each generated commit, generation command is the only
text, indented by 8 spaces in 'git log -1' output, so, to regenerate all
commits (for example, after rebase, or change in coccinelle script), you
may use the following command:

git rebase -x "sh -c \"git show --pretty= --name-only | xargs git checkout HEAD^ -- ; git reset; git log -1 | grep '^        ' | sh\"" HEAD~7

Which will start automated interactive rebase for generated patches,
which will stop if generated patch changed
(you may do git commit --amend to apply updated generated changes).

Note:
  git show --pretty= --name-only   - lists files, changed in HEAD
  git log -1 | grep '^        ' | sh   - rerun generation command of HEAD


Check for compilation of changed .c files
git rebase -x "sh -c \"git show --pretty= --name-only | sed -n 's/\.c$/.o/p' | xargs make -j9\"" HEAD~7
  

Vladimir Sementsov-Ogievskiy (11):
  qapi/error: add (Error **errp) cleaning APIs
  error: auto propagated local_err
  scripts: add coccinelle script to use auto propagated errp
  hw/sd/ssi-sd: fix error handling in ssi_sd_realize
  SD (Secure Card): introduce ERRP_AUTO_PROPAGATE
  pflash: introduce ERRP_AUTO_PROPAGATE
  fw_cfg: introduce ERRP_AUTO_PROPAGATE
  virtio-9p: introduce ERRP_AUTO_PROPAGATE
  TPM: introduce ERRP_AUTO_PROPAGATE
  nbd: introduce ERRP_AUTO_PROPAGATE
  xen: introduce ERRP_AUTO_PROPAGATE

 include/block/nbd.h                           |   1 +
 include/qapi/error.h                          | 112 ++++++++++++-
 block/nbd.c                                   |  49 +++---
 hw/9pfs/9p-local.c                            |  12 +-
 hw/9pfs/9p.c                                  |   1 +
 hw/block/dataplane/xen-block.c                |  17 +-
 hw/block/pflash_cfi01.c                       |   7 +-
 hw/block/pflash_cfi02.c                       |   7 +-
 hw/block/xen-block.c                          | 125 ++++++--------
 hw/nvram/fw_cfg.c                             |  14 +-
 hw/pci-host/xen_igd_pt.c                      |   7 +-
 hw/sd/sdhci-pci.c                             |   7 +-
 hw/sd/sdhci.c                                 |  21 +--
 hw/sd/ssi-sd.c                                |  26 ++-
 hw/tpm/tpm_util.c                             |   7 +-
 hw/xen/xen-backend.c                          |   7 +-
 hw/xen/xen-bus.c                              | 100 +++++------
 hw/xen/xen-host-pci-device.c                  |  27 ++-
 hw/xen/xen_pt.c                               |  25 ++-
 hw/xen/xen_pt_config_init.c                   |  20 +--
 nbd/client.c                                  |   5 +
 nbd/server.c                                  |   5 +
 tpm.c                                         |   7 +-
 scripts/coccinelle/auto-propagated-errp.cocci | 158 ++++++++++++++++++
 24 files changed, 500 insertions(+), 267 deletions(-)
 create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

CC: Eric Blake <eblake@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Greg Kurz <groug@kaod.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Paul Durrant <paul@xen.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: "Philippe Mathieu-Daudé" <philmd@redhat.com>
CC: Laszlo Ersek <lersek@redhat.com>
CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Stefan Berger <stefanb@linux.ibm.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
CC: qemu-block@nongnu.org
CC: xen-devel@lists.xenproject.org

-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Posted by no-reply@patchew.org 4 years, 2 months ago
Patchew URL: https://patchew.org/QEMU/20200131130118.1716-1-vsementsov@virtuozzo.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Message-id: 20200131130118.1716-1-vsementsov@virtuozzo.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
   9281736..adcd6e9  master     -> master
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200131130118.1716-1-vsementsov@virtuozzo.com -> patchew/20200131130118.1716-1-vsementsov@virtuozzo.com
Switched to a new branch 'test'
b0f4834 xen: introduce ERRP_AUTO_PROPAGATE
c704561 nbd: introduce ERRP_AUTO_PROPAGATE
238b4d2 TPM: introduce ERRP_AUTO_PROPAGATE
56af634 virtio-9p: introduce ERRP_AUTO_PROPAGATE
8dd497a fw_cfg: introduce ERRP_AUTO_PROPAGATE
cd54750 pflash: introduce ERRP_AUTO_PROPAGATE
031d7ed SD (Secure Card): introduce ERRP_AUTO_PROPAGATE
7dc91c5 hw/sd/ssi-sd: fix error handling in ssi_sd_realize
57435c4 scripts: add coccinelle script to use auto propagated errp
0883f29 error: auto propagated local_err
df0db83 qapi/error: add (Error **errp) cleaning APIs

=== OUTPUT BEGIN ===
1/11 Checking commit df0db83cd18a (qapi/error: add (Error **errp) cleaning APIs)
2/11 Checking commit 0883f296ed8f (error: auto propagated local_err)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#139: FILE: include/qapi/error.h:427:
+#define ERRP_AUTO_PROPAGATE()                                  \
+    g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp};  \
+    errp = ((errp == NULL || *errp == error_fatal)             \
+            ? &_auto_errp_prop.local_err : errp)

total: 1 errors, 0 warnings, 101 lines checked

Patch 2/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/11 Checking commit 57435c473bf1 (scripts: add coccinelle script to use auto propagated errp)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
new file mode 100644

total: 0 errors, 1 warnings, 167 lines checked

Patch 3/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/11 Checking commit 7dc91c560263 (hw/sd/ssi-sd: fix error handling in ssi_sd_realize)
5/11 Checking commit 031d7eda4bbb (SD (Secure Card): introduce ERRP_AUTO_PROPAGATE)
6/11 Checking commit cd54750748f4 (pflash: introduce ERRP_AUTO_PROPAGATE)
7/11 Checking commit 8dd497a506bc (fw_cfg: introduce ERRP_AUTO_PROPAGATE)
8/11 Checking commit 56af634941d2 (virtio-9p: introduce ERRP_AUTO_PROPAGATE)
9/11 Checking commit 238b4d2c886f (TPM: introduce ERRP_AUTO_PROPAGATE)
10/11 Checking commit c7045610d28d (nbd: introduce ERRP_AUTO_PROPAGATE)
11/11 Checking commit b0f483460534 (xen: introduce ERRP_AUTO_PROPAGATE)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200131130118.1716-1-vsementsov@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Vladimir Sementsov-Ogievskiy 4 years, 2 months ago
31.01.2020 16:12, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200131130118.1716-1-vsementsov@virtuozzo.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
> Message-id: 20200131130118.1716-1-vsementsov@virtuozzo.com
> Type: series
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
>  From https://github.com/patchew-project/qemu
>     9281736..adcd6e9  master     -> master
>  From https://github.com/patchew-project/qemu
>   * [new tag]         patchew/20200131130118.1716-1-vsementsov@virtuozzo.com -> patchew/20200131130118.1716-1-vsementsov@virtuozzo.com
> Switched to a new branch 'test'
> b0f4834 xen: introduce ERRP_AUTO_PROPAGATE
> c704561 nbd: introduce ERRP_AUTO_PROPAGATE
> 238b4d2 TPM: introduce ERRP_AUTO_PROPAGATE
> 56af634 virtio-9p: introduce ERRP_AUTO_PROPAGATE
> 8dd497a fw_cfg: introduce ERRP_AUTO_PROPAGATE
> cd54750 pflash: introduce ERRP_AUTO_PROPAGATE
> 031d7ed SD (Secure Card): introduce ERRP_AUTO_PROPAGATE
> 7dc91c5 hw/sd/ssi-sd: fix error handling in ssi_sd_realize
> 57435c4 scripts: add coccinelle script to use auto propagated errp
> 0883f29 error: auto propagated local_err
> df0db83 qapi/error: add (Error **errp) cleaning APIs
> 
> === OUTPUT BEGIN ===
> 1/11 Checking commit df0db83cd18a (qapi/error: add (Error **errp) cleaning APIs)
> 2/11 Checking commit 0883f296ed8f (error: auto propagated local_err)
> ERROR: Macros with multiple statements should be enclosed in a do - while loop
> #139: FILE: include/qapi/error.h:427:
> +#define ERRP_AUTO_PROPAGATE()                                  \
> +    g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp};  \
> +    errp = ((errp == NULL || *errp == error_fatal)             \
> +            ? &_auto_errp_prop.local_err : errp)


It worth it.

> 
> total: 1 errors, 0 warnings, 101 lines checked
> 
> Patch 2/11 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 3/11 Checking commit 57435c473bf1 (scripts: add coccinelle script to use auto propagated errp)
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #34:
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 167 lines checked
> 
> Patch 3/11 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 4/11 Checking commit 7dc91c560263 (hw/sd/ssi-sd: fix error handling in ssi_sd_realize)
> 5/11 Checking commit 031d7eda4bbb (SD (Secure Card): introduce ERRP_AUTO_PROPAGATE)
> 6/11 Checking commit cd54750748f4 (pflash: introduce ERRP_AUTO_PROPAGATE)
> 7/11 Checking commit 8dd497a506bc (fw_cfg: introduce ERRP_AUTO_PROPAGATE)
> 8/11 Checking commit 56af634941d2 (virtio-9p: introduce ERRP_AUTO_PROPAGATE)
> 9/11 Checking commit 238b4d2c886f (TPM: introduce ERRP_AUTO_PROPAGATE)
> 10/11 Checking commit c7045610d28d (nbd: introduce ERRP_AUTO_PROPAGATE)
> 11/11 Checking commit b0f483460534 (xen: introduce ERRP_AUTO_PROPAGATE)
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20200131130118.1716-1-vsementsov@virtuozzo.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 


-- 
Best regards,
Vladimir

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Markus Armbruster 4 years, 1 month ago
Hi Vladimir,

I've come to rather like your ERRP_AUTO_PROPAGATE() idea.  What I
wouldn't like is a protracted conversion.

Once we're happy with PATCH 1-3, it's a matter of running Coccinelle and
reviewing its output.  I'm confident we can converge on PATCH 1-3.

It's two weeks until soft freeze.  We need to decide whether to pursue a
partial conversion for 5.0 (basically this series plus the two patches
we identified in review of PATCH 1), or delay until 5.1.  In either
case, I want the conversion to be finished in 5.1.

Please do not feel pressured to make the 5.0 deadline.

I can queue up patches for 5.1 during the freeze.

How would you like to proceed?


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Vladimir Sementsov-Ogievskiy 4 years, 1 month ago
03.03.2020 11:01, Markus Armbruster wrote:
> Hi Vladimir,
> 
> I've come to rather like your ERRP_AUTO_PROPAGATE() idea.  What I
> wouldn't like is a protracted conversion.
> 
> Once we're happy with PATCH 1-3, it's a matter of running Coccinelle and
> reviewing its output.  I'm confident we can converge on PATCH 1-3.
> 
> It's two weeks until soft freeze.  We need to decide whether to pursue a
> partial conversion for 5.0 (basically this series plus the two patches
> we identified in review of PATCH 1), or delay until 5.1.  In either
> case, I want the conversion to be finished in 5.1.
> 
> Please do not feel pressured to make the 5.0 deadline.
> 
> I can queue up patches for 5.1 during the freeze.
> 
> How would you like to proceed?
> 

Hi Markus! Funny coincidence: exactly now (less than 1 hour ago), I've
started working for the next version for these series. So, I'm going to
resend today. Of course, I'd prefer to merge something to 5.0 if at all
possible.


-- 
Best regards,
Vladimir

Re: [Xen-devel] [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Markus Armbruster 4 years, 1 month ago
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> 03.03.2020 11:01, Markus Armbruster wrote:
>> Hi Vladimir,
>>
>> I've come to rather like your ERRP_AUTO_PROPAGATE() idea.  What I
>> wouldn't like is a protracted conversion.
>>
>> Once we're happy with PATCH 1-3, it's a matter of running Coccinelle and
>> reviewing its output.  I'm confident we can converge on PATCH 1-3.
>>
>> It's two weeks until soft freeze.  We need to decide whether to pursue a
>> partial conversion for 5.0 (basically this series plus the two patches
>> we identified in review of PATCH 1), or delay until 5.1.  In either
>> case, I want the conversion to be finished in 5.1.
>>
>> Please do not feel pressured to make the 5.0 deadline.
>>
>> I can queue up patches for 5.1 during the freeze.
>>
>> How would you like to proceed?
>>
>
> Hi Markus! Funny coincidence: exactly now (less than 1 hour ago), I've
> started working for the next version for these series. So, I'm going to
> resend today. Of course, I'd prefer to merge something to 5.0 if at all
> possible.

That was v8, followed by v9.  We're clearly converging.  However, the
soft freeze is tomorrow already.

You've persevered with this idea for quite a while; some impatience
would be quite excusable now.  Still, I doubt part I making 5.0 matters.
The hand-written part is likely to rebase easily, and the generated part
should be regenerated instead of rebased anyway.

What actually matters is *finishing* the job.  What does that take?

* Consensus on the hand-written part.  I think we're basically there, we
  just want to work in a few more tweaks.

* Split the generated part into reviewable batches, regenerating patches
  as necessary.  Solicit review.  First batch is part of this series,
  and v9 looks ready to me.  I assume you'll prepare the remaining
  batches.

* Queue up batches as they become ready, post pull requests.  I can do
  that.

* Update the QAPI code generator to the new Error usage.  I can do that.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v7 00/11] error: auto propagated local_err part I
Posted by Vladimir Sementsov-Ogievskiy 4 years, 1 month ago
16.03.2020 17:40, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> 
>> 03.03.2020 11:01, Markus Armbruster wrote:
>>> Hi Vladimir,
>>>
>>> I've come to rather like your ERRP_AUTO_PROPAGATE() idea.  What I
>>> wouldn't like is a protracted conversion.
>>>
>>> Once we're happy with PATCH 1-3, it's a matter of running Coccinelle and
>>> reviewing its output.  I'm confident we can converge on PATCH 1-3.
>>>
>>> It's two weeks until soft freeze.  We need to decide whether to pursue a
>>> partial conversion for 5.0 (basically this series plus the two patches
>>> we identified in review of PATCH 1), or delay until 5.1.  In either
>>> case, I want the conversion to be finished in 5.1.
>>>
>>> Please do not feel pressured to make the 5.0 deadline.
>>>
>>> I can queue up patches for 5.1 during the freeze.
>>>
>>> How would you like to proceed?
>>>
>>
>> Hi Markus! Funny coincidence: exactly now (less than 1 hour ago), I've
>> started working for the next version for these series. So, I'm going to
>> resend today. Of course, I'd prefer to merge something to 5.0 if at all
>> possible.
> 
> That was v8, followed by v9.  We're clearly converging.  However, the
> soft freeze is tomorrow already.
> 
> You've persevered with this idea for quite a while; some impatience
> would be quite excusable now.  Still, I doubt part I making 5.0 matters.

Not a problem. I hope, I'll resend soon, then it will be up to you.

> The hand-written part is likely to rebase easily, and the generated part
> should be regenerated instead of rebased anyway.
> 
> What actually matters is *finishing* the job.  What does that take?
> 
> * Consensus on the hand-written part.  I think we're basically there, we
>    just want to work in a few more tweaks.

I'll resend today, most probably it would be your version of coccinelle (if I will not find real sense in fn inheritance).

> 
> * Split the generated part into reviewable batches, regenerating patches
>    as necessary.  Solicit review.  First batch is part of this series,
>    and v9 looks ready to me.  I assume you'll prepare the remaining
>    batches.

Yes I will. This is the reward for this work: send one hundred generated patches :))

> 
> * Queue up batches as they become ready, post pull requests.  I can do
>    that.
> 
> * Update the QAPI code generator to the new Error usage.  I can do that.
> 

Great!

-- 
Best regards,
Vladimir