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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list