[Libguestfs] [PATCH v2 1/2] daemon: run 'udevadm settle' with --exit-if-exists option

Pavel Butsykin pbutsykin at virtuozzo.com
Thu Apr 13 16:01:09 UTC 2017


On 13.04.2017 18:10, Pino Toscano wrote:
> 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 at 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.

I missed that that reply_with_error is not just error output, but may
lead to termination of the client. Agree.

> 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)

will fix.

> The rest of the changes looks good.

Thanks! Can you add me in CC when you reply to my messages?

> Thanks,
>
>
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
>




More information about the Libguestfs mailing list