mbox series

[v3,0/7] QOM'ify PIIX southbridge creation

Message ID 20220528192057.30910-1-shentey@gmail.com (mailing list archive)
Headers show
Series QOM'ify PIIX southbridge creation | expand

Message

Bernhard Beschow May 28, 2022, 7:20 p.m. UTC
v3:
* Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
* Use embedded structs for touched PCI devices (Mark)
* Fix piix4's rtc embedded struct to be initialized by
  object_initialize_child() (Peter) [2]

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.


v2:
* Preserve `DeviceState *` as return value of piix4_create() (Mark)
* Aggregate all type name movements into first commit (Mark)
* Have piix4 southbridge rather than malta board instantiate piix4 pm (me)

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
Modify pci_piix3_realize() to start with
    error_setg(errp, "This is a test");
Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom 
archlinux-2022.05.01-x86_64.iso`.
Result: qemu-system-x86_64 aborts with: "This is a test"


v1:
The piix3 and piix4 southbridge devices still rely on create() functions which
are deprecated. This series resolves these functions piece by piece to
modernize the code.

Both devices are modified in lockstep where possible to provide more context.

Testing done:
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html

Bernhard Beschow (7):
  include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
  hw/isa/piix4: Use object_initialize_child() for embedded struct
  hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
  hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
  hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
  hw/isa/piix4: QOM'ify PIIX4 PM creation
  hw/isa/piix{3,4}: Inline and remove create() functions

 hw/i386/pc_piix.c             |   7 +-
 hw/isa/piix3.c                |  98 ++++++++++++++-------------
 hw/isa/piix4.c                | 120 +++++++++++++++++-----------------
 hw/mips/malta.c               |   7 +-
 include/hw/isa/isa.h          |   2 -
 include/hw/southbridge/piix.h |   6 +-
 6 files changed, 127 insertions(+), 113 deletions(-)

Comments

Mark Cave-Ayland May 29, 2022, 9:46 a.m. UTC | #1
On 28/05/2022 20:20, Bernhard Beschow wrote:

> v3:
> * Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
> * Use embedded structs for touched PCI devices (Mark)
> * Fix piix4's rtc embedded struct to be initialized by
>    object_initialize_child() (Peter) [2]
> 
> Testing done:
> 
> 1)
> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> Result: All pass.
> 
> 2)
> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
> 
> In both cases the system booted successfully and it was possible to shut down
> the system using the `poweroff` command.
> 
> 
> v2:
> * Preserve `DeviceState *` as return value of piix4_create() (Mark)
> * Aggregate all type name movements into first commit (Mark)
> * Have piix4 southbridge rather than malta board instantiate piix4 pm (me)
> 
> Testing done:
> 
> 1)
> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> Result: All pass.
> 
> 2)
> Modify pci_piix3_realize() to start with
>      error_setg(errp, "This is a test");
> Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
> archlinux-2022.05.01-x86_64.iso`.
> Result: qemu-system-x86_64 aborts with: "This is a test"
> 
> 
> v1:
> The piix3 and piix4 southbridge devices still rely on create() functions which
> are deprecated. This series resolves these functions piece by piece to
> modernize the code.
> 
> Both devices are modified in lockstep where possible to provide more context.
> 
> Testing done:
> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
> 
> In both cases the system booted successfully and it was possible to shut down
> the system using the `poweroff` command.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
> 
> Bernhard Beschow (7):
>    include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
>    hw/isa/piix4: Use object_initialize_child() for embedded struct
>    hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
>    hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
>    hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
>    hw/isa/piix4: QOM'ify PIIX4 PM creation
>    hw/isa/piix{3,4}: Inline and remove create() functions
> 
>   hw/i386/pc_piix.c             |   7 +-
>   hw/isa/piix3.c                |  98 ++++++++++++++-------------
>   hw/isa/piix4.c                | 120 +++++++++++++++++-----------------
>   hw/mips/malta.c               |   7 +-
>   include/hw/isa/isa.h          |   2 -
>   include/hw/southbridge/piix.h |   6 +-
>   6 files changed, 127 insertions(+), 113 deletions(-)

Hi Bernhard,

I've spotted a couple of small things, but once those are fixed this series looks 
good to me so I'm happy to give a:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Thanks for your patience with these series too: you've done some good work here, 
however patchsets like this can sometimes take a while to get reviewed because they 
both i) touch legacy code/APIs and ii) cut across multiple machines and maintainers. 
I'd really like to get this work, along with your RTC updates, merged soon as it is a 
great improvement.

