[libvirt] PATCH: Include OS driver name in device information

[Mark McLoughlin]

Hey,

Looks fine to me, but if you're looking for comments ... :-)

On Fri, 2009-05-29 at 14:39 +0100, Daniel P. Berrange wrote:

> To, also include the operating system driver name. 
> 
> $ virsh nodedev-dumpxml pci_8086_27d6
> <device>
>   <name>pci_8086_27d6</name>
>   <parent>computer</parent>
>   <driver>
>     <name>pcieport-driver</name>
>   </driver>
>   <capability type='pci'>
>     <domain>0</domain>
>     <bus>0</bus>
>     <slot>28</slot>
>     <function>3</function>
>     <product id='0x27d6'>82801G (ICH7 Family) PCI Express Port 4</product>
>     <vendor id='0x8086'>Intel Corporation</vendor>
>   </capability>
> </device>
>
> We're not defining any particular semantics for the driver name, it is
> just a opaque string following rules of the OS in question.

The obvious question is "what's this for?"

If this is just an opaque string, what should people use it for?

> diff -r ec78a5d6c00c src/node_device.c
> --- a/src/node_device.c	Thu May 28 14:42:24 2009 +0100
> +++ b/src/node_device.c	Thu May 28 16:00:12 2009 +0100
> @@ -46,6 +46,60 @@ static int dev_has_cap(const virNodeDevi
>      return 0;
>  }
>  
> +#ifdef __linux__
> +static int update_driver_name(virConnectPtr conn,
> +                              virNodeDeviceObjPtr dev)
> +{
> +    char *driver_link = NULL;
> +    char devpath[PATH_MAX];
> +    char *p;
> +    int ret = -1;
> +    int n;
> +
> +    VIR_FREE(dev->def->driver);
> +
> +    if (virAsprintf(&driver_link, "%s/driver", dev->devicePath) < 0) {
> +        virReportOOMError(conn);
> +        goto cleanup;
> +    }
> +
> +    /* Some devices don't have an explicit driver, so just return
> +       without a name */
> +    if (access(driver_link, R_OK) < 0) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    if ((n = readlink(driver_link, devpath, sizeof devpath)) < 0) {
> +        virReportSystemError(conn, errno,
> +                             _("cannot resolve driver link %s"), driver_link);

Do we really want dumpxml to fail for this?

> +        goto cleanup;
> +    }
> +    devpath[n] = '\0';
> +
> +    p = strrchr(devpath, '/');
> +    if (p) {
> +        dev->def->driver = strdup(p+1);

basename() ?

(you already use basename() in storage_backend_disk.c)

> diff -r ec78a5d6c00c src/node_device_hal.c
> --- a/src/node_device_hal.c	Thu May 28 14:42:24 2009 +0100
> +++ b/src/node_device_hal.c	Thu May 28 16:00:12 2009 +0100

Probably should add a FIXME in node_device_devkit.c about adding support
for this.

Cheers,
Mark.