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

Re: [libvirt] [PATCH 05/27] Add API for 'stop' monitor command



On Mon, Sep 28, 2009 at 02:22:49PM +0100, Mark McLoughlin wrote:
> On Thu, 2009-09-24 at 16:00 +0100, Daniel P. Berrange wrote:
> > * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Add a new
> >   qemuMonitorStopCPUs() API
> > * src/qemu/qemu_driver.c: Replace direct monitor commands for 'stop'
> >   with qemuMonitorStopCPUs()
> > ---
> >  src/qemu/qemu_driver.c       |   28 ++++------------------------
> >  src/qemu/qemu_monitor_text.c |   13 +++++++++++++
> >  src/qemu/qemu_monitor_text.h |    1 +
> >  3 files changed, 18 insertions(+), 24 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 1717cbd..5ebd4b7 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -2749,7 +2749,6 @@ cleanup:
> >  
> >  static int qemudDomainSuspend(virDomainPtr dom) {
> >      struct qemud_driver *driver = dom->conn->privateData;
> > -    char *info;
> >      virDomainObjPtr vm;
> >      int ret = -1;
> >      virDomainEventPtr event = NULL;
> > @@ -2770,17 +2769,12 @@ static int qemudDomainSuspend(virDomainPtr dom) {
> >          goto cleanup;
> >      }
> >      if (vm->state != VIR_DOMAIN_PAUSED) {
> > -        if (qemudMonitorCommand(vm, "stop", &info) < 0) {
> > -            qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> > -                             "%s", _("suspend operation failed"));
> > +        if (qemuMonitorStopCPUs(vm) < 0)
> >              goto cleanup;
> > -        }
> >          vm->state = VIR_DOMAIN_PAUSED;
> > -        qemudDebug("Reply %s", info);
> >          event = virDomainEventNewFromObj(vm,
> >                                           VIR_DOMAIN_EVENT_SUSPENDED,
> >                                           VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
> > -        VIR_FREE(info);
> >      }
> >      if (virDomainSaveStatus(dom->conn, driver->stateDir, vm) < 0)
> >          goto cleanup;
> > @@ -3365,13 +3359,9 @@ static int qemudDomainSave(virDomainPtr dom,
> >      /* Pause */
> >      if (vm->state == VIR_DOMAIN_RUNNING) {
> >          header.was_running = 1;
> > -        if (qemudMonitorCommand(vm, "stop", &info) < 0) {
> > -            qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> > -                             "%s", _("suspend operation failed"));
> > +        if (qemuMonitorStopCPUs(vm) < 0)
> >              goto cleanup;
> > -        }
> >          vm->state = VIR_DOMAIN_PAUSED;
> > -        qemudDebug("Reply %s", info);
> >          VIR_FREE(info);
> 
> Shouldn't be free-ing info here

Hmm, 'info' probbaly shouldn't even exist in this method anymore. I'll check
if its removed by a later patch in the series - probably only killed it once
I added the new methods for migration.

> ...
> > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> > index 3c12073..ec30e72 100644
> > --- a/src/qemu/qemu_monitor_text.c
> > +++ b/src/qemu/qemu_monitor_text.c
> > @@ -438,6 +438,19 @@ qemuMonitorStartCPUs(virConnectPtr conn,
> >  }
> >  
> > 
> > +int
> > +qemuMonitorStopCPUs(const virDomainObjPtr vm) {
> > +    char *info;
> > +
> > +    if (qemudMonitorCommand(vm, "stop", &info) < 0) {
> > +        qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> > +                         "%s", _("cannot stop CPU execution"));
> > +        return -1;
> > +    }
> > +    VIR_FREE(info);
> 
> Lost debugging of replies here, but you re-add it again in 26/27

The original code was quite inconsistent about logging the reply
data, so I didn't worry too much about copying that across perfectly
during the course of the series, since I planned a thorough santization
in the last patch of the series.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.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]