On Mon, 12 Mar 2012 15:49:27 +0200
Alon Levy <al...@redhat.com> wrote:
> On Mon, Mar 12, 2012 at 10:21:20AM -0300, Luiz Capitulino wrote:
> > On Sat, 10 Mar 2012 00:51:29 +0200
> > Alon Levy <al...@redhat.com> wrote:
> >
> > > On Fri, Mar 09, 2012 at 03:13:06PM -0300, Luiz Capitulino wrote:
> > > > The migrate command is one of those commands where HMP and QMP
> > > > completely
> > > > mix up together. This made the conversion to the QAPI (which separates
> > > > the
> > > > command into QMP and HMP parts) a bit difficult.
> > > >
> > > > The first important change to be noticed is that this commit completes
> > > > the
> > > > removal of the Monitor object from migration code, started by the
> > > > previous
> > > > commit.
> > > >
> > > > Another important and tricky change is about supporting the non-detached
> > > > mode. That's, if the user doesn't pass '-d' the migrate command will
> > > > lock
> > > > the monitor and will only release it when migration is finished.
> > > >
> > > > To support that in the new HMP command (hmp_migrate()), it is necessary
> > > > to create a timer which runs every second and checks if the migration is
> > > > still active. If it's, the timer callback will re-schedule itself to run
> > > > one second in the future. If the migration has already finished, the
> > > > monitor lock is relased and the user can use it normally.
> > > >
> > > > All these changes should be transparent to the user.
> > > >
> > > > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
> > > > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> > > > ---
> > > > hmp-commands.hx | 3 +--
> > > > hmp.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
> > > > hmp.h | 1 +
> > > > migration-fd.c | 2 +-
> > > > migration.c | 66
> > > > ++++++++++++++----------------------------------------
> > > > migration.h | 3 ---
> > > > qapi-schema.json | 21 +++++++++++++++++
> > > > qmp-commands.hx | 9 +-------
> > > > savevm.c | 13 +++++------
> > > > sysemu.h | 2 +-
> > > > 10 files changed, 105 insertions(+), 71 deletions(-)
> > > >
> > > > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > > > index ed88877..a86f53f 100644
> > > > --- a/hmp-commands.hx
> > > > +++ b/hmp-commands.hx
> > > > @@ -806,8 +806,7 @@ ETEXI
> > > > " full copy of disk\n\t\t\t -i for migration
> > > > without "
> > > > "shared storage with incremental copy of disk "
> > > > "(base image shared between src and destination)",
> > > > - .user_print = monitor_user_noop,
> > > > - .mhandler.cmd_new = do_migrate,
> > > > + .mhandler.cmd = hmp_migrate,
> > > > },
> > > >
> > > >
> > > > diff --git a/hmp.c b/hmp.c
> > > > index 3a54455..0f62b3f 100644
> > > > --- a/hmp.c
> > > > +++ b/hmp.c
> > > > @@ -14,6 +14,7 @@
> > > > */
> > > >
> > > > #include "hmp.h"
> > > > +#include "qemu-timer.h"
> > > > #include "qmp-commands.h"
> > > >
> > > > static void hmp_handle_error(Monitor *mon, Error **errp)
> > > > @@ -856,3 +857,58 @@ void hmp_block_job_cancel(Monitor *mon, const
> > > > QDict *qdict)
> > > >
> > > > hmp_handle_error(mon, &error);
> > > > }
> > > > +
> > > > +typedef struct MigrationStatus
> > > > +{
> > > > + QEMUTimer *timer;
> > > > + Monitor *mon;
> > > > +} MigrationStatus;
> > > > +
> > > > +static void hmp_migrate_status_cb(void *opaque)
> > > > +{
> > > > + MigrationStatus *status = opaque;
> > > > + MigrationInfo *info;
> > > > +
> > > > + info = qmp_query_migrate(NULL);
> > > > + if (!info->has_status || strcmp(info->status, "active") == 0) {
> > > > + qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock) +
> > > > 1000);
> > > > + } else {
> > > > + monitor_resume(status->mon);
> > > > + qemu_del_timer(status->timer);
> > > > + g_free(status);
> > > > + }
> > > > +
> > > > + qapi_free_MigrationInfo(info);
> > > > +}
> > > > +
> > > > +void hmp_migrate(Monitor *mon, const QDict *qdict)
> > > > +{
> > > > + int detach = qdict_get_try_bool(qdict, "detach", 0);
> > > > + int blk = qdict_get_try_bool(qdict, "blk", 0);
> > > > + int inc = qdict_get_try_bool(qdict, "inc", 0);
> > > > + const char *uri = qdict_get_str(qdict, "uri");
> > > > + Error *err = NULL;
> > > > +
> > > > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> > > > + if (err) {
> > > > + monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
> > > > + error_free(err);
> > > > + return;
> > > > + }
> > > > +
> > > > + if (!detach) {
> > > > + MigrationStatus *status;
> > > > +
> > > > + if (monitor_suspend(mon) < 0) {
> > >
> > > So using monitor_suspend is ok? is that a possible solution for the
> > > screendump issue? I could pass the Monitor through the vga callback. Tbh
> > > I think it's ugly, but at least it doesn't introduce any new command.
> >
> > This is only valid for HMP.
> >
>
> Yes, I'm thinking at this point for qmp to wait for the output file to
> be written via repeated poll / inotify.
Making this synchronous you mean? This will hold the global mutex for too
long, won't it?
