[libvirt] [PATCH v1 02/37] Introduce OOM reporting to virAsprintf

Peter Krempa pkrempa at redhat.com
Thu Jul 4 14:33:37 UTC 2013


On 07/04/13 14:06, Michal Privoznik wrote:
> Actually, I'm turning this function into a macro as filename,
> function name and line number needs to be passed. The new
> function virAsprintfInternal is introduced with the extended set
> of arguments.
> ---
>   cfg.mk                            |  2 +-
>   src/conf/domain_audit.c           | 36 ++++++++++-----------
>   src/driver.c                      |  4 +--
>   src/libvirt_private.syms          |  4 +--
>   src/util/virerror.c               |  2 +-
>   src/util/virstring.c              | 36 ++++++++++++---------
>   src/util/virstring.h              | 67 +++++++++++++++++++++++++++++++++++----
>   tests/domainsnapshotxml2xmltest.c |  2 ++
>   tests/fchosttest.c                |  2 ++
>   tests/interfacexml2xmltest.c      |  2 ++
>   tests/lxcxml2xmltest.c            |  2 ++
>   tests/networkxml2xmltest.c        |  2 ++
>   tests/nodedevxml2xmltest.c        |  2 ++
>   tests/nodeinfotest.c              |  2 ++
>   tests/nwfilterxml2xmltest.c       |  2 ++
>   tests/qemuargv2xmltest.c          |  2 ++
>   tests/qemuhelptest.c              |  2 ++
>   tests/qemuxml2xmltest.c           |  2 ++
>   tests/sexpr2xmltest.c             |  2 ++
>   tests/storagepoolxml2xmltest.c    |  2 ++
>   tests/storagevolxml2argvtest.c    |  2 ++
>   tests/storagevolxml2xmltest.c     |  2 ++
>   tests/sysinfotest.c               |  2 ++
>   tests/virbuftest.c                |  2 ++
>   tests/virhashtest.c               |  2 ++
>   tests/virshtest.c                 |  2 ++
>   tests/xencapstest.c               |  2 ++
>   tests/xml2sexprtest.c             |  2 ++
>   tools/virt-host-validate-common.c |  2 ++
>   29 files changed, 150 insertions(+), 45 deletions(-)
>

Again, there are a few places (at least in src/virlog.c) that will need 
fixing before this patch will be ready. The bulk rest of the patch looks 
okay but without fixing the logging code, we might run into nasty 
recursions on OOM.

As most of the code needs to be evaluated manually anyways, I'd change 
all calls to virAsprintf using a script to something like 
"virAsprintfUnhandled" with the original functionality along with 
introducing the new macros and then changing everything back according 
to the semantics of the code. The problem is that this approach would 
require lots of changes :(. Anyways, I'm okay with reviewing this again, 
but we need to be careful about those few places where reporting an 
error could hurt.

If anybody has some suggestions about other places than 
src/util/virerror.c and src/util/virlog.c that certainly need special 
care, please respond so we don't break stuff.

Peter




More information about the libvir-list mailing list