1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: Set proper PCI backend for <interface/>-s that are actually hostdevs

Michal Privoznik posted 1 patch 10 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/f7999c4054e0c9525d7cd5dc5b4b5df246a84b74.1685626000.git.mprivozn@redhat.com
src/qemu/qemu_process.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] qemu: Set proper PCI backend for <interface/>-s that are actually hostdevs
Posted by Michal Privoznik 10 months, 3 weeks ago 
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@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)
+                return -1;
+
             if (virDomainHostdevInsert(def, hostdev) < 0)
                 return -1;
         } else if (actualType == VIR_DOMAIN_NET_TYPE_USER &&
-- 
2.39.3
Re: [PATCH] qemu: Set proper PCI backend for <interface/>-s that are actually hostdevs
Posted by Michal Prívozník 10 months, 3 weeks ago 
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@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 &&

Michal
Re: [PATCH] qemu: Set proper PCI backend for <interface/>-s that are actually hostdevs
Posted by Martin Kletzander 10 months, 2 weeks ago 
On Thu, Jun 01, 2023 at 05:25:14PM +0200, Michal Prívozník wrote:
>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@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@redhat.com>

>Michal
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.