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

Re: [libvirt] [PATCH 07/14] Pull code to start CPUs executing out of qemudInitCpuAffinity()



On Thu, Dec 03, 2009 at 11:13:38AM +0000, Daniel P. Berrange wrote:
> On Thu, Dec 03, 2009 at 12:48:47AM +0100, Matthias Bolte wrote:
> > 2009/11/26 Daniel P. Berrange <berrange redhat com>:
> > > The code to start CPUs executing has nothing todo with CPU
> > > affinity masks, so pull it out of the qemudInitCpuAffinity()
> > > method and up into qemudStartVMDaemon()
> > >
> > > * src/qemu/qemu_driver.c: Pull code to start CPUs executing out
> > >  of qemudInitCpuAffinity()
> > > ---
> > >  src/qemu/qemu_driver.c |   37 +++++++++++++++++--------------------
> > >  1 files changed, 17 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > > index 2907976..b1f5894 100644
> > > --- a/src/qemu/qemu_driver.c
> > > +++ b/src/qemu/qemu_driver.c
> > > @@ -1370,13 +1370,10 @@ qemuDetectVcpuPIDs(virConnectPtr conn,
> > >  }
> > >
> > >  static int
> > > -qemudInitCpus(virConnectPtr conn,
> > > -              struct qemud_driver *driver,
> > > -              virDomainObjPtr vm,
> > > -              const char *migrateFrom) {
> > > +qemudInitCpuAffinity(virConnectPtr conn,
> > > +                     virDomainObjPtr vm) {
> > >     int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN;
> > >     virNodeInfo nodeinfo;
> > > -    qemuDomainObjPrivatePtr priv = vm->privateData;
> > >     unsigned char *cpumap;
> > >     int cpumaplen;
> > >
> > > @@ -1421,20 +1418,6 @@ qemudInitCpus(virConnectPtr conn,
> > >     }
> > >     VIR_FREE(cpumap);
> > >
> > > -    /* XXX This resume doesn't really belong here. Move it up to caller */
> > > -    if (migrateFrom == NULL) {
> > > -        /* Allow the CPUS to start executing */
> > > -        qemuDomainObjEnterMonitorWithDriver(driver, vm);
> > > -        if (qemuMonitorStartCPUs(priv->mon, conn) < 0) {
> > > -            if (virGetLastError() == NULL)
> > > -                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> > > -                                 "%s", _("resume operation failed"));
> > > -            qemuDomainObjExitMonitorWithDriver(driver, vm);
> > > -            return -1;
> > > -        }
> > > -        qemuDomainObjExitMonitorWithDriver(driver, vm);
> > > -    }
> > > -
> > >     return 0;
> > >  }
> > >
> > > @@ -2324,7 +2307,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> > >     if (qemuDetectVcpuPIDs(conn, driver, vm) < 0)
> > >         goto abort;
> > >
> > > -    if (qemudInitCpus(conn, driver, vm, migrateFrom) < 0)
> > > +    if (qemudInitCpuAffinity(conn, vm) < 0)
> > >         goto abort;
> > >
> > >     if (qemuInitPasswords(driver, vm) < 0)
> > > @@ -2337,6 +2320,20 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> > >     }
> > >     qemuDomainObjExitMonitorWithDriver(driver, vm);
> > 
> > Why do you exit the monitor here if you maybe just reenter it in the
> > following if-block? I would just merge the two enter/exit blocks and
> > put the final exit after the following if-block.
> 
> There's not really any particular reason for that - it just ended up
> that way after re-factoring, so we might as well merge it
> 
> > 
> > > +    if (migrateFrom == NULL) {
> > > +        /* Allow the CPUS to start executing */
> > > +        qemuDomainObjEnterMonitorWithDriver(driver, vm);
> > > +        if (qemuMonitorStartCPUs(priv->mon, conn) < 0) {
> > > +            if (virGetLastError() == NULL)
> > > +                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> > > +                                 "%s", _("resume operation failed"));
> > > +            qemuDomainObjExitMonitorWithDriver(driver, vm);
> > > +            return -1;
> > 
> > goto abort instead of return -1.
> 
> Ah yes :-)
>  
> > > +        }
> > > +        qemuDomainObjExitMonitorWithDriver(driver, vm);
> > > +    }
> > > +
> > > +
> > >     if (virDomainSaveStatus(conn, driver->stateDir, vm) < 0)
> > >         goto abort;
> > >

  ACK, makes sense once fixed :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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