[libvirt] [PATCH] virprocess: make virProcessGetStartTime work on Mac OS X

[Ryota Ozaki]

Hi Doung,

On Wed, Oct 16, 2013 at 3:44 AM, Doug Goldstein <cardoe at gentoo.org> wrote:

> On Tue, Oct 15, 2013 at 12:05 PM, Ryota Ozaki <ozaki.ryota at gmail.com>
> wrote:
> > This patch addresses the following warning output by libvirtd:
> >   warning : virProcessGetStartTime:843 : Process start time of
> >   pid XXXXX not available on this platform
> >
> > For Mac OS X, we can use similar instructions to FreeBSD
> > to get the start time of a process. The difference between them
> > is struct kinfo_proc; kinfo_proc.ki_start is the start time
> > of a process for FreeBSD while kinfo_proc.kp_proc.p_starttime
> > for Mac OS X.
> >
> > Note that this patch works for Lion and Mountain Lion, however,
> > doesn't work for Snow Leopard for some reason;
> > sysctlnametomib("kern.proc.pid", ...) fails on Snow Leopard with
> > the following error:
> >   error : virProcessGetStartTime:822 : Unable to get MIB of
> >   kern.proc.pid: No such file or directory
> >
> > This is unexpected. man 3 sysctl of Snow Leopard says it
> > should work...
> >
> > Anyway libvirtd is able to launch on Snow Leopard regardless of
> > the error.
> >
> > Signed-off-by: Ryota Ozaki <ozaki.ryota at gmail.com>
> > ---
> >  src/util/virprocess.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > index 9fc3207..5a0ed0d 100644
> > --- a/src/util/virprocess.c
> > +++ b/src/util/virprocess.c
> > @@ -32,7 +32,7 @@
> >  #endif
> >  #include <sched.h>
> >
> > -#ifdef __FreeBSD__
> > +#if defined(__FreeBSD__) || defined(__APPLE__)
> >  # include <sys/param.h>
> >  # include <sys/sysctl.h>
> >  # include <sys/user.h>
> > @@ -809,7 +809,7 @@ cleanup:
> >      VIR_FREE(buf);
> >      return ret;
> >  }
> > -#elif defined(__FreeBSD__)
> > +#elif defined(__FreeBSD__) || defined(__APPLE__)
> >  int virProcessGetStartTime(pid_t pid,
> >                             unsigned long long *timestamp)
> >  {
> > @@ -817,7 +817,12 @@ int virProcessGetStartTime(pid_t pid,
> >      int mib[4];
> >      size_t len = 4;
> >
> > -    sysctlnametomib("kern.proc.pid", mib, &len);
> > +    /* FIXME: It doesn't work on Snow Leopard for some reason */
> > +    if (sysctlnametomib("kern.proc.pid", mib, &len) < 0) {
> > +        virReportSystemError(errno, "%s",
> > +                             _("Unable to get MIB of kern.proc.pid"));
> > +        return -1;
> > +    }
> >
> >      len = sizeof(struct kinfo_proc);
> >      mib[3] = pid;
> > @@ -828,7 +833,11 @@ int virProcessGetStartTime(pid_t pid,
> >          return -1;
> >      }
> >
> > +# if defined(__FreeBSD__)
> >      *timestamp = (unsigned long long)p.ki_start.tv_sec;
> > +# else
> > +    *timestamp = (unsigned long long)p.kp_proc.p_starttime.tv_sec;
> > +# endif
> >
> >      return 0;
> >
> > --
> > 1.8.4
>
> I'm hesitant to ACK this because of the failure you mention on Snow
> Leopard. The last time we merged something in that broke Snow Leopard
> we got push back from the Homebrew guys so I'd rather let this bake a
> bit more and figure out where the Snow Leopard issue is. I'm in the
> process of installing Snow Leopard so I should have a chance to look
> at it.


> Additionally, we'll want to test this on the FreeBSDs (finishing up
> installing a FreeBSD machine as well) since there is a behavior change
> for FreeBSD in this patch.
>

Actually I tested the patch on Snow Leopard and FreeBSD 9.2 as well.
(I forgot I have an old iMac with SL when testing the previous patches.)
As mentioned in the commit log, for SL the patch has a problem but
libvirtd can start anyway. For FreeBSD, libvirtd with the patch also starts
with some errors happened in other codes. There seems to be no
regression on the two OSs.

Anyway your tests are of course welcome!
  ozaki-r



>
> --
> Doug Goldstein
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131016/080f6c04/attachment-0001.htm>