One reason that you may not get many reviews is because you've not added the relevant 
maintainers on To/CC. Due to the large volume of emails on qemu-devel, quite a few 
maintainers will filter based upon their own email address so it is definitely worth 
adding them in.

Fortunately you can easily find the relevant maintainer email addresses by running 
"./scripts/get_maintainer.pl <path-to-git-patch-dir>" on your git format-patch 
directory. I'd recommend doing this, and also dropping qemu-trivial since I would say 
these patches are now beyond the trivial threshold.


ATB,

Mark.
Mark Cave-Ayland May 29, 2022, 10:06 a.m. UTC | #2
On 29/05/2022 10:46, Mark Cave-Ayland wrote:

> On 28/05/2022 20:20, Bernhard Beschow wrote:
> 
>> v3:
>> * Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
>> * Use embedded structs for touched PCI devices (Mark)
>> * Fix piix4's rtc embedded struct to be initialized by
>>    object_initialize_child() (Peter) [2]
>>
>> Testing done:
>>
>> 1)
>> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
>> Result: All pass.
>>
>> 2)
>> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
>> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
>>
>> In both cases the system booted successfully and it was possible to shut down
>> the system using the `poweroff` command.
>>
>>
>> v2:
>> * Preserve `DeviceState *` as return value of piix4_create() (Mark)
>> * Aggregate all type name movements into first commit (Mark)
>> * Have piix4 southbridge rather than malta board instantiate piix4 pm (me)
>>
>> Testing done:
>>
>> 1)
>> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
>> Result: All pass.
>>
>> 2)
>> Modify pci_piix3_realize() to start with
>>      error_setg(errp, "This is a test");
>> Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
>> archlinux-2022.05.01-x86_64.iso`.
>> Result: qemu-system-x86_64 aborts with: "This is a test"
>>
>>
>> v1:
>> The piix3 and piix4 southbridge devices still rely on create() functions which
>> are deprecated. This series resolves these functions piece by piece to
>> modernize the code.
>>
>> Both devices are modified in lockstep where possible to provide more context.
>>
>> Testing done:
>> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
>> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
>>
>> In both cases the system booted successfully and it was possible to shut down
>> the system using the `poweroff` command.
>>
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
>> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
>>
>> Bernhard Beschow (7):
>>    include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
>>    hw/isa/piix4: Use object_initialize_child() for embedded struct
>>    hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
>>    hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
>>    hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
>>    hw/isa/piix4: QOM'ify PIIX4 PM creation
>>    hw/isa/piix{3,4}: Inline and remove create() functions
>>
>>   hw/i386/pc_piix.c             |   7 +-
>>   hw/isa/piix3.c                |  98 ++++++++++++++-------------
>>   hw/isa/piix4.c                | 120 +++++++++++++++++-----------------
>>   hw/mips/malta.c               |   7 +-
>>   include/hw/isa/isa.h          |   2 -
>>   include/hw/southbridge/piix.h |   6 +-
>>   6 files changed, 127 insertions(+), 113 deletions(-)
> 
> Hi Bernhard,
> 
> I've spotted a couple of small things, but once those are fixed this series looks 
> good to me so I'm happy to give a:
> 
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Thanks for your patience with these series too: you've done some good work here, 
> however patchsets like this can sometimes take a while to get reviewed because they 
> both i) touch legacy code/APIs and ii) cut across multiple machines and maintainers. 
> I'd really like to get this work, along with your RTC updates, merged soon as it is a 
> great improvement.
> 
> One reason that you may not get many reviews is because you've not added the relevant 
> maintainers on To/CC. Due to the large volume of emails on qemu-devel, quite a few 
> maintainers will filter based upon their own email address so it is definitely worth 
> adding them in.
> 
> Fortunately you can easily find the relevant maintainer email addresses by running 
> "./scripts/get_maintainer.pl <path-to-git-patch-dir>" on your git format-patch 
> directory. I'd recommend doing this, and also dropping qemu-trivial since I would say 
> these patches are now beyond the trivial threshold.

Oh wait - I see now it's just the cover letter which is missing the additional 
maintainer addresses :)  If you could add them into the cover letter for your next 
revision that would be great, since it gives context for maintainers to help with the 
review process.


ATB,

Mark.
Bernhard Beschow May 29, 2022, 1:02 p.m. UTC | #3
On Sun, May 29, 2022 at 12:06 PM Mark Cave-Ayland <
mark.cave-ayland@ilande.co.uk> wrote:

> On 29/05/2022 10:46, Mark Cave-Ayland wrote:
>
> > On 28/05/2022 20:20, Bernhard Beschow wrote:
> >
> >> v3:
> >> * Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function'
> (Mark) [1]
> >> * Use embedded structs for touched PCI devices (Mark)
> >> * Fix piix4's rtc embedded struct to be initialized by
> >>    object_initialize_child() (Peter) [2]
> >>
> >> Testing done:
> >>
> >> 1)
> >> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> >> Result: All pass.
> >>
> >> 2)
> >> * `qemu-system-x86_64 -M pc -m 2G -cdrom
> archlinux-2022.05.01-x86_64.iso`
> >> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
> >>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1
> console=tty0"`
> >>
> >> In both cases the system booted successfully and it was possible to
> shut down
> >> the system using the `poweroff` command.
> >>
> >>
> >> v2:
> >> * Preserve `DeviceState *` as return value of piix4_create() (Mark)
> >> * Aggregate all type name movements into first commit (Mark)
> >> * Have piix4 southbridge rather than malta board instantiate piix4 pm
> (me)
> >>
> >> Testing done:
> >>
> >> 1)
> >> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
> >> Result: All pass.
> >>
> >> 2)
> >> Modify pci_piix3_realize() to start with
> >>      error_setg(errp, "This is a test");
> >> Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
> >> archlinux-2022.05.01-x86_64.iso`.
> >> Result: qemu-system-x86_64 aborts with: "This is a test"
> >>
> >>
> >> v1:
> >> The piix3 and piix4 southbridge devices still rely on create()
> functions which
> >> are deprecated. This series resolves these functions piece by piece to
> >> modernize the code.
> >>
> >> Both devices are modified in lockstep where possible to provide more
> context.
> >>
> >> Testing done:
> >> * `qemu-system-x86_64 -M pc -m 2G -cdrom
> archlinux-2022.05.01-x86_64.iso`
> >> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
> >>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1
> console=tty0"`
> >>
> >> In both cases the system booted successfully and it was possible to
> shut down
> >> the system using the `poweroff` command.
> >>
> >> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
> >> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
> >>
> >> Bernhard Beschow (7):
> >>    include/hw/southbridge/piix: Aggregate all PIIX soughbridge type
> names
> >>    hw/isa/piix4: Use object_initialize_child() for embedded struct
> >>    hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
> >>    hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
> >>    hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
> >>    hw/isa/piix4: QOM'ify PIIX4 PM creation
> >>    hw/isa/piix{3,4}: Inline and remove create() functions
> >>
> >>   hw/i386/pc_piix.c             |   7 +-
> >>   hw/isa/piix3.c                |  98 ++++++++++++++-------------
> >>   hw/isa/piix4.c                | 120 +++++++++++++++++-----------------
> >>   hw/mips/malta.c               |   7 +-
> >>   include/hw/isa/isa.h          |   2 -
> >>   include/hw/southbridge/piix.h |   6 +-
> >>   6 files changed, 127 insertions(+), 113 deletions(-)
> >
> > Hi Bernhard,
> >
> > I've spotted a couple of small things, but once those are fixed this
> series looks
> > good to me so I'm happy to give a:
> >
> > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >
> > Thanks for your patience with these series too: you've done some good
> work here,
> > however patchsets like this can sometimes take a while to get reviewed
> because they
> > both i) touch legacy code/APIs and ii) cut across multiple machines and
> maintainers.
> > I'd really like to get this work, along with your RTC updates, merged
> soon as it is a
> > great improvement.
> >
> > One reason that you may not get many reviews is because you've not added
> the relevant
> > maintainers on To/CC. Due to the large volume of emails on qemu-devel,
> quite a few
> > maintainers will filter based upon their own email address so it is
> definitely worth
> > adding them in.
> >
> > Fortunately you can easily find the relevant maintainer email addresses
> by running
> > "./scripts/get_maintainer.pl <path-to-git-patch-dir>" on your git
> format-patch
> > directory. I'd recommend doing this, and also dropping qemu-trivial
> since I would say
> > these patches are now beyond the trivial threshold.
>
> Oh wait - I see now it's just the cover letter which is missing the
> additional
> maintainer addresses :)  If you could add them into the cover letter for
> your next
> revision that would be great, since it gives context for maintainers to
> help with the
> review process.
>

