Re: [libvirt] [PATCH 11/18] virpcimock: Store PCI address as ints not string
On 8/16/19 10:56 PM, Daniel Henrique Barboza wrote:
>
>
> On 8/14/19 8:57 AM, Michal Privoznik wrote:
>> In upcoming patches we will need only some portions of the PCI
>> address. To construct that easily, it's better if the PCI address
>> of a device is stored as four integers rather than one string.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> tests/virpcimock.c | 76 +-
>> 1 file changed, 61 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
>> index de365cdb78..c10764dcdd 100644
>> --- a/tests/virpcimock.c
>> +++ b/tests/virpcimock.c
>> @@ -118,8 +118,16 @@ struct pciDriver {
>> unsigned int fail; /* Bitwise-OR of driverActions that should
>> fail */
>> };
>> +struct pciDeviceAddress {
>> + unsigned int domain;
>> + unsigned int bus;
>> + unsigned int device;
>> + unsigned int function;
>> +};
>> +# define ADDR_STR_FMT "%04x:%02x:%02x.%d"
>> +
>
> I was going to complain "this stuff is similar to what we already have
> in utils/virpci.c, why can't we use it here", but in a second thought I
> realized virpci.h is too big of a import just for a macro and a couple
> of parse functions.
Yet again, great minds think alike :-) This was exactly my reasoning for
not including virpci.h. I mean, I wanted to but then realized it's
probably better if we keep the mock separate.
>
>
> Reviewed-by: Daniel Henrique Barboza
Thanks,
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/18] virpcimock: Store PCI address as ints not string
On 8/14/19 8:57 AM, Michal Privoznik wrote:
In upcoming patches we will need only some portions of the PCI
address. To construct that easily, it's better if the PCI address
of a device is stored as four integers rather than one string.
Signed-off-by: Michal Privoznik
---
tests/virpcimock.c | 76 +-
1 file changed, 61 insertions(+), 15 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index de365cdb78..c10764dcdd 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -118,8 +118,16 @@ struct pciDriver {
unsigned int fail; /* Bitwise-OR of driverActions that should fail */
};
+struct pciDeviceAddress {
+unsigned int domain;
+unsigned int bus;
+unsigned int device;
+unsigned int function;
+};
+# define ADDR_STR_FMT "%04x:%02x:%02x.%d"
+
I was going to complain "this stuff is similar to what we already have
in utils/virpci.c, why can't we use it here", but in a second thought I
realized virpci.h is too big of a import just for a macro and a couple
of parse functions.
Reviewed-by: Daniel Henrique Barboza
struct pciDevice {
-const char *id;
+struct pciDeviceAddress addr;
int vendor;
int device;
int klass;
@@ -145,7 +153,7 @@ static void init_env(void);
static int pci_device_autobind(struct pciDevice *dev);
static void pci_device_new_from_stub(const struct pciDevice *data);
-static struct pciDevice *pci_device_find_by_id(const char *id);
+static struct pciDevice *pci_device_find_by_id(struct pciDeviceAddress const
*addr);
static struct pciDevice *pci_device_find_by_content(const char *path);
static void pci_driver_new(const char *name, int fail, ...);
@@ -337,6 +345,29 @@ remove_fd(int fd)
/*
* PCI Device functions
*/
+static char *
+pci_address_format(struct pciDeviceAddress const *addr)
+{
+char *ret;
+
+ignore_value(virAsprintfQuiet(, ADDR_STR_FMT,
+ addr->domain, addr->bus,
+ addr->device, addr->function));
+return ret;
+}
+
+static int
+pci_address_parse(struct pciDeviceAddress *addr,
+ const char *buf)
+{
+if (sscanf(buf, ADDR_STR_FMT,
+ >domain, >bus,
+ >device, >function) != 4)
+return -1;
+return 0;
+}
+
+
static char *
pci_device_get_path(const struct pciDevice *dev,
const char *file,
@@ -344,16 +375,20 @@ pci_device_get_path(const struct pciDevice *dev,
{
char *ret = NULL;
const char *prefix = "";
+VIR_AUTOFREE(char *) devid = NULL;
if (faked)
prefix = fakerootdir;
+if (!(devid = pci_address_format(>addr)))
+return NULL;
+
if (file) {
ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX
"devices/%s/%s",
- prefix, dev->id, file));
+ prefix, devid, file));
} else {
ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX
"devices/%s",
- prefix, dev->id));
+ prefix, devid));
}
return ret;
@@ -366,13 +401,15 @@ pci_device_new_from_stub(const struct pciDevice *data)
struct pciDevice *dev;
VIR_AUTOFREE(char *) devpath = NULL;
VIR_AUTOFREE(char *) id = NULL;
+VIR_AUTOFREE(char *) devid = NULL;
char *c;
VIR_AUTOFREE(char *) configSrc = NULL;
char tmp[256];
struct stat sb;
bool configSrcExists = false;
-if (VIR_STRDUP_QUIET(id, data->id) < 0)
+if (!(devid = pci_address_format(>addr)) ||
+VIR_STRDUP_QUIET(id, devid) < 0)
ABORT_OOM();
/* Replace ':' with '-' to create the config filename from the
@@ -452,20 +489,20 @@ pci_device_new_from_stub(const struct pciDevice *data)
make_symlink(devpath, "iommu_group", tmp);
if (pci_device_autobind(dev) < 0)
-ABORT("Unable to bind: %s", data->id);
+ABORT("Unable to bind: %s", devid);
if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0)
ABORT_OOM();
}
static struct pciDevice *
-pci_device_find_by_id(const char *id)
+pci_device_find_by_id(struct pciDeviceAddress const *addr)
{
size_t i;
for (i = 0; i < nPCIDevices; i++) {
struct pciDevice *dev = pciDevices[i];
-if (STREQ(dev->id, id))
+if (!memcmp(>addr, addr, sizeof(*addr)))
return dev;
}
@@ -476,11 +513,13 @@ static struct pciDevice *
pci_device_find_by_content(const char *path)
{
char tmp[32];
+struct pciDeviceAddress addr;
-if (pci_read_file(path, tmp, sizeof(tmp), true) < 0)
+if (pci_read_file(path, tmp, sizeof(tmp), true) < 0 ||
+pci_address_parse(, tmp) < 0)
return NULL;
-return pci_device_find_by_id(tmp);
+return pci_device_find_by_id();
}
static int
@@
[libvirt] [PATCH 11/18] virpcimock: Store PCI address as ints not string
In upcoming patches we will need only some portions of the PCI
address. To construct that easily, it's better if the PCI address
of a device is stored as four integers rather than one string.
Signed-off-by: Michal Privoznik
---
tests/virpcimock.c | 76 +-
1 file changed, 61 insertions(+), 15 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index de365cdb78..c10764dcdd 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -118,8 +118,16 @@ struct pciDriver {
unsigned int fail; /* Bitwise-OR of driverActions that should fail */
};
+struct pciDeviceAddress {
+unsigned int domain;
+unsigned int bus;
+unsigned int device;
+unsigned int function;
+};
+# define ADDR_STR_FMT "%04x:%02x:%02x.%d"
+
struct pciDevice {
-const char *id;
+struct pciDeviceAddress addr;
int vendor;
int device;
int klass;
@@ -145,7 +153,7 @@ static void init_env(void);
static int pci_device_autobind(struct pciDevice *dev);
static void pci_device_new_from_stub(const struct pciDevice *data);
-static struct pciDevice *pci_device_find_by_id(const char *id);
+static struct pciDevice *pci_device_find_by_id(struct pciDeviceAddress const
*addr);
static struct pciDevice *pci_device_find_by_content(const char *path);
static void pci_driver_new(const char *name, int fail, ...);
@@ -337,6 +345,29 @@ remove_fd(int fd)
/*
* PCI Device functions
*/
+static char *
+pci_address_format(struct pciDeviceAddress const *addr)
+{
+char *ret;
+
+ignore_value(virAsprintfQuiet(, ADDR_STR_FMT,
+ addr->domain, addr->bus,
+ addr->device, addr->function));
+return ret;
+}
+
+static int
+pci_address_parse(struct pciDeviceAddress *addr,
+ const char *buf)
+{
+if (sscanf(buf, ADDR_STR_FMT,
+ >domain, >bus,
+ >device, >function) != 4)
+return -1;
+return 0;
+}
+
+
static char *
pci_device_get_path(const struct pciDevice *dev,
const char *file,
@@ -344,16 +375,20 @@ pci_device_get_path(const struct pciDevice *dev,
{
char *ret = NULL;
const char *prefix = "";
+VIR_AUTOFREE(char *) devid = NULL;
if (faked)
prefix = fakerootdir;
+if (!(devid = pci_address_format(>addr)))
+return NULL;
+
if (file) {
ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX
"devices/%s/%s",
- prefix, dev->id, file));
+ prefix, devid, file));
} else {
ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX "devices/%s",
- prefix, dev->id));
+ prefix, devid));
}
return ret;
@@ -366,13 +401,15 @@ pci_device_new_from_stub(const struct pciDevice *data)
struct pciDevice *dev;
VIR_AUTOFREE(char *) devpath = NULL;
VIR_AUTOFREE(char *) id = NULL;
+VIR_AUTOFREE(char *) devid = NULL;
char *c;
VIR_AUTOFREE(char *) configSrc = NULL;
char tmp[256];
struct stat sb;
bool configSrcExists = false;
-if (VIR_STRDUP_QUIET(id, data->id) < 0)
+if (!(devid = pci_address_format(>addr)) ||
+VIR_STRDUP_QUIET(id, devid) < 0)
ABORT_OOM();
/* Replace ':' with '-' to create the config filename from the
@@ -452,20 +489,20 @@ pci_device_new_from_stub(const struct pciDevice *data)
make_symlink(devpath, "iommu_group", tmp);
if (pci_device_autobind(dev) < 0)
-ABORT("Unable to bind: %s", data->id);
+ABORT("Unable to bind: %s", devid);
if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0)
ABORT_OOM();
}
static struct pciDevice *
-pci_device_find_by_id(const char *id)
+pci_device_find_by_id(struct pciDeviceAddress const *addr)
{
size_t i;
for (i = 0; i < nPCIDevices; i++) {
struct pciDevice *dev = pciDevices[i];
-if (STREQ(dev->id, id))
+if (!memcmp(>addr, addr, sizeof(*addr)))
return dev;
}
@@ -476,11 +513,13 @@ static struct pciDevice *
pci_device_find_by_content(const char *path)
{
char tmp[32];
+struct pciDeviceAddress addr;
-if (pci_read_file(path, tmp, sizeof(tmp), true) < 0)
+if (pci_read_file(path, tmp, sizeof(tmp), true) < 0 ||
+pci_address_parse(, tmp) < 0)
return NULL;
-return pci_device_find_by_id(tmp);
+return pci_device_find_by_id();
}
static int
@@ -607,6 +646,7 @@ pci_driver_find_by_path(const char *path)
static struct pciDriver *
pci_driver_find_by_driver_override(struct pciDevice *dev)
{
+VIR_AUTOFREE(char *) devid = NULL;
VIR_AUTOFREE(char *) path = NULL;
char tmp[32];
size_t i;
@@ -631,6 +671,7 @@ static int
pci_driver_bind(struct pciDriver *driver,
struct pciDevice *dev)
{
+VIR_AUTOFREE(char *) devid = NULL;
