qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 0/7] QOM'ify PIIX southbridge creation


From: Mark Cave-Ayland
Subject: Re: [PATCH v3 0/7] QOM'ify PIIX southbridge creation
Date: Mon, 30 May 2022 20:11:36 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0

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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]