[libvirt] PATCH: Add NUMA info to QEMU driver

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> This patch includes NUMA topology info in the QEMU driver capabilities
> XML output. It also implements the free memory driver APIs. This is done
> with the LGPL'd  numactl library. The configure script probes for it and
> only enables this functionality if it is found. The numactl library has
> been around for quite a while - RHEL-3 vintage at least

Looks fine, modulo a few nits...

...
> +        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
> +            if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)
...
> +        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
> +            if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)

The above bit-is-set query deserves a macro.

> Index: configure.in
> ===================================================================
> RCS file: /data/cvs/libvirt/configure.in,v
> retrieving revision 1.143
> diff -u -p -r1.143 configure.in
> --- configure.in	5 May 2008 19:58:56 -0000	1.143
> +++ configure.in	15 May 2008 21:08:05 -0000
> @@ -534,6 +534,40 @@ AM_CONDITIONAL(HAVE_SELINUX, [test "$wit
>  AC_SUBST(SELINUX_CFLAGS)
>  AC_SUBST(SELINUX_LIBS)
>
> +dnl NUMA lib
> +AC_ARG_WITH(numactl,
> +  [  --with-numactl         use numactl for host topology info],
> +  [],
> +  [with_numactl=check])
> +
> +NUMACTL_CFLAGS=
> +NUMACTL_LIBS=
> +if test "$with_qemu" = "yes" -a "$with_numactl" != "no"; then
> +  old_cflags="$CFLAGS"
> +  old_libs="$LIBS"
> +  if test "$with_numactl" = "check"; then
> +    AC_CHECK_HEADER([numa.h],[],[with_numactl=no])
> +    AC_CHECK_LIB(numa, numa_available,[],[with_numactl=no])
> +    if test "$with_numactl" != "no"; then
> +      with_numactl="yes"
> +    fi
> +  else
> +    AC_CHECK_HEADER([numa.h],[],
> +       [AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])])
> +    AC_CHECK_LIB(numa, numa_available,[],
> +       [AC_MSG_ERROR([You must install the numactl development package in order to compile and run libvirt])])

If the above diagnostics needn't be different,
you could do this:

   fail=0
   AC_CHECK_HEADER([numa.h], [], [fail=1])
   AC_CHECK_LIB([numa], [numa_available], [], [fail=1])
   test $fail = 1 &&
     AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])

> +  fi
> +  CFLAGS="$old_cflags"
> +  LIBS="$old_libs"
> +fi
> +if test "$with_numactl" = "yes"; then
> +  NUMACTL_LIBS="-lnuma"
> +  AC_DEFINE_UNQUOTED(HAVE_NUMACTL, 1, [whether Numactl is available for security])
> +fi
> +AM_CONDITIONAL(HAVE_NUMACTL, [test "$with_numactl" != "no"])
> +AC_SUBST(NUMACTL_CFLAGS)
> +AC_SUBST(NUMACTL_LIBS)

[I hesitate to mention this, since there is so much existing
 code here that does it the same way, but eventually this may well
 bite someone, so best to start doing it right. ]

No big deal, but these are underquoted, and technically will cause trouble
if there's ever an m4 macro with a conflicting name, so this syntax is
preferred:

      AC_DEFINE_UNQUOTED([HAVE_NUMACTL], 1, [whether Numactl is available for security])
    fi
    AM_CONDITIONAL([HAVE_NUMACTL], [test "$with_numactl" != "no"])
    AC_SUBST([NUMACTL_CFLAGS])
    AC_SUBST([NUMACTL_LIBS])

same with AC_CHECK_LIB and AC_ARG_WITH arguments:

    AC_ARG_WITH([numactl],
    AC_CHECK_LIB([numa], [numa_available],[],[with_numactl=no])