Re: [PULL 06/35] hw/acpi: refactor acpi hp modules so that targets can just use what they need

[Ani Sinha]

On Mon, Sep 6, 2021 at 4:19 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> On Mon, Sep 6, 2021 at 3:54 PM Philippe Mathieu-Daudé <philmd@redhat.com> 
> wrote:
> >
> > On 9/6/21 12:03 PM, Ani Sinha wrote:
> > > On Mon, 6 Sep 2021, Philippe Mathieu-Daudé wrote:
> > >> On 9/4/21 11:36 PM, Michael S. Tsirkin wrote:
> > >>> From: Ani Sinha <ani@anisinha.ca>
> > >>>
> > >>> Currently various acpi hotplug modules like cpu hotplug, memory 
> > >>> hotplug, pci
> > >>> hotplug, nvdimm hotplug are all pulled in when CONFIG_ACPI_X86 is 
> > >>> turned on.
> > >>> This brings in support for whole lot of subsystems that some targets 
> > >>> like
> > >>> mips does not need. They are added just to satisfy symbol dependencies. 
> > >>> This
> > >>> is ugly and should be avoided. Targets should be able to pull in just 
> > >>> what they
> > >>> need and no more. For example, mips only needs support for PIIX4 and 
> > >>> does not
> > >>> need acpi pci hotplug support or cpu hotplug support or memory hotplug 
> > >>> support
> > >>> etc. This change is an effort to clean this up.
> > >>> In this change, new config variables are added for various acpi hotplug
> > >>> subsystems. Targets like mips can only enable PIIX4 support and not the 
> > >>> rest
> > >>> of all the other modules which were being previously pulled in as a 
> > >>> part of
> > >>> CONFIG_ACPI_X86. Function stubs make sure that symbols which piix4 
> > >>> needs but
> > >>> are not required by mips (for example, symbols specific to pci hotplug 
> > >>> etc)
> > >>> are available to satisfy the dependencies.
> > >>>
> > >>> Currently, this change only addresses issues with mips malta targets. 
> > >>> In future
> > >>> we might be able to clean up other targets which are similarly pulling 
> > >>> in lot
> > >>> of unnecessary hotplug modules by enabling ACPI_X86.
> > >>>
> > >>> This change should also address issues such as the following:
> > >>> https://gitlab.com/qemu-project/qemu/-/issues/221
> > >>> https://gitlab.com/qemu-project/qemu/-/issues/193
> > >>
> > >> FYI per 
> > >> https://docs.gitlab.com/ee/administration/issue_closing_pattern.html
> > >> this should have been:
> > >>
> > >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/193
> > >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/221
> > >>
> > >
> > > Ah my apologies. Will do this next time.
> > >
> > >> Can we close these issues manually?
> > >
> > > Since both you and I have verified that those issues gets fixed with my
> > > change, yes we can close them. I do not have a gitlab account. Should I
> > > have one? Is there special permissions needed to handle these tickets?
> >
> > Since you are listed in the MAINTAINERS file, long-term you'll
> > eventually use it anyway (i.e. to run the CI pipelines before sending
> > patches, to subscribe to the 'ACPI' label to get notifications or
> > comment ACPI-related issues).
> >
> > The process is quite straight-forward, once having an account you
> > simply request to be member of the project via the WebUI then you
> > can help triaging the issues (and closing these two).
>
> Hmm. I created an account and added a comment to the tickets. However
> I am unable to close them. I requested access to the project.

I could be wrong, but I think only reporters can open and close bugs
like yourself on gitlab.