[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/5] Introduce functions for checking whether a pidfile is valid



Eh, I forgot to add some more notes...

...
> diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
> index 25c3272..dc92868 100644
> --- a/src/util/virpidfile.c
> +++ b/src/util/virpidfile.c
> @@ -24,6 +24,7 @@
>  #include <config.h>
>  
>  #include <fcntl.h>
> +#include <signal.h>
>  
>  #include "virpidfile.h"
>  #include "virfile.h"
> @@ -164,6 +165,63 @@ int virPidFileRead(const char *dir,
>  }
>  
>  
> +int virPidFileReadPathIfAlive(const char *path,
> +                              pid_t *pid,
> +                              const char *binpath)
> +{
> +    int rc;
> +    char *procpath = NULL;
> +
> +    rc = virPidFileReadPath(path, pid);
> +    if (rc < 0)
> +        return rc;
> +
> +    /* Check that it's still alive */
> +    if (kill(*pid, 0) < 0) {
> +        *pid = -1;
> +        return 0;
> +    }
> +
> +    if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0) {
> +        *pid = -1;
> +        return 0;
> +    }
> +#ifdef __linux__
> +    if (virFileLinkPointsTo(procpath, binpath) == 0)
> +        *pid = -1;
> +#endif
> +   VIR_FREE(procpath);

Indentation.

And anyway, what about implementing the second half of this function as
follows:

#ifdef __linux__
    if (binpath) {
        char *procpath = NULL;

        if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0 ||
            !virFileLinkPointsTo(procpath, binpath))
            *pid = -1;
        VIR_FREE(procpath);
    }
#endif

Building procpath makes little sense on non-linux and checking binpath first
allows callers to use *IfAlive for checking pid liveliness without binpath
checks (in case any caller wants to do so) and allows us to move procpath
declaration into the same ifdef block :-)

> +
> +    return 0;
> +}

I think we should also document that both *IfAlive functions return -errno if
the pid file cannot be read or doesn't contain integer value. If it contains a
number but such process is not running or doesn't correspond to binpath, the
return value is 0 but pid is -1.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]