Re: [libvirt] [PATCH] expose SR IOV physical/virtual function relationships

14 Dec 2009


      On Thu, Dec 10, 2009 at 04:51:12PM -0500, Dave Allan wrote:
...
...
ACK aside from the minor logging fixes&  attribute whitespace
Daniel
Ok, thanks.  Final version attached.
Dave
[...]
...
+    if ((address == NULL) || (logStrToLong_ui(address,
+                                              &p,
+                                              16,
+                                              &bdf->domain) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &bdf->bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &bdf->slot) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &bdf->function) == -1)) {
+        goto out;
+    }
I reformatted this a bit, no need to have one arg per line
...
+static int get_sriov_function(const char *device_link,
+                              struct pci_config_address **bdf)
+{
+    char *device_path = NULL, *config_address = NULL;
+    char errbuf[64];
+    int ret = SRIOV_ERROR;
[...]
+    device_path = realpath(device_link, device_path);
+    if (device_path == NULL) {
+        memset(errbuf, '\0', sizeof(errbuf));
+        VIR_ERROR("Failed to resolve device link '%s': '%s'\n", device_link,
+                  virStrerror(errno, errbuf, sizeof(errbuf)));
Hum  I wonder if we don't have a wrapper error  macro for this to use
  instead of allocating a small buffer on the stack, but it's minor.
...
+        goto out;
+    }
[...]
+            VIR_DEBUG("Number of virtual functions: %d\n", *num_funcs);
+            if (VIR_REALLOC_N(d->pci_dev.virtual_functions, (*num_funcs) + 1) != 0) {
I reformatted that line too
...
+                virReportOOMError(NULL);
+                goto out;
+            }
+
+            if (get_sriov_function(device_link,
+                                   &d->pci_dev.virtual_functions[*num_funcs]) !=
+                SRIOV_FOUND) {
there as well
...
+
+                /* We should not get back SRIOV_NOT_FOUND in this
+                 * case, so if we do, it's an error. */
+                VIR_ERROR("Failed to get SR IOV function from device link '%s'\n",
unneeded \n, fit in a line as a result
...
+                          device_link);
+                goto out;
+            } else {
+                (*num_funcs)++;
+                d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+            }
+
+            VIR_FREE(device_link);
+        }
+    }
+
+    closedir(dir);
+
+    ret = 0;
+
+out:
+    VIR_FREE(device_link);
+    return 0;
+}
+
 #endif /* __linux__ */
With those tiny fixes, I phsed the patch,

    thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/