Re: [PATCH 03/10] qemu: replace qemuHostdevPreparePCIDevices

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