[libvirt] [RFC PATCH 08/12] qemu: add support for memory devices

[Peter Krempa]

On Wed, Feb 04, 2015 at 17:28:00 -0500, John Ferlan wrote:
> 
> 
> On 01/30/2015 08:21 AM, Peter Krempa wrote:
> > Add support to start qemu instance with 'pc-dimm' device. Thanks to the
> > refactors we are able to reuse the existing function to determine the
> > parameters.
> > ---
> >  src/qemu/qemu_command.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_domain.c  |  18 ++++++--
> >  src/qemu/qemu_domain.h  |   2 +
> >  tests/qemuxml2xmltest.c |   1 +
> >  4 files changed, 132 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 5820fb5..7c31723 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c

...

> > +}
> > +
> > +
> >  char *
> >  qemuBuildNicStr(virDomainNetDefPtr net,
> >                  const char *prefix,
> > @@ -8351,6 +8446,25 @@ qemuBuildCommandLine(virConnectPtr conn,
> >          if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
> >              goto error;
> > 
> 
> Coverity has a FORWARD_NULL complaint... Right above this code there's:
> 
> 
> 8445 	    if (def->cpu && def->cpu->ncells)
> 8446 	        if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
> 8447 	            goto error;
> 8448 	
> 
> So there's a chance "def->cpu == NULL"
> 
> 
> > +    for (i = 0; i < def->nmems; i++) {
> > +        char *backStr;
> > +        char *dimmStr;
> > +
> > +        if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def,
> > +                                                      qemuCaps, cfg)))
> 
> Because def->cpu is NULL above, Coverity points out that
> qemuBuildMemoryDimmBackendStr will call qemuBuildMemoryBackendStr which
> deref's def->cpu->cells[guestNode].memAccess

Actually, def->cpu won't be NULL at this point as there is code that
makes sure that NUMA is enabled before even letting the VM to start with
memory devices so the warning should be a false positive due to a
complex structure.

There's a different problem though. The code is not range-checking that
the memory device's guest NUMA node (passed as 'guestNode' in the code
above) is actually in range of valid guest nodes.

I'm going to add a check that will solve both of the problems including
coverity's false positive.

Thanks for the catch :).

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150206/461caa4a/attachment-0001.sig>