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

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.

> +    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.

> +        } > +        qemuDomainObjExitMonitorWithDriver(driver, vm); > +    } > + > + >     if (virDomainSaveStatus(conn, driver->stateDir, vm) < 0) >         goto abort; >