Hi Mark,

Thanks for your great work you put into reviews and the useful insights! It
seems to me that the time it takes for patches to be queued depends on the
subsystem - some are faster, some are slower...

I've automated my setup as described in [1]. However, it doesn't seem to
work for the cover letter which I'd expect to be sent to the union of all
reviewers of all patches. Any idea how to fix this?

Best regards,
Bernhard

[1]
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer

>
>
> ATB,
>
> Mark.
>
Mark Cave-Ayland May 30, 2022, 7:11 p.m. UTC | #4
On 29/05/2022 14:02, Bernhard Beschow wrote:

> On Sun, May 29, 2022 at 12:06 PM Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk 
> <mailto:mark.cave-ayland@ilande.co.uk>> wrote:
> 
>     On 29/05/2022 10:46, Mark Cave-Ayland wrote:
> 
>      > On 28/05/2022 20:20, Bernhard Beschow wrote:
>      >
>      >> v3:
>      >> * Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
>      >> * Use embedded structs for touched PCI devices (Mark)
>      >> * Fix piix4's rtc embedded struct to be initialized by
>      >>    object_initialize_child() (Peter) [2]
>      >>
>      >> Testing done:
>      >>
>      >> 1)
>      >> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
>      >> Result: All pass.
>      >>
>      >> 2)
>      >> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
>      >> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>      >>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
>      >>
>      >> In both cases the system booted successfully and it was possible to shut down
>      >> the system using the `poweroff` command.
>      >>
>      >>
>      >> v2:
>      >> * Preserve `DeviceState *` as return value of piix4_create() (Mark)
>      >> * Aggregate all type name movements into first commit (Mark)
>      >> * Have piix4 southbridge rather than malta board instantiate piix4 pm (me)
>      >>
>      >> Testing done:
>      >>
>      >> 1)
>      >> `make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
>      >> Result: All pass.
>      >>
>      >> 2)
>      >> Modify pci_piix3_realize() to start with
>      >>      error_setg(errp, "This is a test");
>      >> Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
>      >> archlinux-2022.05.01-x86_64.iso`.
>      >> Result: qemu-system-x86_64 aborts with: "This is a test"
>      >>
>      >>
>      >> v1:
>      >> The piix3 and piix4 southbridge devices still rely on create() functions which
>      >> are deprecated. This series resolves these functions piece by piece to
>      >> modernize the code.
>      >>
>      >> Both devices are modified in lockstep where possible to provide more context.
>      >>
>      >> Testing done:
>      >> * `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
>      >> * `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
>      >>    debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`
>      >>
>      >> In both cases the system booted successfully and it was possible to shut down
>      >> the system using the `poweroff` command.
>      >>
>      >> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
>     <https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html>
>      >> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html
>     <https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html>
>      >>
>      >> Bernhard Beschow (7):
>      >>    include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names
>      >>    hw/isa/piix4: Use object_initialize_child() for embedded struct
>      >>    hw/isa/piix{3,4}: Move pci_map_irq_fn's near pci_set_irq_fn's
>      >>    hw/isa/piix{3,4}: QOM'ify PCI device creation and wiring
>      >>    hw/isa/piix{3,4}: Factor out ISABus retrieval from create() functions
>      >>    hw/isa/piix4: QOM'ify PIIX4 PM creation
>      >>    hw/isa/piix{3,4}: Inline and remove create() functions
>      >>
>      >>   hw/i386/pc_piix.c             |   7 +-
>      >>   hw/isa/piix3.c                |  98 ++++++++++++++-------------
>      >>   hw/isa/piix4.c                | 120 +++++++++++++++++-----------------
>      >>   hw/mips/malta.c               |   7 +-
>      >>   include/hw/isa/isa.h          |   2 -
>      >>   include/hw/southbridge/piix.h |   6 +-
>      >>   6 files changed, 127 insertions(+), 113 deletions(-)
>      >
>      > Hi Bernhard,
>      >
>      > I've spotted a couple of small things, but once those are fixed this series looks
>      > good to me so I'm happy to give a:
>      >
>      > Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk
>     <mailto:mark.cave-ayland@ilande.co.uk>>
>      >
>      > Thanks for your patience with these series too: you've done some good work here,
>      > however patchsets like this can sometimes take a while to get reviewed because
>     they
>      > both i) touch legacy code/APIs and ii) cut across multiple machines and
>     maintainers.
>      > I'd really like to get this work, along with your RTC updates, merged soon as
>     it is a
>      > great improvement.
>      >
>      > One reason that you may not get many reviews is because you've not added the
>     relevant
>      > maintainers on To/CC. Due to the large volume of emails on qemu-devel, quite a
>     few
>      > maintainers will filter based upon their own email address so it is definitely
>     worth
>      > adding them in.
>      >
>      > Fortunately you can easily find the relevant maintainer email addresses by
>     running
>      > "./scripts/get_maintainer.pl <http://get_maintainer.pl>
>     <path-to-git-patch-dir>" on your git format-patch
>      > directory. I'd recommend doing this, and also dropping qemu-trivial since I
>     would say
>      > these patches are now beyond the trivial threshold.
> 
>     Oh wait - I see now it's just the cover letter which is missing the additional
>     maintainer addresses :)  If you could add them into the cover letter for your next
>     revision that would be great, since it gives context for maintainers to help with
>     the
>     review process.
> 
> 
> Hi Mark,
> 
> Thanks for your great work you put into reviews and the useful insights! It seems to 
> me that the time it takes for patches to be queued depends on the subsystem - some 
> are faster, some are slower...
> 
> I've automated my setup as described in [1]. However, it doesn't seem to work for the 
> cover letter which I'd expect to be sent to the union of all reviewers of all 
> patches. Any idea how to fix this?
> 
> Best regards,
> Bernhard
> 
> [1] 
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer 
> <https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer> 

Good question. I tend to do "git format-patch -o /tmp/foo --cover-letter" to generate 
the series, fill in the cover letter, and then use "git send-email /tmp/foo" to send 
out the emails (entering in the results of get_maintainer.pl by hand). I'm not sure 
why the cover letter isn't being generated correctly in your case I'm afraid.


ATB,

Mark.
Philippe Mathieu-Daudé May 30, 2022, 7:45 p.m. UTC | #5
On 30/5/22 21:11, Mark Cave-Ayland wrote:
> On 29/05/2022 14:02, Bernhard Beschow wrote:

>>     Oh wait - I see now it's just the cover letter which is missing 
>> the additional
>>     maintainer addresses :)  If you could add them into the cover 
>> letter for your next
>>     revision that would be great, since it gives context for 
>> maintainers to help with
>>     the
>>     review process.
>>
>>
>> Hi Mark,
>>
>> Thanks for your great work you put into reviews and the useful 
>> insights! It seems to me that the time it takes for patches to be 
>> queued depends on the subsystem - some are faster, some are slower...
>>
>> I've automated my setup as described in [1]. However, it doesn't seem 
>> to work for the cover letter which I'd expect to be sent to the union 
>> of all reviewers of all patches. Any idea how to fix this?
>>
>> Best regards,
>> Bernhard
>>
>> [1] 
>> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer 
>> <https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer> 
> 
> 
> Good question. I tend to do "git format-patch -o /tmp/foo 
> --cover-letter" to generate the series, fill in the cover letter, and 
> then use "git send-email /tmp/foo" to send out the emails (entering in 
> the results of get_maintainer.pl by hand). I'm not sure why the cover 
> letter isn't being generated correctly in your case I'm afraid.

Or try git-publish :) It does a first pass collecting Cc for each patch
(calling get_maintainer.pl) then use that set on the cover.

https://github.com/stefanha/git-publish
Bernhard Beschow June 4, 2022, 8:36 a.m. UTC | #6
Am 30. Mai 2022 19:45:26 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 30/5/22 21:11, Mark Cave-Ayland wrote:
>> On 29/05/2022 14:02, Bernhard Beschow wrote:
>
>>>     Oh wait - I see now it's just the cover letter which is missing the additional
>>>     maintainer addresses :)  If you could add them into the cover letter for your next
>>>     revision that would be great, since it gives context for maintainers to help with
>>>     the
>>>     review process.
>>> 
>>> 
>>> Hi Mark,
>>> 
>>> Thanks for your great work you put into reviews and the useful insights! It seems to me that the time it takes for patches to be queued depends on the subsystem - some are faster, some are slower...
>>> 
>>> I've automated my setup as described in [1]. However, it doesn't seem to work for the cover letter which I'd expect to be sent to the union of all reviewers of all patches. Any idea how to fix this?
>>> 
>>> Best regards,
>>> Bernhard
>>> 
>>> [1] https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer <https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer> 
>> 
>> 
>> Good question. I tend to do "git format-patch -o /tmp/foo --cover-letter" to generate the series, fill in the cover letter, and then use "git send-email /tmp/foo" to send out the emails (entering in the results of get_maintainer.pl by hand). I'm not sure why the cover letter isn't being generated correctly in your case I'm afraid.
>
>Or try git-publish :) It does a first pass collecting Cc for each patch
>(calling get_maintainer.pl) then use that set on the cover.
>
>https://github.com/stefanha/git-publish

Yes, that worked. What an improvement!

Best regards,
Bernhard