On 10/14/21 12:16 PM, Ani Sinha wrote:
> https://gitlab.com/libvirt/libvirt/-/commit/bdc3e8f47be108fa552b72a6d913528869e61097
> <https://gitlab.com/libvirt/libvirt/-/commit/bdc3e8f47be108fa552b72a6d913528869e61097>
>
> I think you completely missed the logic here and you are rewriting a
> patch without cc'ing the original author. I'm sorry but this is rude.
>
For acpi-bridge hotplug some conditions have to be met. If the guest
uses q35 then QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE capability has to be
set, or if guest is i440fx then the QEMU_CAPS_PIIX4_ACPI_HOTPLUG_BRIDGE
capability has to be set.
Previously, the code did not check for the i440fx case at all.
That is not quite correct because for i440fx case !q35cap would be true and then the AND logic would have checked whether the cap for i440fx exists or not. However the ANd logic is then broken for q35 side because of the machine is q35 we do not care if the cap exists for i440fx.
In any case, logic was broken and the review did not catch it and I was about to leave for vacation which is still ongoing. I would perhaps have fixed this in a slightly different way if it was prompted to me in a timely manner.
Nevertheless, on qemu side from what I have seen and I myself try to practice is that the subsequent fixes to a patch is generally also cc’d to the original author. Particularly when fixes=foo here identifies the patch which introduced the bug.