11:31 a.m.
On Fri, Feb 01, 2013 at 11:18:30 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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