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

Re: [libvirt] [PATCH 2/2]: Call udevsettle in the appropriate places



Daniel P. Berrange wrote:
>> Index: src/storage_backend.c
>> ===================================================================
>> RCS file: /data/cvs/libvirt/src/storage_backend.c,v
>> retrieving revision 1.29
>> diff -u -r1.29 storage_backend.c
>> --- a/src/storage_backend.c	17 Nov 2008 11:19:33 -0000	1.29
>> +++ b/src/storage_backend.c	26 Nov 2008 12:40:19 -0000
>> @@ -270,6 +270,20 @@
>>      return 0;
>>  }
>>  
>> +void virStorageBackendWaitForDevices(virConnectPtr conn)
>> +{
>> +    const char *const settleprog[] = { UDEVSETTLE, NULL };
>> +    int exitstatus;
>> +
>> +    /*
>> +     * NOTE: we ignore errors here; this is just to make sure that any device
>> +     * nodes that are being created finish before we try to
>> +     * scan them.  If this fails (udevsettle exits badly, or doesn't exist
>> +     * at all), then we still have a fallback mechanism anyway.
>> +     */
>> +    virRun(conn, settleprog, &exitstatus);
>> +}
> 
> If we know it doesn't exist, we should not try to run it.
> 
> 
>> Index: configure.in
>> ===================================================================
>> RCS file: /data/cvs/libvirt/configure.in,v
>> retrieving revision 1.187
>> diff -u -r1.187 configure.in
>> --- a/configure.in	25 Nov 2008 15:48:11 -0000	1.187
>> +++ b/configure.in	26 Nov 2008 12:40:19 -0000
>> @@ -115,11 +115,15 @@
>>  	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
>>  AC_PATH_PROG([BRCTL], [brctl], [brctl],
>>  	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
>> +AC_PATH_PROG([UDEVSETTLE], [udevsettle], [udevsettle],
>> +	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
> 
> This will set UDEVSETTLE=udevsettle even if it doesn't exist. This will
> result in us running it on every OS we build on. We should not set this
> variable if its not found.

Right, both points make sense.  I think the following patch should address it; I
only conditionally set the UDEVADM variable if I find it.  So, for machines
without it, the meat of virStorageBackendWaitForDevices is compiled out.  In
places where I've found it on the build machine, I then do "access" for
executable at runtime, and only if that succeeds do I run it.  Does that seem
correct?  In addition, based on the comment from Guido, I changed it over to use
"udevadm settle" instead of "udevsettle".

Signed-off-by: Chris Lalancette <clalance redhat com>
Index: configure.in
===================================================================
RCS file: /data/cvs/libvirt/configure.in,v
retrieving revision 1.187
diff -u -r1.187 configure.in
--- a/configure.in	25 Nov 2008 15:48:11 -0000	1.187
+++ b/configure.in	27 Nov 2008 07:44:51 -0000
@@ -115,11 +115,17 @@
 	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
 AC_PATH_PROG([BRCTL], [brctl], [brctl],
 	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([UDEVADM], [udevadm], [],
+	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
 
 AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"],
         [Location or name of the dnsmasq program])
 AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"],
         [Location or name of the brctl program (see bridge-utils)])
+if test -n "$UDEVADM"; then
+  AC_DEFINE_UNQUOTED([UDEVADM],["$UDEVADM"],
+        [Location or name of the udevadm program])
+fi
 
 dnl Specific dir for HTML output ?
 AC_ARG_WITH([html-dir], [AC_HELP_STRING([--with-html-dir=path],
Index: src/storage_backend.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.29
diff -u -r1.29 storage_backend.c
--- a/src/storage_backend.c	17 Nov 2008 11:19:33 -0000	1.29
+++ b/src/storage_backend.c	27 Nov 2008 07:44:51 -0000
@@ -270,6 +270,25 @@
     return 0;
 }
 
+void virStorageBackendWaitForDevices(virConnectPtr conn)
+{
+#ifdef UDEVADM
+    const char *const settleprog[] = { UDEVADM, "settle", NULL };
+    int exitstatus;
+
+    if (access(UDEVADM, X_OK) != 0)
+        return;
+
+    /*
+     * NOTE: we ignore errors here; this is just to make sure that any device
+     * nodes that are being created finish before we try to scan them.
+     * If this fails for any reason, we still have the backup of polling for
+     * 5 seconds for device nodes.
+     */
+    virRun(conn, settleprog, &exitstatus);
+#endif
+}
+
 /*
  * Given a volume path directly in /dev/XXX, iterate over the
  * entries in the directory pool->def->target.path and find the
Index: src/storage_backend_iscsi.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_iscsi.c,v
retrieving revision 1.18
diff -u -r1.18 storage_backend_iscsi.c
--- a/src/storage_backend_iscsi.c	17 Nov 2008 11:19:33 -0000	1.18
+++ b/src/storage_backend_iscsi.c	27 Nov 2008 07:44:51 -0000
@@ -603,6 +603,8 @@
 
     pool->def->allocation = pool->def->capacity = pool->def->available = 0;
 
+    virStorageBackendWaitForDevices(conn);
+
     if ((session = virStorageBackendISCSISession(conn, pool)) == NULL)
         goto cleanup;
     if (virStorageBackendISCSIRescanLUNs(conn, pool, session) < 0)
Index: src/storage_backend_disk.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_disk.c,v
retrieving revision 1.20
diff -u -r1.20 storage_backend_disk.c
--- a/src/storage_backend_disk.c	17 Nov 2008 11:19:33 -0000	1.20
+++ b/src/storage_backend_disk.c	27 Nov 2008 07:44:51 -0000
@@ -262,6 +262,8 @@
     VIR_FREE(pool->def->source.devices[0].freeExtents);
     pool->def->source.devices[0].nfreeExtent = 0;
 
+    virStorageBackendWaitForDevices(conn);
+
     return virStorageBackendDiskReadPartitions(conn, pool, NULL);
 }
 
Index: src/storage_backend_logical.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_logical.c,v
retrieving revision 1.26
diff -u -r1.26 storage_backend_logical.c
--- a/src/storage_backend_logical.c	17 Nov 2008 11:19:33 -0000	1.26
+++ b/src/storage_backend_logical.c	27 Nov 2008 07:44:51 -0000
@@ -470,6 +470,8 @@
     };
     int exitstatus;
 
+    virStorageBackendWaitForDevices(conn);
+
     /* Get list of all logical volumes */
     if (virStorageBackendLogicalFindLVs(conn, pool, NULL) < 0) {
         virStoragePoolObjClearVols(pool);
Index: src/storage_backend.h
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.13
diff -u -r1.13 storage_backend.h
--- a/src/storage_backend.h	17 Nov 2008 11:19:33 -0000	1.13
+++ b/src/storage_backend.h	27 Nov 2008 07:44:51 -0000
@@ -69,6 +69,8 @@
                                      int fd,
                                      int withCapacity);
 
+void virStorageBackendWaitForDevices(virConnectPtr conn);
+
 char *virStorageBackendStablePath(virConnectPtr conn,
                                   virStoragePoolObjPtr pool,
                                   const char *devpath);

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