Hi, Daniel
Thanks!
在 2012-09-07五的 12:59 +0100，Daniel P. Berrange写道：
> On Thu, Sep 06, 2012 at 01:24:45PM +0800, liguang wrote:
> > original migration did not aware of offline case,
> > so, add code to support offline migration quietly
> > (did not disturb original migration) by pass
> > VIR_MIGRATE_OFFLINE flag to migration APIs, and
> > migration process will not puzzeled by domain
> > offline and exit unexpectly.
>
> IIUC, if you pass VIR_MIGRATE_OFFLINE and the guest is not running,
> this API is more or less a no-op. Only if you add in the
> VIR_MIGRATE_PERSISTENT or VIR_MIGRATE_UNDEFINE_SOURCE flag will
> anything actually be done.
>
> > these changes did not take care of disk images the
> > domain required, for disk images could be transfered
> > by other APIs as suggested.
> > so, the migration result is just make domain
> > definition alive at target side.
>
> But only if VIR_MIGRATE_PERSISTENT is set, right ?
>
No, this action does it's work regardless of VIR_MIGRATE_PERSIST_DEST
flag
> > Signed-off-by: liguang <lig.fnst@xxxxxxxxxxxxxx>
> > ---
> > include/libvirt/libvirt.h.in | 1 +
> > src/qemu/qemu_driver.c | 8 +++++
> > src/qemu/qemu_migration.c | 63 ++++++++++++++++++++++++++++++++++++-----
> > src/qemu/qemu_migration.h | 3 +-
> > tools/virsh-domain.c | 4 ++
> > 5 files changed, 70 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index cfe5047..77df2ab 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -995,6 +995,7 @@ typedef enum {
> > * whole migration process; this will be used automatically
> > * when supported */
> > VIR_MIGRATE_UNSAFE = (1 << 9), /* force migration even if it is considered unsafe */
> > + VIR_MIGRATE_OFFLINE = (1 << 10), /* offline migrate */
> > } virDomainMigrateFlags;
> >
> > /* Domain migration. */
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index b12d9bc..0ed7053 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -9641,6 +9641,8 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
> > }
> >
> > if (!virDomainObjIsActive(vm)) {
> > + if (flags |= VIR_MIGRATE_OFFLINE)
> > + goto offline;
>
> This doesn't make sense - it should be '&' not '|='.
yes, absolutely, '&' is right.
>
> > virReportError(VIR_ERR_OPERATION_INVALID,
> > "%s", _("domain is not running"));
> > goto endjob;
> > @@ -9653,6 +9655,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
> > if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob) < 0)
> > goto endjob;
> >
> > +offline:
> > if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname,
> > cookieout, cookieoutlen,
> > flags)))
> > @@ -9888,6 +9891,11 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
> > goto cleanup;
> > }
> >
> > + if (flags & VIR_MIGRATE_OFFLINE) {
> > + ret = 0;
> > + goto cleanup;
> > + }
> > +
> > if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_OUT))
> > goto cleanup;
> >
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 1b21ef6..cf140d4 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags {
> > QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
> > QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
> > QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
> > + QEMU_MIGRATION_COOKIE_FLAG_OFFLINE,
> >
> > QEMU_MIGRATION_COOKIE_FLAG_LAST
> > };
> > @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags {
> > VIR_ENUM_DECL(qemuMigrationCookieFlag);
> > VIR_ENUM_IMPL(qemuMigrationCookieFlag,
> > QEMU_MIGRATION_COOKIE_FLAG_LAST,
> > - "graphics", "lockstate", "persistent");
> > + "graphics", "lockstate", "persistent", "offline");
> >
> > enum qemuMigrationCookieFeatures {
> > QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
> > QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
> > QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
> > + QEMU_MIGRATION_COOKIE_OFFLINE = (1 << QEMU_MIGRATION_COOKIE_FLAG_OFFLINE),
> > };
> >
> > typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
> > @@ -439,6 +441,12 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> > virBufferAdjustIndent(buf, -2);
> > }
> >
> > + if (mig->flags & QEMU_MIGRATION_COOKIE_OFFLINE) {
> > + virBufferAsprintf(buf, " <offline mig_ol='%d'>\n",
> > + 1);
> > + virBufferAddLit(buf, " </offline>\n");
> > + }
>
> The 'mig_ol' attribute appears to be entirely pointless.
>
> > +
> > virBufferAddLit(buf, "</qemu-migration>\n");
> > return 0;
> > }
> > @@ -662,6 +670,18 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> > VIR_FREE(nodes);
> > }
> >
> > + if ((flags & QEMU_MIGRATION_COOKIE_OFFLINE) &&
> > + virXPathBoolean("count(./offline) > 0", ctxt)) {
> > + int offline = 0;
> > + if (virXPathInt("string(./offline/@mig_ol)", ctxt, &offline) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("missing mig_ol attribute in migration data"));
> > + goto error;
> > + }
>
> This just re-inforces it. The existance of <offline/> is enough to show
> that this was offline.
yes, '<offline>' is enough, but '<offline>' pair is just an empty
element, so I prefer to give this attribute a value 'mig_ol=1', and I
think it's rational.
>
> > + if (offline != 1)
> > + mig->flags &= ~QEMU_MIGRATION_COOKIE_OFFLINE;
> > + }
> > +
> > return 0;
> >
> > error:
> > @@ -721,6 +741,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
> > qemuMigrationCookieAddPersistent(mig, dom) < 0)
> > return -1;
> >
> > + if (flags & QEMU_MIGRATION_COOKIE_OFFLINE) {
> > + mig->flags |= QEMU_MIGRATION_COOKIE_OFFLINE;
> > + }
> > +
> > if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
> > return -1;
> >
> > @@ -1151,6 +1175,13 @@ char *qemuMigrationBegin(struct qemud_driver *driver,
> > QEMU_MIGRATION_COOKIE_LOCKSTATE) < 0)
> > goto cleanup;
> >
> > + if (flags & VIR_MIGRATE_OFFLINE) {
> > + if (qemuMigrationBakeCookie(mig, driver, vm,
> > + cookieout, cookieoutlen,
> > + QEMU_MIGRATION_COOKIE_OFFLINE) < 0)
> > + goto cleanup;
> > + }
> > +
> > if (xmlin) {
> > if (!(def = virDomainDefParseString(driver->caps, xmlin,
> > QEMU_EXPECTED_VIRT_TYPES,
> > @@ -1314,6 +1345,15 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
> > goto endjob;
> > }
> >
> > + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
> > + QEMU_MIGRATION_COOKIE_OFFLINE)))
> > + return ret;
> > +
> > + if (mig->flags & QEMU_MIGRATION_COOKIE_OFFLINE) {
> > + ret = 0;
> > + goto cleanup;
> > + }
> > +
> > /* Start the QEMU daemon, with the same command-line arguments plus
> > * -incoming $migrateFrom
> > */
> > @@ -1856,7 +1896,8 @@ qemuMigrationRun(struct qemud_driver *driver,
> > virLockManagerPluginGetName(driver->lockManager));
> > return -1;
> > }
> > -
> > + if (flags & VIR_MIGRATE_OFFLINE)
> > + return 0;
> > if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
> > QEMU_MIGRATION_COOKIE_GRAPHICS)))
> > goto cleanup;
> > @@ -2372,6 +2413,8 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver,
> > qemuDomainObjExitRemoteWithDriver(driver, vm);
> > }
> > VIR_FREE(dom_xml);
> > + if (flags & VIR_MIGRATE_OFFLINE)
> > + goto cleanup;
> > if (ret == -1)
> > goto cleanup;
> >
> > @@ -2477,7 +2520,7 @@ finish:
> > vm->def->name);
> >
> > cleanup:
> > - if (ddomain) {
> > + if (ddomain || (flags & VIR_MIGRATE_OFFLINE)) {
> > virObjectUnref(ddomain);
> > ret = 0;
> > } else {
> > @@ -2554,10 +2597,9 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver,
> > }
> >
> > /* domain may have been stopped while we were talking to remote daemon */
> > - if (!virDomainObjIsActive(vm)) {
> > + if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE)) {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > _("guest unexpectedly quit"));
> > - goto cleanup;
> > }
>
> Huh, why are you removing this goto ?
yes, this 'goto' should come back, miss it.
And new patch to fix up flaws will come up soon.
>
>
> Daniel
--
liguang lig.fnst@xxxxxxxxxxxxxx
FNST linux kernel team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list