Re: [PATCH 11/18] virpcimock: Store PCI address as ints not string

Daniel Henrique Barboza <danielhb413@xxxxxxxxx> · Fri, 16 Aug 2019 17:56:09 -0300

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@xxxxxxxxxx>
---
  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@xxxxxxxxx>



  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)
  

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list






