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

[Daniel P. Berrange]

On Fri, Nov 28, 2008 at 03:50:11PM +0100, Chris Lalancette wrote:
> Alan Pevec wrote:
> > On Thu, Nov 27, 2008 at 8:53 AM, Chris Lalancette <clalance at redhat.com
> > <mailto:clalance at redhat.com>> wrote:
> > 
> >     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".
> > 
> > 
> > Please make it try both: RHEL5 has an ancient udev095 and doesn't have
> > udevadm but it does have udevsettle
> 
> Sigh.  Silly older distros :).  Thanks for the heads up, though, good to know it
> now rather than later.  Unfortunately, it's not so easy to make it try both; we
> would get into a twisty passage of #ifdef's, I think.  Dan, what do you think of
> the attached patch for older udev compatibility?

>  
>  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.31
> diff -u -r1.31 storage_backend.c
> --- src/storage_backend.c	28 Nov 2008 07:50:20 -0000	1.31
> +++ src/storage_backend.c	28 Nov 2008 14:50:11 -0000
> @@ -287,6 +287,23 @@
>       */
>      virRun(conn, settleprog, &exitstatus);
>  }
> +#elif defined(UDEVSETTLE)
> +void virStorageBackendWaitForDevices(virConnectPtr conn)
> +{
> +    const char *const settleprog[] = { UDEVSETTLE, NULL };
> +    int exitstatus;
> +
> +    if (access(UDEVSETTLE, 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);
> +}

This seems rather overkill when you could just do

#if defined(UDEVADM) || defined(UDEVSETTLE)
void virStorageBackendWaitForDevices(virConnectPtr conn)
{
#ifdef UDEVADM
    const char *const settleprog[] = { UDEVADM, "settle", NULL };
#else
    const char *const settleprog[] = { UDEVSETTLE, NULL };
#endif
    int exitstatus;


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|