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

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/