Pino Toscano
2017-Apr-13 15:10 UTC
Re: [Libguestfs] [PATCH v2 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
On Thursday, 13 April 2017 16:55:26 CEST Pavel Butsykin wrote:> Add udev_settle_file() to run 'udevadm settle' with --exit-if-exists option. It > will slightly reduce the waiting-time for pending events if we need to wait > for events related to a particular device/file. > > Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> > --- > daemon/daemon.h | 2 ++ > daemon/guestfsd.c | 39 ++++++++++++++++++++++++++++----------- > 2 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/daemon/daemon.h b/daemon/daemon.h > index 79a5288f6..90ebaafbe 100644 > --- a/daemon/daemon.h > +++ b/daemon/daemon.h > @@ -141,6 +141,8 @@ extern int parse_btrfsvol (const char *desc, mountable_t *mountable); > > extern int prog_exists (const char *prog); > > +extern void udev_settle_file (const char *file); > + > extern void udev_settle (void); > > extern int random_name (char *template); > diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c > index 85ce5d2ad..b82c98acd 100644 > --- a/daemon/guestfsd.c > +++ b/daemon/guestfsd.c > @@ -63,6 +63,8 @@ > > #include "daemon.h" > > +#define MAX_ARGS 64 > + > GUESTFSD_EXT_CMD(str_udevadm, udevadm); > GUESTFSD_EXT_CMD(str_uuidgen, uuidgen); > > @@ -1213,20 +1215,35 @@ random_name (char *template) > * fussed if it fails. > */ > void > -udev_settle (void) > +udev_settle_file (const char *file) > { > - char cmd[80]; > + const char *argv[MAX_ARGS]; > + CLEANUP_FREE char *err = NULL; > + size_t i = 0; > int r; > > - snprintf (cmd, sizeof cmd, "%s%s settle", > - str_udevadm, verbose ? " --debug" : ""); > - if (verbose) > - printf ("%s\n", cmd); > - r = system (cmd); > - if (r == -1) > - perror ("system"); > - else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) > - fprintf (stderr, "warning: udevadm command failed\n"); > + ADD_ARG (argv, i, str_udevadm); > + if (verbose) { > + ADD_ARG (argv, i, "--debug"); > + } > + > + ADD_ARG (argv, i, "settle"); > + if (file) { > + ADD_ARG (argv, i, "-E"); > + ADD_ARG (argv, i, file); > + } > + ADD_ARG (argv, i, NULL); > + > + r = commandv (NULL, &err, argv); > + if (r == -1) { > + reply_with_error ("udevadm command failed: %s", err);IMHO reply_with_error is not correct here -- so far a failure to run udevadm is not fatal at all, so it should be kept that way. Also most probably commandrv, so we can still - show perror when udevadm could not be run (return code = -1) - show the warning when udevadm failed to run (exit code != 0) The rest of the changes looks good. Thanks, -- Pino Toscano
Reasonably Related Threads
- Re: [PATCH 1/2] daemon: run 'udevadm settle' with --exit-if-exists option
- [PATCH] daemon: remove call to obsolete udevsettle
- [PATCH v4 5/7] daemon: add get_random_uuid
- Re: [PATCH v2 2/2] daemon: add udev_settle_file to is_root_device
- [PATCH] daemon: use str_udevadm in udev_settle