Re: [libvirt PATCH v3 2/8] util: add stub driver name to virPCIDevice object

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 <laine(a)redhat.com&gt; > --- > 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.