Re: [libvirt] PATCH: 1/4: Add OOM hooks to the memory allocator

[Jim Meyering <jim meyering net>]

"Daniel P. Berrange" <berrange redhat com> wrote:
> This patch adds extra code to src/memory.c which allows us to force an OOM
> condition on specific allocations. This is not code you *ever* want to use
> in a production build, so its all conditional on TEST_OOM, which is enabled
> by passing  --enable-test-oom to the configure script.
>
> The hooks work as follows...
>
>  - The test suite first calls  virAllocTestInit() to initialize the hooks.
>    This causes it to start counting allocations.
>
>  ....then run the code you want to check for OOM...
>
>  - Next call virAllocTestCount() to find out how many allocations were
>    made.
>
>  - Given a number of allocations 'n', we need to repeat 'n' times...
>
>        - Call virAllocTestOOM(n, m) to tell it to fail the n'th allocation
>          upto the (n + m -1)'th allocation. eg,  virAllocTestOOM(3, 2)
>          will cause allocations 3 and 4 to fail.
>
>         ... run the code you want to check and verify it reports OOM
>         in the way you expect.
>
>
> It can be quite hard to find out just where allocation failure bugs are
> hiding. So there is also a virAllocTestHook() function which lets you
> register a callback to be invoked at the time an allocation is artificially
> failed. Obvious use for this is to capture a stack trace.

Cool!

> diff -r 9f962ac84b09 configure.in
...
> +if test "${enable_oom}" = yes; then
> +  have_trace=yes
> +  AC_CHECK_HEADER([execinfo.h],[],[have_trace=no])
> +  AC_CHECK_FUNC([backtrace],[],[have_trace=no])
> +  if test "$have_trace" = "yes"; then
> +    AC_DEFINE([HAVE_TRACE], 1, [Whether backtrace() is available])

Please use some prefix other than "HAVE_" here.
The HAVE_* namespace belongs to autoconf, and if some other
autoconf macro ever does AC_CHECK_FUNCS([trace]), that will
also define (or not) HAVE_TRACE.

For compound features like this, it is common to use the USE_ prefix.
So you might want to USE_BACKTRACE instead.

Otherwise, ACK.