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

[Pavel Butsykin]

On 13.04.2017 00:11, Richard W.M. Jones wrote:
> On Wed, Apr 12, 2017 at 05:01:10PM +0300, 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 | 20 ++++++++++++++++----
>>   2 files changed, 18 insertions(+), 4 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..dccfa15bc 100644
>> --- a/daemon/guestfsd.c
>> +++ b/daemon/guestfsd.c
>> @@ -1213,13 +1213,18 @@ random_name (char *template)
>>    * fussed if it fails.
>>    */
>>   void
>> -udev_settle (void)
>> +udev_settle_file (const char *file)
>>   {
>> -  char cmd[80];
>> +  size_t cmd_size = strlen (str_udevadm) +
>> +                    sizeof (" settle")   +
>> +                    sizeof (" --debug")  +
>> +                    (file ? sizeof (" --exit-if-exists=") + strlen (file) : 0);
>> +  char *cmd = malloc (cmd_size);
>>     int r;
>>
>> -  snprintf (cmd, sizeof cmd, "%s%s settle",
>> -            str_udevadm, verbose ? " --debug" : "");
>> +  snprintf(cmd, cmd_size, "%s%s settle%s%s",
>> +           str_udevadm, verbose ? " --debug" : "",
>> +           file ? " --exit-if-exists=" : "", file ? file : "");
>>     if (verbose)
>>       printf ("%s\n", cmd);
>>     r = system (cmd);
>> @@ -1227,6 +1232,13 @@ udev_settle (void)
>>       perror ("system");
>>     else if (!WIFEXITED (r) || WEXITSTATUS (r) != 0)
>>       fprintf (stderr, "warning: udevadm command failed\n");
>> +  free (cmd);
>
> There are some ugly quoting (and hence, possibly, security) problems
> with this patch.
>
> It's more secure and also much simpler to call ADD_ARG + commandv
> here.  For an example of how to use it, see:
>
>    https://github.com/libguestfs/libguestfs/blob/master/daemon/mkfs.c#L41-L247

I agree, it would be better, thanks!

> Rich.
>