1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/4] virutil: Rework how 'udevadm settle' is called
  4. View patch

[libvirt] [PATCH 3/4] m4: Provide default value fore UDEVADM

Michal Privoznik posted 4 patches 6 years, 8 months ago
[libvirt] [PATCH 3/4] m4: Provide default value fore UDEVADM
Posted by Michal Privoznik 6 years, 8 months ago 
https://bugzilla.redhat.com/show_bug.cgi?id=1710575

It may happen that the system where libvirt is built at doesn't
have udevadm binary but the one where it runs does have it.
If we change how udevadm is ran in virWaitForDevices() then we
can safely pass a default value in m4 macro.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 m4/virt-external-programs.m4 |  9 +++------
 src/util/virutil.c           | 14 ++++++--------
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index 3c915e1a65..f1ae104b32 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -45,7 +45,7 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
   AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq], [$LIBVIRT_SBIN_PATH])
   AC_PATH_PROG([RADVD], [radvd], [radvd], [$LIBVIRT_SBIN_PATH])
   AC_PATH_PROG([TC], [tc], [tc], [$LIBVIRT_SBIN_PATH])
-  AC_PATH_PROG([UDEVADM], [udevadm], [], [$LIBVIRT_SBIN_PATH])
+  AC_PATH_PROG([UDEVADM], [udevadm], [udevadm], [$LIBVIRT_SBIN_PATH])
   AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [$LIBVIRT_SBIN_PATH])
   AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [$LIBVIRT_SBIN_PATH])
   AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], [$LIBVIRT_SBIN_PATH])
@@ -65,11 +65,8 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
                      [Location or name of the mm-ctl program])
   AC_DEFINE_UNQUOTED([OVSVSCTL], ["$OVSVSCTL"],
                      [Location or name of the ovs-vsctl program])
-
-  if test -n "$UDEVADM"; then
-    AC_DEFINE_UNQUOTED([UDEVADM], ["$UDEVADM"],
-                       [Location or name of the udevadm program])
-  fi
+  AC_DEFINE_UNQUOTED([UDEVADM], ["$UDEVADM"],
+                     [Location or name of the udevadm program])
   if test -n "$MODPROBE"; then
     AC_DEFINE_UNQUOTED([MODPROBE], ["$MODPROBE"],
                        [Location or name of the modprobe program])
diff --git a/src/util/virutil.c b/src/util/virutil.c
index ecef24d2f3..2db0e9769c 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1479,25 +1479,23 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups,
 #endif
 
 
-#if defined(UDEVADM)
 void virWaitForDevices(void)
 {
-    const char *const settleprog[] = { UDEVADM, "settle", NULL };
+    VIR_AUTOPTR(virCommand) cmd = NULL;
+    VIR_AUTOFREE(char *) udev = NULL;
     int exitstatus;
 
-    if (access(settleprog[0], X_OK) != 0)
+    if (!(udev = virFindFileInPath(UDEVADM)) ||
+        !virFileIsExecutable(udev) ||
+        !(cmd = virCommandNewArgList(udev, "settle", NULL)))
         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.
      */
-    ignore_value(virRun(settleprog, &exitstatus));
+    ignore_value(virCommandRun(cmd, &exitstatus));
 }
-#else
-void virWaitForDevices(void)
-{}
-#endif
 
 #if WITH_DEVMAPPER
 bool
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] m4: Provide default value fore UDEVADM
Posted by Ján Tomko 6 years, 8 months ago 
On Fri, May 17, 2019 at 11:54:16AM +0200, Michal Privoznik wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1710575
>
>It may happen that the system where libvirt is built at doesn't
>have udevadm binary but the one where it runs does have it.
>If we change how udevadm is ran in virWaitForDevices() then we

s/ran/run/

>can safely pass a default value in m4 macro.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> m4/virt-external-programs.m4 |  9 +++------
> src/util/virutil.c           | 14 ++++++--------
> 2 files changed, 9 insertions(+), 14 deletions(-)
>
>diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
>index 3c915e1a65..f1ae104b32 100644
>--- a/m4/virt-external-programs.m4
>+++ b/m4/virt-external-programs.m4
>@@ -45,7 +45,7 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
>   AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq], [$LIBVIRT_SBIN_PATH])
>   AC_PATH_PROG([RADVD], [radvd], [radvd], [$LIBVIRT_SBIN_PATH])
>   AC_PATH_PROG([TC], [tc], [tc], [$LIBVIRT_SBIN_PATH])
>-  AC_PATH_PROG([UDEVADM], [udevadm], [], [$LIBVIRT_SBIN_PATH])
>+  AC_PATH_PROG([UDEVADM], [udevadm], [udevadm], [$LIBVIRT_SBIN_PATH])
>   AC_PATH_PROG([MODPROBE], [modprobe], [modprobe], [$LIBVIRT_SBIN_PATH])
>   AC_PATH_PROG([RMMOD], [rmmod], [rmmod], [$LIBVIRT_SBIN_PATH])
>   AC_PATH_PROG([MMCTL], [mm-ctl], [mm-ctl], [$LIBVIRT_SBIN_PATH])
>@@ -65,11 +65,8 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
>                      [Location or name of the mm-ctl program])
>   AC_DEFINE_UNQUOTED([OVSVSCTL], ["$OVSVSCTL"],
>                      [Location or name of the ovs-vsctl program])
>-
>-  if test -n "$UDEVADM"; then
>-    AC_DEFINE_UNQUOTED([UDEVADM], ["$UDEVADM"],
>-                       [Location or name of the udevadm program])
>-  fi
>+  AC_DEFINE_UNQUOTED([UDEVADM], ["$UDEVADM"],
>+                     [Location or name of the udevadm program])
>   if test -n "$MODPROBE"; then
>     AC_DEFINE_UNQUOTED([MODPROBE], ["$MODPROBE"],
>                        [Location or name of the modprobe program])
>diff --git a/src/util/virutil.c b/src/util/virutil.c
>index ecef24d2f3..2db0e9769c 100644
>--- a/src/util/virutil.c
>+++ b/src/util/virutil.c
>@@ -1479,25 +1479,23 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups,
> #endif
>
>
>-#if defined(UDEVADM)
> void virWaitForDevices(void)
> {
>-    const char *const settleprog[] = { UDEVADM, "settle", NULL };
>+    VIR_AUTOPTR(virCommand) cmd = NULL;
>+    VIR_AUTOFREE(char *) udev = NULL;
>     int exitstatus;
>
>-    if (access(settleprog[0], X_OK) != 0)
>+    if (!(udev = virFindFileInPath(UDEVADM)) ||
>+        !virFileIsExecutable(udev) ||

virFileIsExecutable is not necessary here, virFindFileInPath has already
done the check.

>+        !(cmd = virCommandNewArgList(udev, "settle", NULL)))

Please give this condition a separate 'if'.

>         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.
>      */

I'm not sure what kind of errors can happen here, but we should be able
to expect that 'udevadm' is present on a Linux system. Reporting an
error in that case might be better than silently dropping devices.

But strictly speaking, this patch is an improvement, so:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

>-    ignore_value(virRun(settleprog, &exitstatus));
>+    ignore_value(virCommandRun(cmd, &exitstatus));
> }
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.