[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Removing devicePath member from dev struct



On Wed, Nov 18, 2009 at 04:48:42PM -0500, Dave Allan wrote:
> I realized that I inadvertently added a member to the def struct to  
> contain each device's sysfs path when there was an existing member in  
> the dev struct for "OS specific path to device metadat, eg sysfs"  Since  
>  the udev backend needs to record the sysfs path while it's in the  
> process of creating the device, before the dev struct gets allocated, I  
> chose to remove the member from the dev struct.  I've attached a patch.  
> An alternative would be to store the information twice, but that seems  
> crufty to me.
>
> Dave

> >From 74b97811c298653924b10ce29c26e4282c3786a3 Mon Sep 17 00:00:00 2001
> From: David Allan <dallan redhat com>
> Date: Wed, 18 Nov 2009 16:10:32 -0500
> Subject: [PATCH 1/1] Remove devicePath member from dev struct to avoid redundancy
> 
> ---
>  src/conf/node_device_conf.c          |    1 -
>  src/conf/node_device_conf.h          |    1 -
>  src/node_device/node_device_driver.c |    2 +-
>  src/node_device/node_device_hal.c    |    2 +-
>  src/node_device/node_device_udev.c   |    7 -------
>  5 files changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index f55c9c7..f2faeec 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -156,7 +156,6 @@ void virNodeDeviceObjFree(virNodeDeviceObjPtr dev)
>      if (!dev)
>          return;
> 
> -    VIR_FREE(dev->devicePath);
>      virNodeDeviceDefFree(dev->def);
>      if (dev->privateFree)
>          (*dev->privateFree)(dev->privateData);
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 639a7e7..7a20bd6 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -178,7 +178,6 @@ typedef virNodeDeviceObj *virNodeDeviceObjPtr;
>  struct _virNodeDeviceObj {
>      virMutex lock;
> 
> -    char *devicePath;                   /* OS specific path to device metadat, eg sysfs */
>      virNodeDeviceDefPtr def;		/* device definition */
>      void *privateData;			/* driver-specific private data */
>      void (*privateFree)(void *data);	/* destructor for private data */
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index cddd994..7aed916 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -85,7 +85,7 @@ static int update_driver_name(virConnectPtr conn,
> 
>      VIR_FREE(dev->def->driver);
> 
> -    if (virAsprintf(&driver_link, "%s/driver", dev->devicePath) < 0) {
> +    if (virAsprintf(&driver_link, "%s/driver", dev->def->sysfs_path) < 0) {
>          virReportOOMError(conn);
>          goto cleanup;
>      }
> diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
> index 1e1d872..31c1764 100644
> --- a/src/node_device/node_device_hal.c
> +++ b/src/node_device/node_device_hal.c
> @@ -470,7 +470,7 @@ static void dev_create(const char *udi)
> 
>      dev->privateData = privData;
>      dev->privateFree = free_udi;
> -    dev->devicePath = devicePath;
> +    dev->def->sysfs_path = devicePath;
> 
>      virNodeDeviceObjUnlock(dev);
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 4ddf360..9b48052 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1230,13 +1230,6 @@ static int udevAddOneDevice(struct udev_device *device)
>          goto out;
>      }
> 
> -    dev->devicePath = strdup(udev_device_get_devpath(device));
> -    if (dev->devicePath == NULL) {
> -        virReportOOMError(NULL);
> -        virNodeDeviceObjRemove(&driverState->devs, dev);
> -        goto out;
> -    }
> -
>      virNodeDeviceObjUnlock(dev);
> 
>      ret = 0;

  ACK, it's a cleanup, so please commit before tomorrow's release,

   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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]