Re: [libvirt] [PATCH 4/6] Added domainMigrateStartPostCopy in qemu driver

On Tue, Sep 23, 2014 at 16:09:59 +0200, Cristian Klein wrote: > Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; > --- > src/qemu/qemu_driver.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor.c | 19 +++++++++++++++++++ > src/qemu/qemu_monitor.h | 2 ++ > src/qemu/qemu_monitor_json.c | 18 ++++++++++++++++++ > src/qemu/qemu_monitor_json.h | 1 + > src/qemu/qemu_monitor_text.c | 11 +++++++++++ > src/qemu/qemu_monitor_text.h | 2 ++ No need to touch qemu_monitor_text.[ch] since we will only use QMP with any QEMU that supports post-copy migration. > 7 files changed, 97 insertions(+) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index e73d4f9..02c9a3b 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -11861,6 +11861,49 @@ qemuDomainGetJobStats(virDomainPtr dom, > } > > > +static int qemuMigrateStartPostCopy(virDomainPtr dom) > +{ > + virQEMUDriverPtr driver = dom->conn->privateData; > + virDomainObjPtr vm; > + int ret = -1; > + qemuDomainObjPrivatePtr priv; > + > + if (!(vm = qemuDomObjFromDomain(dom))) > + goto cleanup; > + > + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) > + goto cleanup; > + > + if (!virDomainObjIsActive(vm)) { > + virReportError(VIR_ERR_OPERATION_INVALID, > + "%s", _("domain is not running")); > + goto endjob; > + } > + > + priv = vm->privateData; > + > + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("post-copy can only be started " > + "while migration is in progress")); We forbid the use of tabs for indentation. Please, run make syntax-check (in addition to make check) before sending patches to catch this type of issues. > + goto cleanup; > + } > + > + VIR_DEBUG("Starting post-copy"); > + qemuDomainObjEnterMonitor(driver, vm); > + ret = qemuMonitorMigrateStartPostCopy(priv->mon); > + qemuDomainObjExitMonitor(driver, vm); > + > + endjob: > + if (!qemuDomainObjEndJob(driver, vm)) > + vm = NULL; > + > + cleanup: > + if (vm) > + virObjectUnlock(vm); > + return ret; > +} > + > static int qemuDomainAbortJob(virDomainPtr dom) > { > virQEMUDriverPtr driver = dom->conn->privateData; > @@ -18259,6 +18302,7 @@ static virDriver qemuDriver = { > .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */ > .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 1.2.7 */ > .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */ > + .domainMigrateStartPostCopy = qemuMigrateStartPostCopy, /* 1.2.9 */ This will need to be updated. > }; > > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 14688bf..0b230cc 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -2384,6 +2384,25 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, > return ret; > } > > +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon) > +{ > + int ret; > + VIR_DEBUG("mon=%p", mon); > + > + if (!mon) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("monitor must not be NULL")); > + return -1; > + } > + > + if (mon->json) > + ret = qemuMonitorJSONMigrateStartPostCopy(mon); > + else > + ret = qemuMonitorTextMigrateStartPostCopy(mon); Just return an error if !mon->json as other recent functions in qemu_monitor.c do. > + return ret; > +} > + > + > int qemuMonitorMigrateCancel(qemuMonitorPtr mon) > { > int ret; > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 587f779..97d980d 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -539,6 +539,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, > unsigned int flags, > const char *unixfile); > > +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon); > + > int qemuMonitorMigrateCancel(qemuMonitorPtr mon); > > int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index e98962b..14e7f84 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -2798,6 +2798,24 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, > return ret; > } > > +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon) > +{ > + int ret; > + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("migrate-start-postcopy", NULL); > + virJSONValuePtr reply = NULL; > + if (!cmd) > + return -1; I think the following would be better to avoid the long line: virJSONValuePtr cmd; cmd = qemuMonitorJSONMakeCommand("migrate-start-postcopy", NULL); if (!cmd) ... > + > + ret = qemuMonitorJSONCommand(mon, cmd, &reply); > + > + if (ret == 0) > + ret = qemuMonitorJSONCheckError(cmd, reply); > + > + virJSONValueFree(cmd); > + virJSONValueFree(reply); > + return ret; > +} > + > int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) > { > int ret; > diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h > index d366179..54beb7f 100644 > --- a/src/qemu/qemu_monitor_json.h > +++ b/src/qemu/qemu_monitor_json.h > @@ -151,6 +151,7 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, > bool *spice_migrated); > > > +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon); > int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); > > int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, ...

This mostly looks good in isolation but I think this is not going to work. When post-copy is started, QEMU on the destination host will be resumed (I'm not sure if that happens automatically or we have to do it), which basically means we need to jump out of the Perform state and call Finish and once it returns, we should keep waiting for the post-copy migration to finish in Confirm state and kill the domain at the end. It's certainly possible the steps we need to do are a bit different since I'm not familiar with all the details about post-copy migration, but I believe we need to do something. And just running a single QEMU command is not enough to start post-copy in libvirt.