Please explain to me this commit

[Ani Sinha]

On Thu, Oct 14, 2021 at 16:54 Michal Prívozník <mprivozn at redhat.com> wrote:

> 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.

>


>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211014/0bb53d9d/attachment-0001.htm>