On 8/21/23 21:32, Laine Stump wrote:
> There can be many different drivers that are of the type "VFIO", so
> add the driver name to the object and allow getting/setting it.
> 
> Signed-off-by: Laine Stump <la...@redhat.com>
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virpci.c        | 16 ++++++++++++++++
>  src/util/virpci.h        |  3 +++
>  3 files changed, 21 insertions(+)
> 

> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 88a020fb86..103bc4254e 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c

> @@ -1580,6 +1583,19 @@ virPCIDeviceGetStubDriverType(virPCIDevice *dev)
>      return dev->stubDriverType;
>  }
>  
> +void
> +virPCIDeviceSetStubDriverName(virPCIDevice *dev,
> +                                   const char *driverName)
> +{
> +    dev->stubDriverName = g_strdup(driverName);
> +}


I think it's worth freeing dev->stubDriverName before setting another
one. Please consider squashing this in:

diff --git i/src/util/virpci.c w/src/util/virpci.c
index 3f207a24f3..15e53e6749 100644
--- i/src/util/virpci.c
+++ w/src/util/virpci.c
@@ -1586,8 +1586,9 @@ virPCIDeviceGetStubDriverType(virPCIDevice *dev)

 void
 virPCIDeviceSetStubDriverName(virPCIDevice *dev,
-                                   const char *driverName)
+                              const char *driverName)
 {
+    g_free(dev->stubDriverName);
     dev->stubDriverName = g_strdup(driverName);
 }


And just a thought (maybe it was covered in earlier discussions, maybe
it'll be covered in next patches) - what about the following scenario:

virPCIDeviceSetStubDriverType(&dev, VIR_PCI_STUB_DRIVER_VFIO);
virPCIDeviceSetStubDriverName(&dev, "blah");

Should the latter reset dev->stubDriverType to _NONE? Similarly, what if
the two are reversed? But I guess we can always fine tune this later.

Michal

Reply via email to