3:09 a.m.
$ mv blurb here/
Andrea Bolognani (3):
conf: Move virDomainPCIAddressAsString() to util/virpci
conf: Rename virDomainPCIAddressAsString()
util: Drop virPCIGetAddrString()
src/conf/domain_addr.c | 20 +++-----------------
src/conf/domain_addr.h | 4 ----
src/libvirt_private.syms | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain_address.c | 6 +++---
src/util/virnetdev.c | 6 +-----
src/util/virpci.c | 21 +++++++++------------
src/util/virpci.h | 8 ++------
8 files changed, 20 insertions(+), 49 deletions(-)
--
2.17.1
3:09 a.m.
New subject: [libvirt] [PATCH v2 1/3] conf: Move virDomainPCIAddressAsString() to util/virpci
It's a better fit than conf/domain_conf.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 14 --------------
src/conf/domain_addr.h | 4 ----
src/libvirt_private.syms | 2 +-
src/util/virpci.c | 13 +++++++++++++
src/util/virpci.h | 3 +++
5 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index b62fd53c66..d2e1142462 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -573,20 +573,6 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
}
-char *
-virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
-{
- char *str;
-
- ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x",
- addr->domain,
- addr->bus,
- addr->slot,
- addr->function));
- return str;
-}
-
-
/*
* Check if the PCI slot is used by another device.
*/
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5ad9d8ef3d..f8ab742715 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -124,9 +124,6 @@ struct _virDomainPCIAddressSet {
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
-char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
- ATTRIBUTE_NONNULL(1);
-
virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses);
void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs);
@@ -138,7 +135,6 @@ bool virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs,
bool fromConfig)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
-
int virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
virDomainControllerModelPCI model)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e2340d23a6..04242a22b1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -119,7 +119,6 @@ virPCIDeviceAddressParseXML;
virDomainCCWAddressAssign;
virDomainCCWAddressSetCreateFromDomain;
virDomainCCWAddressSetFree;
-virDomainPCIAddressAsString;
virDomainPCIAddressBusIsFullyReserved;
virDomainPCIAddressBusSetModel;
virDomainPCIAddressEnsureAddr;
@@ -2497,6 +2496,7 @@ virObjectUnref;
# util/virpci.h
+virDomainPCIAddressAsString;
virPCIDeviceAddressGetIOMMUGroupAddresses;
virPCIDeviceAddressGetIOMMUGroupNum;
virPCIDeviceAddressGetSysfsFile;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 6cf2acf2d1..512e365cad 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1684,6 +1684,19 @@ virPCIGetAddrString(unsigned int domain,
return 0;
}
+char *
+virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+{
+ char *str;
+
+ ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x",
+ addr->domain,
+ addr->bus,
+ addr->slot,
+ addr->function));
+ return str;
+}
+
virPCIDevicePtr
virPCIDeviceNew(unsigned int domain,
unsigned int bus,
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 2ac87694df..9ef1b838b7 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -225,6 +225,9 @@ int virPCIGetAddrString(unsigned int domain,
char **pciConfigAddr)
ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
+char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+ ATTRIBUTE_NONNULL(1);
+
int virPCIDeviceAddressParse(char *address, virPCIDeviceAddressPtr bdf);
int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
--
2.17.1
7:47 a.m.
New subject: [libvirt] [PATCH v2 1/3] conf: Move virDomainPCIAddressAsString() to util/virpci
On Wed, Sep 05, 2018 at 10:09:24AM +0200, Andrea Bolognani wrote:
It's a better fit than conf/domain_conf.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 14 --------------
src/conf/domain_addr.h | 4 ----
src/libvirt_private.syms | 2 +-
src/util/virpci.c | 13 +++++++++++++
src/util/virpci.h | 3 +++
5 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index b62fd53c66..d2e1142462 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -573,20 +573,6 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
}
-char *
-virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
-{
- char *str;
-
- ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x",
- addr->domain,
- addr->bus,
- addr->slot,
- addr->function));
- return str;
-}
-
-
/*
* Check if the PCI slot is used by another device.
*/
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5ad9d8ef3d..f8ab742715 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -124,9 +124,6 @@ struct _virDomainPCIAddressSet {
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
-char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
- ATTRIBUTE_NONNULL(1);
-
virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses);
void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs);
@@ -138,7 +135,6 @@ bool virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs,
bool fromConfig)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
-
Separate this into trivial patch.
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
3:09 a.m.
New subject: [libvirt] [PATCH v2 2/3] conf: Rename virDomainPCIAddressAsString()
The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 6 +++---
src/libvirt_private.syms | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain_address.c | 6 +++---
src/util/virpci.c | 2 +-
src/util/virpci.h | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index d2e1142462..442e6aab94 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -604,7 +604,7 @@ virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr
addrs,
virErrorNumber errType = (fromConfig
? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
- if (!(addrStr = virDomainPCIAddressAsString(addr)))
+ if (!(addrStr = virPCIDeviceAddressAsString(addr)))
goto cleanup;
/* Add an extra bus if necessary */
@@ -689,7 +689,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
if (!flags)
return 0;
- if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
+ if (!(addrStr = virPCIDeviceAddressAsString(&dev->addr.pci)))
goto cleanup;
if (virDeviceInfoPCIAddressIsPresent(dev)) {
@@ -770,7 +770,7 @@ virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr
bus,
*found = false;
- if (!(addrStr = virDomainPCIAddressAsString(searchAddr)))
+ if (!(addrStr = virPCIDeviceAddressAsString(searchAddr)))
goto cleanup;
if (!virDomainPCIAddressFlagsCompatible(searchAddr, addrStr, bus->flags,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 04242a22b1..0fc5314b02 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2496,7 +2496,7 @@ virObjectUnref;
# util/virpci.h
-virDomainPCIAddressAsString;
+virPCIDeviceAddressAsString;
virPCIDeviceAddressGetIOMMUGroupAddresses;
virPCIDeviceAddressGetIOMMUGroupNum;
virPCIDeviceAddressGetSysfsFile;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8aa20496bc..b72506f4e1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -301,7 +301,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
size_t i;
- if (!(devStr = virDomainPCIAddressAsString(&info->addr.pci)))
+ if (!(devStr = virPCIDeviceAddressAsString(&info->addr.pci)))
goto cleanup;
for (i = 0; i < domainDef->ncontrollers; i++) {
virDomainControllerDefPtr cont = domainDef->controllers[i];
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index dda14adeb3..24fdf12128 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1306,7 +1306,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
* inappropriate address types.
*/
if (!info->pciConnectFlags) {
- char *addrStr = virDomainPCIAddressAsString(&info->addr.pci);
+ char *addrStr = virPCIDeviceAddressAsString(&info->addr.pci);
VIR_WARN("qemuDomainDeviceCalculatePCIConnectFlags() thinks that the "
"device with PCI address %s should not have a PCI address",
@@ -1554,7 +1554,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
memset(&tmp_addr, 0, sizeof(tmp_addr));
tmp_addr.slot = 2;
- if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr)))
+ if (!(addrStr = virPCIDeviceAddressAsString(&tmp_addr)))
goto cleanup;
if (!virDomainPCIAddressValidate(addrs, &tmp_addr,
addrStr, flags, true))
@@ -1743,7 +1743,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
memset(&tmp_addr, 0, sizeof(tmp_addr));
tmp_addr.slot = 1;
- if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr)))
+ if (!(addrStr = virPCIDeviceAddressAsString(&tmp_addr)))
goto cleanup;
if (!virDomainPCIAddressValidate(addrs, &tmp_addr,
addrStr, flags, true))
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 512e365cad..d82da710ee 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1685,7 +1685,7 @@ virPCIGetAddrString(unsigned int domain,
}
char *
-virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+virPCIDeviceAddressAsString(virPCIDeviceAddressPtr addr)
{
char *str;
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 9ef1b838b7..271a753be2 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -225,7 +225,7 @@ int virPCIGetAddrString(unsigned int domain,
char **pciConfigAddr)
ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
-char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+char *virPCIDeviceAddressAsString(virPCIDeviceAddressPtr addr)
ATTRIBUTE_NONNULL(1);
int virPCIDeviceAddressParse(char *address, virPCIDeviceAddressPtr bdf);
--
2.17.1
7:47 a.m.
New subject: [libvirt] [PATCH v2 2/3] conf: Rename virDomainPCIAddressAsString()
On Wed, Sep 05, 2018 at 10:09:25AM +0200, Andrea Bolognani wrote:
The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 6 +++---
src/libvirt_private.syms | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain_address.c | 6 +++---
src/util/virpci.c | 2 +-
src/util/virpci.h | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
3:09 a.m.
New subject: [libvirt] [PATCH v2 3/3] util: Drop virPCIGetAddrString()
There's a single user for it which takes an existing
virPCIDeviceAddress, passes its various bits to the
function which in turn constructs a virPCIDevice and
then copies the string representation for the caller
to use: we can use virPCIDeviceAddressAsString()
instead and avoid creating the virPCIDevice in the
first place. Since the function ends up having no
users after the change, we can just drop it.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/util/virnetdev.c | 6 +-----
src/util/virpci.c | 16 ----------------
src/util/virpci.h | 7 -------
3 files changed, 1 insertion(+), 28 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 8eac419725..9cc9d18155 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1305,11 +1305,7 @@ virNetDevGetVirtualFunctions(const char *pfname,
goto cleanup;
for (i = 0; i < *n_vfname; i++) {
- if (virPCIGetAddrString((*virt_fns)[i]->domain,
- (*virt_fns)[i]->bus,
- (*virt_fns)[i]->slot,
- (*virt_fns)[i]->function,
- &pciConfigAddr) < 0) {
+ if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i]))) {
virReportSystemError(ENOSYS, "%s",
_("Failed to get PCI Config Address
String"));
goto cleanup;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index d82da710ee..1730d888f7 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1668,22 +1668,6 @@ virPCIDeviceReadID(virPCIDevicePtr dev, const char *id_name)
return id_str;
}
-int
-virPCIGetAddrString(unsigned int domain,
- unsigned int bus,
- unsigned int slot,
- unsigned int function,
- char **pciConfigAddr)
-{
- VIR_AUTOPTR(virPCIDevice) dev = NULL;
-
- dev = virPCIDeviceNew(domain, bus, slot, function);
- if (!dev || VIR_STRDUP(*pciConfigAddr, dev->name) < 0)
- return -1;
-
- return 0;
-}
-
char *
virPCIDeviceAddressAsString(virPCIDeviceAddressPtr addr)
{
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 271a753be2..b4f72f8f06 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -218,13 +218,6 @@ int virPCIGetSysfsFile(char *virPCIDeviceName,
char **pci_sysfs_device_link)
ATTRIBUTE_RETURN_CHECK;
-int virPCIGetAddrString(unsigned int domain,
- unsigned int bus,
- unsigned int slot,
- unsigned int function,
- char **pciConfigAddr)
- ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
-
char *virPCIDeviceAddressAsString(virPCIDeviceAddressPtr addr)
ATTRIBUTE_NONNULL(1);
--
2.17.1
7:53 a.m.
New subject: [libvirt] [PATCH v2 3/3] util: Drop virPCIGetAddrString()
On Wed, Sep 05, 2018 at 10:09:26AM +0200, Andrea Bolognani wrote:
There's a single user for it which takes an existing
virPCIDeviceAddress, passes its various bits to the
function which in turn constructs a virPCIDevice and
then copies the string representation for the caller
to use: we can use virPCIDeviceAddressAsString()
instead and avoid creating the virPCIDevice in the
first place. Since the function ends up having no
users after the change, we can just drop it.
Much
wrapping,
such
narrow
=)
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/util/virnetdev.c | 6 +-----
src/util/virpci.c | 16 ----------------
src/util/virpci.h | 7 -------
3 files changed, 1 insertion(+), 28 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 8eac419725..9cc9d18155 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1305,11 +1305,7 @@ virNetDevGetVirtualFunctions(const char *pfname,
goto cleanup;
for (i = 0; i < *n_vfname; i++) {
- if (virPCIGetAddrString((*virt_fns)[i]->domain,
- (*virt_fns)[i]->bus,
- (*virt_fns)[i]->slot,
- (*virt_fns)[i]->function,
- &pciConfigAddr) < 0) {
+ if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i]))) {
There is one more thing that this change does, previously it would fail for
non-existing device, which it will not now. I don't think that is a problem,
but wanted to mention that.
virReportSystemError(ENOSYS, "%s",
_("Failed to get PCI Config Address String"));
You should remove this error message as virAsprintf() in
virPCIDeviceAddressAsString() already sets an OOM Error.
I you like working on this function there are lot of things that leak from the
loop, so looking at that would be great, thanks.
With the removal of the error message above:
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
8:50 a.m.
New subject: [libvirt] [PATCH v2 3/3] util: Drop virPCIGetAddrString()
On Wed, 2018-09-05 at 14:53 +0200, Martin Kletzander wrote:
On Wed, Sep 05, 2018 at 10:09:26AM +0200, Andrea Bolognani wrote:
> for (i = 0; i < *n_vfname; i++) {
> - if (virPCIGetAddrString((*virt_fns)[i]->domain,
> - (*virt_fns)[i]->bus,
> - (*virt_fns)[i]->slot,
> - (*virt_fns)[i]->function,
> - &pciConfigAddr) < 0) {
> + if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i]))) {
There is one more thing that this change does, previously it would fail for
non-existing device, which it will not now. I don't think that is a problem,
but wanted to mention that.
Well spotted; in this case, however, the addresses were obtained
directly from sysfs through virPCIGetVirtualFunctions() so, as you
mention, validating them again is unnecessary.
> virReportSystemError(ENOSYS, "%s",
> _("Failed to get PCI Config Address
String"));
You should remove this error message as virAsprintf() in
virPCIDeviceAddressAsString() already sets an OOM Error.
Done.
I you like working on this function there are lot of things that leak
from the
loop, so looking at that would be great, thanks.
I'll see what I can do :)
--
Andrea Bolognani / Red Hat / Virtualization
2270
days inactive
2270
days old
7 comments
2 participants
participants (2)
-
Andrea Bolognani -
Martin Kletzander