On Fri, Mar 11, 2016 at 18:13:39 +0300, Dmitry Andreev wrote:

It wasn't possible to add XML not assigned to a domain because virDomainObjPtr was an argument type. --- src/qemu/qemu_migration.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 38fa81c..92d2ce9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -507,7 +507,7 @@ qemuMigrationCookieAddLockstate(qemuMigrationCookiePtr mig,

static int qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig, - virDomainObjPtr dom) + virDomainDefPtr def) { if (mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -515,10 +515,10 @@ qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig, return -1; }

- if (!dom->newDef) + if (!def) return 0;

- mig->persistent = dom->newDef; + mig->persistent = def; mig->flags |= QEMU_MIGRATION_COOKIE_PERSISTENT; mig->flagsMandatory |= QEMU_MIGRATION_COOKIE_PERSISTENT; return 0; @@ -1367,7 +1367,7 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, return -1;

if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT && - qemuMigrationCookieAddPersistent(mig, dom) < 0) + qemuMigrationCookieAddPersistent(mig, dom->newDef) < 0) return -1;

As mentioned in my reply to the second patch, I think this if statement could just be dropped completely, forcing the caller to use qemuMigrationCookieAddPersistent explicitly. Jirka

On Fri, Mar 11, 2016 at 18:13:40 +0300, Dmitry Andreev wrote:

Migration API allows to specify a destination domain configuration. Offline domain has only inactive XML and it is replaced by configuration specified using VIR_MIGRATE_PARAM_DEST_XML param. In case of live migration VIR_MIGRATE_PARAM_DEST_XML param is applied for active XML.

This commit introduces the new VIR_MIGRATE_PARAM_DEST_PERSIST_XML param that can be used within live migration to replace persistent/inactive configuration.

Required for: https://bugzilla.redhat.com/show_bug.cgi?id=835300 --- include/libvirt/libvirt-domain.h | 15 +++++++++++ src/qemu/qemu_driver.c | 10 +++++--- src/qemu/qemu_migration.c | 54 +++++++++++++++++++++++++++------------- src/qemu/qemu_migration.h | 2 ++ 4 files changed, 61 insertions(+), 20 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8ea3df6..2a589fd 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -723,6 +723,21 @@ typedef enum { # define VIR_MIGRATE_PARAM_DEST_XML "destination_xml"

/** + * VIR_MIGRATE_PARAM_DEST_PERSIST_XML: + * + * virDomainMigrate* params field: the new persistant configuration to be used + * for the domain on the destination host as VIR_TYPED_PARAM_STRING. + * This field cannot be used to rename the domain during migration (use + * VIR_MIGRATE_PARAM_DEST_NAME field for that purpose). Domain name in the + * destination XML must match the original domain name. + * + * Omitting this parameter keeps the original domain persistent configuration. + * Using this field with hypervisors that do not support changing domain + * configuration during migration will result in a failure. + */ +# define VIR_MIGRATE_PARAM_DEST_PERSIST_XML "destination_persistent_xml"

I think VIR_MIGRATE_PARAM_PERSIST_XML "persistent_xml" would be shorter and still clear enough.

