Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On 10/4/21 1:05 PM, Andrea Bolognani wrote: On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: On Mon, 4 Oct 2021, Andrea Bolognani wrote: On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: +++ b/src/qemu/qemu_capabilities.c + /* 410 */ + "piix4-acpi-root-hotplug-en", /* QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ Sorry if this is a silly question, but can you please explain where the "-en" suffix comes from? "en" stands for enable. AFAICT there are no other capabilities that use this convention. Moreover, the option can be used both to enable *and* disable hotplug, so the name doesn't seem accurate either... The string is not exposed to users, so ultimately none of this really matters, but I would still suggest changing it to piix4.acpi-root-pci-hotplug I'd be happy to either review a patch of yours that does that, or preparing such a patch myself. BAH!!! Someone else mentioned that in an earlier iteration of the patches (and may have even suggested the "." after piix4 instead of "-"), and I had meant to check/ask about it in the respin, and then forgot. :-/ (somehow my brain skipped right over those 3 characters) I agree with Andrea's suggested change, and apologize for not catching it in review (fortunately it was pushed just *after* a release instead of before, so we can still make such a change). I'll gladly review and/or push any such patch that arrives.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, 4 Oct 2021, Andrea Bolognani wrote: > On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: > > On Mon, 4 Oct 2021, Andrea Bolognani wrote: > > > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > > > +++ b/src/qemu/qemu_capabilities.c > > > > + /* 410 */ > > > > + "piix4-acpi-root-hotplug-en", /* > > > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > > > > > Sorry if this is a silly question, but can you please explain where > > > the "-en" suffix comes from? > > > > "en" stands for enable. > > AFAICT there are no other capabilities that use this convention. > Moreover, the option can be used both to enable *and* disable > hotplug, so the name doesn't seem accurate either... > > The string is not exposed to users, so ultimately none of this really > matters, but I would still suggest changing it to > > piix4.acpi-root-pci-hotplug > > I'd be happy to either review a patch of yours that does that, or > preparing such a patch myself. Sent a patch to rename it.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, Oct 04, 2021 at 10:19:17PM +0530, Ani Sinha wrote: > On Mon, 4 Oct 2021, Andrea Bolognani wrote: > > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > > +++ b/src/qemu/qemu_capabilities.c > > > + /* 410 */ > > > + "piix4-acpi-root-hotplug-en", /* > > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > > > Sorry if this is a silly question, but can you please explain where > > the "-en" suffix comes from? > > "en" stands for enable. AFAICT there are no other capabilities that use this convention. Moreover, the option can be used both to enable *and* disable hotplug, so the name doesn't seem accurate either... The string is not exposed to users, so ultimately none of this really matters, but I would still suggest changing it to piix4.acpi-root-pci-hotplug I'd be happy to either review a patch of yours that does that, or preparing such a patch myself. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Mon, 4 Oct 2021, Andrea Bolognani wrote: > On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > > +++ b/src/qemu/qemu_capabilities.c > > + /* 410 */ > > + "piix4-acpi-root-hotplug-en", /* > > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ > > Sorry if this is a silly question, but can you please explain where > the "-en" suffix comes from? "en" stands for enable.
Re: [PATCH v7 1/4] qemu: capablities: detect presence of acpi-root-pci-hotplug for i440fx machines
On Fri, Oct 01, 2021 at 02:59:45PM +0530, Ani Sinha wrote: > +++ b/src/qemu/qemu_capabilities.c > + /* 410 */ > + "piix4-acpi-root-hotplug-en", /* > QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG */ Sorry if this is a silly question, but can you please explain where the "-en" suffix comes from? -- Andrea Bolognani / Red Hat / Virtualization
