[libvirt] [PATCH 3/5] Introduce functions for checking whether a pidfile is valid
Jiri Denemark
jdenemar at redhat.com
Fri Aug 12 12:10:00 UTC 2011
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
More information about the libvir-list
mailing list