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

[Laine Stump]

On 8/23/23 3:52 AM, Michal Prívozník wrote:
> 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 at 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);
>   }

Sure, that seems prudent.

> 
> 
> 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.

Although the two settings are conceptually intertwined, I think the APIs 
should be orthogonal, with the setting of one not affecting the other; 
otherwise people would get confused about which one should come first 
and/or be surprised when it didn't do what they wanted.