[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]