On Fri, Feb 01, 2013 at 11:18:30 +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Rename all the pciDeviceXXX and pciXXXDevice APIs to have a
> fixed virPCIDevice name prefix
Some functions gained just virPCI prefix, I guess that means they don't
take virPCIDevicePtr arguments. In any case, the shorter prefix the
better so I'm not opposed to it :-)
...
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 1b8a9cd..b5d7c5e 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
...
> @@ -856,7 +856,7 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
> hostdevs,
> nhostdevs))) {
> virErrorPtr err = virGetLastError();
> - VIR_ERROR(_("Failed to allocate pciDeviceList: %s"),
> + VIR_ERROR(_("Failed to allocate virPCIDeviceList: %s"),
Why not just "PCI device list"?
> err ? err->message : _("unknown error"));
> virResetError(err);
> goto cleanup;
...
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 0fb9923..695f372 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
...
> @@ -748,39 +748,39 @@ pciTryPowerManagementReset(pciDevice *dev, int cfgfd)
> }
>
> static int
> -pciInitDevice(pciDevice *dev, int cfgfd)
> +virPCIDeviceInitDevice(virPCIDevicePtr dev, int cfgfd)
Why not just virPCIDeviceInit?
> {
> int flr;
>
> - dev->pcie_cap_pos = pciFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP);
> - dev->pci_pm_cap_pos = pciFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM);
> - flr = pciDetectFunctionLevelReset(dev, cfgfd);
> + dev->pcie_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_EXP);
> + dev->pci_pm_cap_pos = virPCIDeviceFindCapabilityOffset(dev, cfgfd, PCI_CAP_ID_PM);
> + flr = virPCIDeviceDetectFunctionLevelReset(dev, cfgfd);
> if (flr < 0)
> return flr;
> dev->has_flr = flr;
> - dev->has_pm_reset = pciDetectPowerManagementReset(dev, cfgfd);
> + dev->has_pm_reset = virPCIDeviceDetectPowerManagementReset(dev, cfgfd);
>
> return 0;
> }
...
> @@ -887,13 +887,13 @@ recheck:
> }
>
> static int
> -pciUnbindDeviceFromStub(pciDevice *dev, const char *driver)
> +virPCIDeviceUnbindDeviceFromStub(virPCIDevicePtr dev, const char *driver)
virPCIDeviceUnbindFromStub would be a much better name.
> {
> int result = -1;
> char *drvdir = NULL;
> char *path = NULL;
>
> - if (pciDriverDir(&drvdir, driver) < 0)
> + if (virPCIDriverDir(&drvdir, driver) < 0)
> goto cleanup;
>
> if (!dev->unbind_from_stub)
...
> @@ -975,7 +975,7 @@ cleanup:
>
>
> static int
> -pciBindDeviceToStub(pciDevice *dev, const char *driver)
> +virPCIDeviceBindDeviceToStub(virPCIDevicePtr dev, const char *driver)
virPCIDeviceBindToStub sounds better.
> {
> int result = -1;
> char *drvdir = NULL;
...
> @@ -1118,36 +1118,36 @@ cleanup:
> VIR_FREE(path);
>
> if (result < 0) {
> - pciUnbindDeviceFromStub(dev, driver);
> + virPCIDeviceUnbindDeviceFromStub(dev, driver);
> }
>
> return result;
> }
>
> int
> -pciDettachDevice(pciDevice *dev,
> - pciDeviceList *activeDevs,
> - pciDeviceList *inactiveDevs,
> - const char *driver)
> +virPCIDeviceDettach(virPCIDevicePtr dev,
> + virPCIDeviceList *activeDevs,
> + virPCIDeviceList *inactiveDevs,
> + const char *driver)
Since you're already changing the function name, you could have fixed
the typo: virPCIDeviceDetach.
> {
> - if (pciProbeStubDriver(driver) < 0) {
> + if (virPCIProbeStubDriver(driver) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to load PCI stub module %s"), driver);
> return -1;
> }
...
> @@ -1155,29 +1155,29 @@ pciDettachDevice(pciDevice *dev,
> }
>
> int
> -pciReAttachDevice(pciDevice *dev,
> - pciDeviceList *activeDevs,
> - pciDeviceList *inactiveDevs,
> - const char *driver)
> +virPCIDeviceReAttach(virPCIDevicePtr dev,
> + virPCIDeviceListPtr activeDevs,
> + virPCIDeviceListPtr inactiveDevs,
> + const char *driver)
I'd make the "A" lower case.
> {
> - if (pciProbeStubDriver(driver) < 0) {
> + if (virPCIProbeStubDriver(driver) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to load PCI stub module %s"), driver);
> return -1;
> }
...
> @@ -1292,12 +1292,12 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher)
> }
>
> static char *
> -pciReadDeviceID(pciDevice *dev, const char *id_name)
> +virPCIDeviceReadDeviceID(virPCIDevicePtr dev, const char *id_name)
Would virPCIDeviceReadID be better? I'm not sure but DeviceReadDeviceID
is weird.
> {
> char *path = NULL;
> char *id_str;
>
> - if (pciDeviceFile(&path, dev->name, id_name) < 0) {
> + if (virPCIFile(&path, dev->name, id_name) < 0) {
> return NULL;
> }
>
...
> @@ -1880,8 +1880,8 @@ out:
> }
>
> static int
> -pciGetPciConfigAddressFromSysfsDeviceLink(const char *device_link,
> - struct pci_config_address **bdf)
> +virPCIGetDeviceAddressFromSysfsDeviceLink(const char *device_link,
> + virPCIDeviceAddressPtr *bdf)
virPCIGetDeviceAddressFromSysfsLink would sound a bit better to me.
> {
> char *config_address = NULL;
> char *device_path = NULL;
...
ACK whether you implement changes I suggested or not (or just some of
them) as long as make all check syntax-check succeeds.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list