+ +/** * VIR_MIGRATE_PARAM_BANDWIDTH: * * virDomainMigrate* params field: the maximum bandwidth (in MiB/s) that will diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..72bc17a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12066,7 +12066,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * * Consume any cookie we were able to decode though */ - ret = qemuMigrationPerform(driver, dom->conn, vm, + ret = qemuMigrationPerform(driver, dom->conn, vm, NULL, NULL, dconnuri, uri, NULL, NULL, 0, NULL, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ @@ -12450,7 +12450,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, return -1; }

- return qemuMigrationPerform(driver, dom->conn, vm, xmlin, + return qemuMigrationPerform(driver, dom->conn, vm, xmlin, NULL, dconnuri, uri, NULL, NULL, 0, NULL, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -12471,6 +12471,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; const char *dom_xml = NULL; + const char *persist_xml = NULL; const char *dname = NULL; const char *uri = NULL; const char *graphicsuri = NULL; @@ -12488,6 +12489,9 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, VIR_MIGRATE_PARAM_DEST_XML, &dom_xml) < 0 || virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_DEST_PERSIST_XML, + &persist_xml) < 0 || + virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 || virTypedParamsGetString(params, nparams, @@ -12519,7 +12523,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, goto cleanup; }

- ret = qemuMigrationPerform(driver, dom->conn, vm, dom_xml, + ret = qemuMigrationPerform(driver, dom->conn, vm, dom_xml, persist_xml, dconnuri, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, cookiein, cookieinlen, cookieout, cookieoutlen, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 92d2ce9..872b82f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4282,6 +4282,7 @@ qemuMigrationConnect(virQEMUDriverPtr driver, static int qemuMigrationRun(virQEMUDriverPtr driver, virDomainObjPtr vm, + const char *xmlin_persist,

Any reason for using different name? I'd stick with persist_xml.

const char *cookiein, int cookieinlen, char **cookieout, @@ -4296,6 +4297,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, { int ret = -1; unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; + virDomainDefPtr def = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; qemuMigrationCookiePtr mig = NULL; qemuMigrationIOThreadPtr iothread = NULL; @@ -4546,14 +4548,25 @@ qemuMigrationRun(virQEMUDriverPtr driver,

cookieFlags |= QEMU_MIGRATION_COOKIE_NETWORK | QEMU_MIGRATION_COOKIE_STATS; - if (flags & VIR_MIGRATE_PERSIST_DEST) - cookieFlags |= QEMU_MIGRATION_COOKIE_PERSISTENT; + + if (flags & VIR_MIGRATE_PERSIST_DEST) { + if (!xmlin_persist) { + cookieFlags |= QEMU_MIGRATION_COOKIE_PERSISTENT; + } else { + def = qemuMigrationPrepareDef(driver, xmlin_persist, NULL, NULL); + + if (!def || qemuMigrationCookieAddPersistent(mig, def) < 0) + ret = -1; + } + } +

I think it would be a bit nicer if qemuMigrationCookieAddPersistent was removed from qemuMigrationBakeCookie and the code above would just call it directly with either vm's def or the one supplied in params.

if (ret == 0 && qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, cookieFlags) < 0) { VIR_WARN("Unable to encode migration cookie"); }

+ virDomainDefFree(def); qemuMigrationCookieFree(mig);

if (events) @@ -4588,6 +4601,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, */ static int doNativeMigrate(virQEMUDriverPtr driver, virDomainObjPtr vm, + const char *xmlin_persist, const char *uri, const char *cookiein, int cookieinlen, @@ -4646,7 +4660,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver, spec.dest.host.port = uribits->port; spec.fwdType = MIGRATION_FWD_DIRECT;

- ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout, + ret = qemuMigrationRun(driver, vm, xmlin_persist, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks);

@@ -4663,6 +4677,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver, static int doTunnelMigrate(virQEMUDriverPtr driver, virDomainObjPtr vm, virStreamPtr st, + const char *xml_persist, const char *cookiein, int cookieinlen, char **cookieout, @@ -4707,7 +4722,7 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, goto cleanup; }

- ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout, + ret = qemuMigrationRun(driver, vm, xml_persist, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks);

@@ -4817,12 +4832,12 @@ static int doPeer2PeerMigrate2(virQEMUDriverPtr driver, VIR_DEBUG("Perform %p", sconn); qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2); if (flags & VIR_MIGRATE_TUNNELLED) - ret = doTunnelMigrate(driver, vm, st, + ret = doTunnelMigrate(driver, vm, st, NULL, NULL, 0, NULL, NULL, flags, resource, dconn, NULL, 0, NULL); else - ret = doNativeMigrate(driver, vm, uri_out, + ret = doNativeMigrate(driver, vm, NULL, uri_out, cookie, cookielen, NULL, NULL, /* No out cookie with v2 migration */ flags, resource, dconn, NULL, 0, NULL); @@ -4883,6 +4898,7 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver, const char *dconnuri, virDomainObjPtr vm, const char *xmlin, + const char *xmlin_persist, const char *dname, const char *uri, const char *graphicsuri, @@ -5047,13 +5063,13 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver, cookieout = NULL; cookieoutlen = 0; if (flags & VIR_MIGRATE_TUNNELLED) { - ret = doTunnelMigrate(driver, vm, st, + ret = doTunnelMigrate(driver, vm, st, xmlin_persist, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, nmigrate_disks, migrate_disks); } else { - ret = doNativeMigrate(driver, vm, uri, + ret = doNativeMigrate(driver, vm, xmlin_persist, uri, cookiein, cookieinlen, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, @@ -5226,6 +5242,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, virConnectPtr sconn, virDomainObjPtr vm, const char *xmlin, + const char *xmlin_persist, const char *dconnuri, const char *uri, const char *graphicsuri, @@ -5344,9 +5361,9 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,

if (*v3proto) { ret = doPeer2PeerMigrate3(driver, sconn, dconn, dconnuri, vm, xmlin, - dname, uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, resource, - useParams, flags); + xmlin_persist, dname, uri, graphicsuri, + listenAddress, nmigrate_disks, migrate_disks, + resource, useParams, flags); } else { ret = doPeer2PeerMigrate2(driver, sconn, dconn, vm, dconnuri, flags, dname, resource); @@ -5377,6 +5394,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, virConnectPtr conn, virDomainObjPtr vm, const char *xmlin, + const char *xmlin_persist, const char *dconnuri, const char *uri, const char *graphicsuri, @@ -5416,13 +5434,13 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, qemuMigrationStoreDomainState(vm);

if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { - ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, + ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, xmlin_persist, dconnuri, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, flags, dname, resource, &v3proto); } else { qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2); - ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, + ret = doNativeMigrate(driver, vm, xmlin_persist, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, NULL, 0, NULL); } @@ -5481,6 +5499,7 @@ static int qemuMigrationPerformPhase(virQEMUDriverPtr driver, virConnectPtr conn, virDomainObjPtr vm, + const char *xmlin_persist, const char *uri, const char *graphicsuri, size_t nmigrate_disks, @@ -5507,7 +5526,7 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver, virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationCleanup);

- ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, + ret = doNativeMigrate(driver, vm, xmlin_persist, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri, nmigrate_disks, migrate_disks); @@ -5546,6 +5565,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver, virConnectPtr conn, virDomainObjPtr vm, const char *xmlin, + const char *xmlin_persist, const char *dconnuri, const char *uri, const char *graphicsuri, @@ -5578,7 +5598,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver, return -1; }

- return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, uri, + return qemuMigrationPerformJob(driver, conn, vm, xmlin, xmlin_persist, dconnuri, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, cookiein, cookieinlen, @@ -5592,14 +5612,14 @@ qemuMigrationPerform(virQEMUDriverPtr driver, }

if (v3proto) { - return qemuMigrationPerformPhase(driver, conn, vm, uri, + return qemuMigrationPerformPhase(driver, conn, vm, xmlin_persist, uri, graphicsuri, nmigrate_disks, migrate_disks, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource); } else { - return qemuMigrationPerformJob(driver, conn, vm, xmlin, NULL, + return qemuMigrationPerformJob(driver, conn, vm, xmlin, xmlin_persist, NULL, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, cookiein, cookieinlen, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 2c67a02..ac16a8f 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -48,6 +48,7 @@ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \ VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_PERSIST_XML, VIR_TYPED_PARAM_STRING, \

What's wrong with adding the new parameter at the end of this list?

VIR_MIGRATE_PARAM_BANDWIDTH, VIR_TYPED_PARAM_ULLONG, \ VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_TYPED_PARAM_STRING, \ VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_TYPED_PARAM_STRING, \ @@ -140,6 +141,7 @@ int qemuMigrationPerform(virQEMUDriverPtr driver, virConnectPtr conn, virDomainObjPtr vm, const char *xmlin, + const char *xmlin_persist, const char *dconnuri, const char *uri, const char *graphicsuri,

Looks OK apart from a few nits. Jirka