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