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

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

 



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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]