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

[Daniel P. Berrange]

On Fri, Aug 12, 2011 at 02:10:00PM +0200, Jiri Denemark wrote:
> 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.

What's wrong with 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

I didn't do that since then 'binpath' will generate an compile
warning about unused parameters on non-linux. The extra alloc
isn't really worth worrying about IMHO.

> > +
> > +    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.

Yep,


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|