On Wed, 28 Jul 2021, Michael S. Tsirkin wrote:
> On Mon, Jul 26, 2021 at 10:27:43PM +0530, Ani Sinha wrote:
> > All existing code using acpi_get_i386_pci_host() checks for a non-null
> > return value from this function call. Instead of returning early when the
> > value
> > returned is NULL, assert instead. Since there are only two possible host
> > buses
> > for i386 - q35 and i440fx, a null value return from the function does not
> > make
> > sense in most cases and is likely an error situation.
>
> add "on i386"?
>
> > Fixes: c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
>
> This that seems inappropriate, this is not a bugfix.
>
Forgot to answer this. I started this patch because I saw a gap that was
introduced with the above patch. In acpi_pcihp_disable_root_bus(), Julia's
code did not check for null return value from acpi_get_i386_pci_host().
See v2. Hence, I added the fixes tag. Then Igor suggested that I assert
instead and I also thought perhaps assertion is a better idea. Hence v3. I
am not conflicted after reading your argument. We should assert only when
a certain invariant is always respected. Otherwise we should not assert.
If you think acpi_get_i386_pci_host() can be called from non-i386 path as
well, maybe v2 approach is better.
