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/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
