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

On Thu, Dec 03, 2009 at 12:48:47AM +0100, Matthias Bolte wrote: > 2009/11/26 Daniel P. Berrange <berrange(a)redhat.com&gt;: > > 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; > >