qemuHostdevPreparePCIDevices, virHostdevPreparePCIDevices are identical.
Replacing qemu method with the generic one to reduce code duplication.
This generic method will be used in ch driver to passthrough PCI
devices.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
---
src/qemu/qemu_hostdev.c | 17 ++---------------
src/qemu/qemu_hostdev.h | 6 ------
src/qemu/qemu_hotplug.c | 5 +++--
3 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index ab2769d482..894f12e04f 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -152,20 +152,6 @@ qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
name, disks, ndisks);
}
-int
-qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
- const char *name,
- const unsigned char *uuid,
- virDomainHostdevDef **hostdevs,
- int nhostdevs,
- unsigned int flags)
-{
- return virHostdevPreparePCIDevices(driver->hostdevMgr,
- QEMU_DRIVER_NAME,
- name, uuid, hostdevs,
- nhostdevs, flags);
-}
-
int
qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
const char *name,
@@ -244,7 +230,8 @@ qemuHostdevPrepareDomainDevices(virQEMUDriver *driver,
if (qemuHostdevPrepareNVMeDisks(driver, def->name, def->disks, def->ndisks) < 0)
return -1;
- if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid,
+ if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME,
+ def->name, def->uuid,
def->hostdevs, def->nhostdevs, flags) < 0)
return -1;
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index b6dd2e0207..6f6c4f82bc 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -43,12 +43,6 @@ int qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
const char *name,
virDomainDiskDef **disks,
size_t ndisks);
-int qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
- const char *name,
- const unsigned char *uuid,
- virDomainHostdevDef **hostdevs,
- int nhostdevs,
- unsigned int flags);
int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
const char *name,
virDomainHostdevDef **hostdevs,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4d4bcde1bc..d1c042c6a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1544,8 +1544,9 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver,
if (!cfg->relaxedACS)
flags |= VIR_HOSTDEV_STRICT_ACS_CHECK;
- if (qemuHostdevPreparePCIDevices(driver, vm->def->name,
- vm->def->uuid, &hostdev, 1, flags) < 0)
+ if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME,
+ vm->def->name, vm->def->uuid,
+ &hostdev, 1, flags) < 0)
return -1;
if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0)
--
2.44.0On 10/11/24 20:13, Praveen K Paladugu wrote:
> qemuHostdevPreparePCIDevices, virHostdevPreparePCIDevices are identical.
> Replacing qemu method with the generic one to reduce code duplication.
> This generic method will be used in ch driver to passthrough PCI
> devices.
>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> ---
> src/qemu/qemu_hostdev.c | 17 ++---------------
> src/qemu/qemu_hostdev.h | 6 ------
> src/qemu/qemu_hotplug.c | 5 +++--
> 3 files changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index ab2769d482..894f12e04f 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -152,20 +152,6 @@ qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
> name, disks, ndisks);
> }
>
> -int
> -qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
> - const char *name,
> - const unsigned char *uuid,
> - virDomainHostdevDef **hostdevs,
> - int nhostdevs,
> - unsigned int flags)
> -{
> - return virHostdevPreparePCIDevices(driver->hostdevMgr,
> - QEMU_DRIVER_NAME,
> - name, uuid, hostdevs,
> - nhostdevs, flags);
> -}
> -
> int
> qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
> const char *name,
> @@ -244,7 +230,8 @@ qemuHostdevPrepareDomainDevices(virQEMUDriver *driver,
> if (qemuHostdevPrepareNVMeDisks(driver, def->name, def->disks, def->ndisks) < 0)
> return -1;
>
> - if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid,
> + if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME,
> + def->name, def->uuid,
> def->hostdevs, def->nhostdevs, flags) < 0)
I'm not a fan of this patch and here you can see why. Sometimes we have
function that are just wrapper to a function with more complicated
arguments structure and this is such case. Since you don't need this
patch, I'll drop this one.
> return -1;
>
> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
> index b6dd2e0207..6f6c4f82bc 100644
> --- a/src/qemu/qemu_hostdev.h
> +++ b/src/qemu/qemu_hostdev.h
> @@ -43,12 +43,6 @@ int qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
> const char *name,
> virDomainDiskDef **disks,
> size_t ndisks);
> -int qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
> - const char *name,
> - const unsigned char *uuid,
> - virDomainHostdevDef **hostdevs,
> - int nhostdevs,
> - unsigned int flags);
> int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
> const char *name,
> virDomainHostdevDef **hostdevs,
See? It fits nicely into the rest of family. If anything, these function
can take fewer arguments: [driver, virDomainDef *, **hostdevs,
nhostdevs, flags]. But at this point there's not much value in replacing
all of them.
Michal
On 11/15/2024 6:22 AM, Michal Prívozník wrote:
> On 10/11/24 20:13, Praveen K Paladugu wrote:
>> qemuHostdevPreparePCIDevices, virHostdevPreparePCIDevices are identical.
>> Replacing qemu method with the generic one to reduce code duplication.
>> This generic method will be used in ch driver to passthrough PCI
>> devices.
>>
>> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
>> ---
>> src/qemu/qemu_hostdev.c | 17 ++---------------
>> src/qemu/qemu_hostdev.h | 6 ------
>> src/qemu/qemu_hotplug.c | 5 +++--
>> 3 files changed, 5 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
>> index ab2769d482..894f12e04f 100644
>> --- a/src/qemu/qemu_hostdev.c
>> +++ b/src/qemu/qemu_hostdev.c
>> @@ -152,20 +152,6 @@ qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
>> name, disks, ndisks);
>> }
>>
>> -int
>> -qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
>> - const char *name,
>> - const unsigned char *uuid,
>> - virDomainHostdevDef **hostdevs,
>> - int nhostdevs,
>> - unsigned int flags)
>> -{
>> - return virHostdevPreparePCIDevices(driver->hostdevMgr,
>> - QEMU_DRIVER_NAME,
>> - name, uuid, hostdevs,
>> - nhostdevs, flags);
>> -}
>> -
>> int
>> qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
>> const char *name,
>> @@ -244,7 +230,8 @@ qemuHostdevPrepareDomainDevices(virQEMUDriver *driver,
>> if (qemuHostdevPrepareNVMeDisks(driver, def->name, def->disks, def->ndisks) < 0)
>> return -1;
>>
>> - if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid,
>> + if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME,
>> + def->name, def->uuid,
>> def->hostdevs, def->nhostdevs, flags) < 0)
>
> I'm not a fan of this patch and here you can see why. Sometimes we have
> function that are just wrapper to a function with more complicated
> arguments structure and this is such case. Since you don't need this
> patch, I'll drop this one.
>
>> return -1;
>>
>> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
>> index b6dd2e0207..6f6c4f82bc 100644
>> --- a/src/qemu/qemu_hostdev.h
>> +++ b/src/qemu/qemu_hostdev.h
>> @@ -43,12 +43,6 @@ int qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver,
>> const char *name,
>> virDomainDiskDef **disks,
>> size_t ndisks);
>> -int qemuHostdevPreparePCIDevices(virQEMUDriver *driver,
>> - const char *name,
>> - const unsigned char *uuid,
>> - virDomainHostdevDef **hostdevs,
>> - int nhostdevs,
>> - unsigned int flags);
>> int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver,
>> const char *name,
>> virDomainHostdevDef **hostdevs,
>
> See? It fits nicely into the rest of family. If anything, these function
> can take fewer arguments: [driver, virDomainDef *, **hostdevs,
> nhostdevs, flags]. But at this point there's not much value in replacing
> all of them.
>
> Michal
>
Understood, thanks for the feedback Michal.
--
Regards,
Praveen K Paladugu
© 2016 - 2026 Red Hat, Inc.