[libvirt] [PATCH 11/18] virpcimock: Store PCI address as ints not string
Daniel Henrique Barboza
danielhb413 at gmail.com
Fri Aug 16 20:56:09 UTC 2019
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 <mprivozn at redhat.com>
> ---
> 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 <danielhb413 at gmail.com>
> 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(&ret, 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,
> + &addr->domain, &addr->bus,
> + &addr->device, &addr->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(&dev->addr)))
> + return NULL;
> +
> if (file) {
> ignore_value(virAsprintfQuiet(&ret, "%s" SYSFS_PCI_PREFIX "devices/%s/%s",
> - prefix, dev->id, file));
> + prefix, devid, file));
> } else {
> ignore_value(virAsprintfQuiet(&ret, "%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(&data->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(&dev->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(&addr, tmp) < 0)
> return NULL;
>
> - return pci_device_find_by_id(tmp);
> + return pci_device_find_by_id(&addr);
> }
>
> 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;
> VIR_AUTOFREE(char *) devpath = NULL;
> VIR_AUTOFREE(char *) driverpath = NULL;
>
> @@ -653,8 +694,9 @@ pci_driver_bind(struct pciDriver *driver,
> /* Make symlink under driver tree */
> VIR_FREE(devpath);
> VIR_FREE(driverpath);
> - if (!(devpath = pci_device_get_path(dev, NULL, true)) ||
> - !(driverpath = pci_driver_get_path(driver, dev->id, true))) {
> + if (!(devid = pci_address_format(&dev->addr)) ||
> + !(devpath = pci_device_get_path(dev, NULL, true)) ||
> + !(driverpath = pci_driver_get_path(driver, devid, true))) {
> errno = ENOMEM;
> return -1;
> }
> @@ -670,6 +712,7 @@ static int
> pci_driver_unbind(struct pciDriver *driver,
> struct pciDevice *dev)
> {
> + VIR_AUTOFREE(char *) devid = NULL;
> VIR_AUTOFREE(char *) devpath = NULL;
> VIR_AUTOFREE(char *) driverpath = NULL;
>
> @@ -680,8 +723,9 @@ pci_driver_unbind(struct pciDriver *driver,
> }
>
> /* Make symlink under device tree */
> - if (!(devpath = pci_device_get_path(dev, "driver", true)) ||
> - !(driverpath = pci_driver_get_path(driver, dev->id, true))) {
> + if (!(devid = pci_address_format(&dev->addr)) ||
> + !(devpath = pci_device_get_path(dev, "driver", true)) ||
> + !(driverpath = pci_driver_get_path(driver, devid, true))) {
> errno = ENOMEM;
> return -1;
> }
> @@ -913,8 +957,10 @@ init_env(void)
>
> # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \
> do { \
> - struct pciDevice dev = {.id = Id, .vendor = Vendor, \
> + struct pciDevice dev = {.vendor = Vendor, \
> .device = Device, __VA_ARGS__}; \
> + if (pci_address_parse(&dev.addr, Id) < 0) \
> + ABORT("Unable to parse PCI address " Id); \
> pci_device_new_from_stub(&dev); \
> } while (0)
>
More information about the libvir-list
mailing list