On Wed, Aug 10, 2011 at 16:37:27 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> In daemons using pidfiles to protect against concurrent
> execution there is a possibility that a crash may leave a stale
> pidfile on disk, which then prevents later restart of the daemon.
>
> To avoid this problem, introduce a pair of APIs which make
> use of virFileLock to ensure crash-safe & race condition-safe
> pidfile acquisition & releae
>
> * src/libvirt_private.syms, src/util/virpidfile.c,
> src/util/virpidfile.h: Add virPidFileAcquire and virPidFileRelease
> ---
> po/POTFILES.in | 1 +
> src/libvirt_private.syms | 4 +
> src/util/virpidfile.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virpidfile.h | 12 ++++
> 4 files changed, 159 insertions(+), 0 deletions(-)
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 7bb08b0..b200f89 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -123,6 +123,7 @@ src/util/sysinfo.c
> src/util/util.c
> src/util/viraudit.c
> src/util/virfile.c
> +src/util/virpidfile.c
> src/util/virterror.c
> src/util/xml.c
> src/vbox/vbox_MSCOMGlue.c
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fee3520..64b91ee 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1120,11 +1120,15 @@ virFileFdopen;
>
>
> # virpidfile.h
> +virPidFileAcquire;
> +virPidFileAcquirePath;
> virPidFileBuildPath;
> virPidFileRead;
> virPidFileReadIfAlive;
> virPidFileReadPath;
> virPidFileReadPathIfAlive;
> +virPidFileRelease;
> +virPidFileReleasePath;
> virPidFileWrite;
> virPidFileWritePath;
> virPidFileDelete;
> diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
> index dc92868..005c95d 100644
> --- a/src/util/virpidfile.c
> +++ b/src/util/virpidfile.c
> @@ -25,11 +25,15 @@
>
> #include <fcntl.h>
> #include <signal.h>
> +#include <sys/stat.h>
>
> #include "virpidfile.h"
> #include "virfile.h"
> #include "memory.h"
> #include "util.h"
> +#include "intprops.h"
> +#include "logging.h"
> +#include "virterror_internal.h"
>
>
> char *virPidFileBuildPath(const char *dir, const char* name)
> @@ -255,3 +259,141 @@ cleanup:
> VIR_FREE(pidfile);
> return rc;
> }
> +
> +
> +
> +int virPidFileAcquirePath(const char *path,
> + pid_t pid)
> +{
> + int fd = -1;
> + char ebuf[1024];
> + char pidstr[INT_BUFSIZE_BOUND(pid)];
> + verify(sizeof(pid_t) <= sizeof(unsigned int));
> +
> + if (path[0] == '\0')
> + return 0;
> +
> + while (1) {
> + struct stat a, b;
> + if ((fd = open(path, O_WRONLY|O_CREAT, 0644)) < 0) {
> + VIR_ERROR(_("Failed to open pid file '%s' : %s"),
> + path, virStrerror(errno, ebuf, sizeof ebuf));
Hmm, why not virReportSystemError(errno, ...) everywhere instead of VIR_ERROR
and virStrerror?
Also I think we should call virSetCloseExec on the file descriptor for a
little bit of extra safety.
...
ACK with the comments addressed.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list