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.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list