Re: [libvirt] [PATCH] nodeinfo: fix physical memory size on Mac OS X

Hi Doug, On Mon, Oct 21, 2013 at 10:51 AM, Doug Goldstein <cardoe(a)gentoo.org&gt; wrote: > > On Sun, Oct 20, 2013 at 10:14 AM, Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; wrote: > > HW_PHYSMEM is available on Mac OS X as well as FreeBSD, however, > > its resulting value for Mac OS X is 32 bits. Mac OS X provides > > HW_MEMSIZE that is 64 bits version of HW_PHYSMEM. We have to use it. > > > > I tested the patch on Mac OS X 10.6.8, 10.7.4, 10.8.5 and FreeBSD 9.2. > > > > Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; > > --- > > src/nodeinfo.c | 38 ++++++++++++++++++++++++++++---------- > > 1 file changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/src/nodeinfo.c b/src/nodeinfo.c > > index 70814c2..a0fdfe7 100644 > > --- a/src/nodeinfo.c > > +++ b/src/nodeinfo.c > > @@ -73,6 +73,33 @@ appleFreebsdNodeGetCPUCount(void) > > > > return ncpu; > > } > > + > > +/* VIR_HW_PHYSMEM - the resulting value of HW_PHYSMEM of FreeBSD > > + * is 64 bits while that of Mac OS X is still 32 bits. > > + * Mac OS X provides HW_MEMSIZE for 64 bits version of HW_PHYSMEM > > + * since 10.6.8 (Snow Leopard) at least. > > + */ > > +# ifdef HW_MEMSIZE > > +# define VIR_HW_PHYSMEM HW_MEMSIZE > > +# else > > +# define VIR_HW_PHYSMEM HW_PHYSMEM > > +# endif > > +static int > > +appleFreebsdNodeGetMemorySize(unsigned long *memory) > > +{ > > + int mib[2] = { CTL_HW, VIR_HW_PHYSMEM }; > > + unsigned long physmem; > > + size_t len = sizeof(physmem); > > + > > + if (sysctl(mib, 2, &physmem, &len, NULL, 0) == -1) { > > + virReportSystemError(errno, "%s", _("cannot obtain memory size")); > > + return -1; > > + } > > + > > + *memory = (unsigned long)(physmem / 1024); > > + > > + return 0; > > +} > > #endif > > > > #ifdef __linux__ > > @@ -914,17 +941,8 @@ cleanup: > > nodeinfo->mhz = cpu_freq / 1000000; > > # endif > > > > - /* get memory information */ > > - int mib[2] = { CTL_HW, HW_PHYSMEM }; > > - unsigned long physmem; > > - size_t len = sizeof(physmem); > > - > > - if (sysctl(mib, 2, &physmem, &len, NULL, 0) == -1) { > > - virReportSystemError(errno, "%s", _("cannot obtain memory size")); > > + if (appleFreebsdNodeGetMemorySize(&nodeinfo->memory) < 0) > > return -1; > > - } > > - > > - nodeinfo->memory = (unsigned long)(physmem / 1024); > > > > return 0; > > } > > -- > > 1.8.4 > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > > I'd say ACK on this but I'm not sure how the other devs feel about > breaking this out into another function with no clear benefit. The aim of the pull-out is to reduce the complexity of nodeGetInfo. I can separate the patch into two, tidy-up and fix-up, if preferable. Thanks, ozaki-r