[libvirt] [PATCH] qemu: avoid double reservation of PCI address when hotplugging interface type='hostdev'
Andrea Bolognani
abologna at redhat.com
Mon Oct 21 13:02:15 UTC 2019
On Sat, 2019-10-19 at 02:18 -0400, Laine Stump wrote:
> Commit 01ca4010d86 (libvirt v5.1.0) moved address reservation for
> hotplugged interface devices up to an earlier point in
> qemuDomainAttachNetDevice() because some function called when
> hotplugging on aarch64 needed to know the address type, and failed
> when it was "none".
I don't see anything in the original commit suggesting the change
was connected to aarch64? In fact, for the most part I would expect
aarch64/virt guests to go down the very same code paths as x86/q35
guests.
> This unfortunately caused a regression, because it also made PCI
> address reservation happen before we noticed that the device was a
> *hostdev* interface. Those interfaces are hotplugged by just calling
> out to qemuDomainAttachHostdevDevice() - that function would then also
> attempt to reserve the *same PCI address* that had just been reserved
> in qemuDomainAttachNetDevice().
And of course at the time we didn't have, and after this patch still
don't have, test suite coverage for that O:-)
> The solution is to move the bit of code that short-circuits out to
> virDomainHostdevAttach() up *even earlier* so that no PCI address has
> been allocated by the time it's called.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1744523
> Signed-off-by: Laine Stump <laine at redhat.com>
The prevailing style seems to be
...
been allocated by the time it's called.
https://bugzilla.redhat.com/show_bug.cgi?id=1744523
Signed-off-by: Laine Stump <laine at redhat.com>
Feel free to change it or leave it as-is, I personally don't feel
very strongly either way.
> +++ b/src/qemu/qemu_hotplug.c
> + case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> + /* this switch is skipped by hostdev interfaces */
Maybe something like
/* hostdev interfaces have been dealt with earlier */
instead?
With at least the reference to aarch64 removed (unless of course I
have misunderstood the situation and the original commit was really
aarch64-related),
Reviewed-by: Andrea Bolognani <abologna at redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
More information about the libvir-list
mailing list