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

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

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]