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

Re: [libvirt] [PATCH 1/2] Fix multiple potential NULL pointer references in monitor usage



On Mon, May 17, 2010 at 08:41:08AM -0600, Eric Blake wrote:
> On 05/17/2010 05:53 AM, Daniel P. Berrange wrote:
> > @@ -5444,15 +5471,15 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> >      int i, rc;
> >      int ret = -1;
> >  
> > +    qemuDomainObjEnterMonitor(vm);
> > +
> >      /* We need different branches here, because we want to offline
> >       * in reverse order to onlining, so any partial fail leaves us in a
> >       * reasonably sensible state */
> >      if (nvcpus > vm->def->vcpus) {
> >          for (i = vm->def->vcpus ; i < nvcpus ; i++) {
> >              /* Online new CPU */
> > -            qemuDomainObjEnterMonitor(vm);
> >              rc = qemuMonitorSetCPU(priv->mon, i, 1);
> > -            qemuDomainObjExitMonitor(vm);
> >              if (rc == 0)
> >                  goto unsupported;
> >              if (rc < 0)
> > @@ -5463,9 +5490,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> >      } else {
> >          for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) {
> >              /* Offline old CPU */
> > -            qemuDomainObjEnterMonitor(vm);
> >              rc = qemuMonitorSetCPU(priv->mon, i, 0);
> > -            qemuDomainObjExitMonitor(vm);
> >              if (rc == 0)
> >                  goto unsupported;
> >              if (rc < 0)
> > @@ -5478,6 +5503,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> >      ret = 0;
> >  
> >  cleanup:
> > +    qemuDomainObjExitMonitor(vm);
> 
> This is a larger critical section now, but it didn't look like you were
> doing anything that had potentially long-running actions that would
> block such a large critical section.

Actually I've noticed a flaw in this block, not visible in the diff. My 
change means that we're not holding the mutex when doing vm->def->vcpus++;
I'll have to fix this the more verbose way...

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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