On 6/1/23 15:26, Michal Privoznik wrote:
> When starting a domain, it's done so in two steps (actually more,
> but lets focus on just the following two):
>
> 1) qemuProcessPrepareDomain(), followed by
>
> 2) qemuProcessPrepareHost().
>
> Now, in the first step (PrepareDomain()), PCI backends for all
> hostdevs is set (qemuProcessPrepareDomain() ->
> qemuProcessPrepareDomainHostdevs() -> qemuDomainPrepareHostdev()
> -> qemuDomainPrepareHostdevPCI()). Perfect.
>
> But then, additional hostdevs may appear, because in the host
> prepare phase we may insert some hostdevs into domain definition
> (qemuProcessPrepareHost() -> qemuProcessNetworkPrepareDevices()).
>
> Now, these additional hostdevs don't undergo the same prepare as
> hostdevs that were already present in the domain definition (i.e.
> in qemuProcessPrepareDomain() phase). Therefore, we have to call
> corresponding prepare function explicitly.
>
> NB, the interface hotplug code (qemuDomainAttachNetDevice()) does
> not suffer from this problem, because it calls top level
> qemuDomainAttachHostDevice() which is used to hotplug regular
> hostdevs too and as such calls qemuDomainPrepareHostdev().
>
> Fixes: 3b87709c768480e085556e06bd8d08f62270d42d
> Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=2209853
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_process.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 730e59eb7e..1431fa8310 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5834,6 +5834,13 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver,
> net->data.network.name, def->name);
> return -1;
> }
> +
> + /* For hostdev present in qemuProcessPrepareDomain() phase this was
> + * done already, but this code runs after that, so we have to call
> + * it ourselves. */
> + if (qemuDomainPrepareHostdevPCI(hostdev, priv) < 0)
This should have been just qemuDomainPrepareHostdev() (i.e. without the
PCI suffix). Forgot to squash it in before sending. Fixed locally.
> + return -1;
> +
> if (virDomainHostdevInsert(def, hostdev) < 0)
> return -1;
> } else if (actualType == VIR_DOMAIN_NET_TYPE_USER &&
Sounds reasonable, looks good
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>