[PATCH v1 00/25] migration code cleanups + kbase document
Hi, My intention was to better understand how Libvirt migration works in the code level, aside from the documentation we already have in migration.html.in. During my reading I ended up doing some trivial cleanups use g_auto* functions here and there. Last patch is a new kbase document with a summary of what I've learned. Figured it might be a good complement for migration.html for developers. Daniel Henrique Barboza (25): libvirt-domain.c: modernize virDomainMigrateCheckNotLocal() libvirt-domain.c: modernize virDomainMigrateVersion1 libvirt-domain.c: use g_autofree in virDomainMigrateVersion* functions qemu_driver.c: modernize qemuDomainMigrateBegin3Params() qemu_driver.c: modernize qemuDomainMigratePrepare3() qemu_driver.c: modernize qemuDomainMigratePrepare3Params() qemu_migration.c: modernize qemuMigrationDstPrepare() qemu_migration.c: use g_auto* in qemuMigrationDstPrepareAny() qemu_migration_params.c: modernize qemuMigrationParamsFetch() qemu_migration_params.c: modernize qemuMigrationParamsEnableTLS() qemu_migration_params.c: use g_autofree in qemuMigrationParamsApply() qemu_monitor_json.c: modernize qemuMonitorJSONMigrateIncoming() qemu_migration.c: use g_auto* in qemuMigrationDstPrepareDirect() qemu_migration_cookie.h: register AUTOPTR_CLEANUP_FUNC for qemuMigrationCookiePtr qemu_migration.c: modernize qemuMigrationSrcBeginPhase() qemu_migration_cookie.c: modernize qemuMigrationEatCookie() qemu_driver.c: use g_auto* in qemuDomainMigratePerform3* functions qemu_migration.c: modernize qemuMigrationSrcPerformNative() qemu_migration.c: use auto* in qemuMigrationSrcRun() qemu_migration.c: use g_auto* in qemuMigrationSrcPerformJob and Peer2Peer qemu_migration.c: modernize qemuMigrationDstPersist() qemu_migration.c: use g_auto* in qemuMigrationDstFinish() libvirt-domain.c: g_autofree in virDomainMigrate() and virDomainMigrate2() qemu_migration.c: modernize qemuMigrationSrcConfirmPhase() docs/kbase: introduce migrationinternals.rst docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ src/libvirt-domain.c | 68 ++++-------- src/qemu/qemu_driver.c | 105 ++++++++---------- src/qemu/qemu_migration.c | 158 ++++++++++----------------- src/qemu/qemu_migration_cookie.c | 16 ++- src/qemu/qemu_migration_cookie.h | 1 + src/qemu/qemu_migration_params.c | 56 ++++------ src/qemu/qemu_monitor_json.c | 14 +-- 8 files changed, 323 insertions(+), 269 deletions(-) create mode 100644 docs/kbase/migrationinternals.rst -- 2.26.2
Use g_autoptr() and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt-domain.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9f65cba2e8..0941caa67f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3286,22 +3286,17 @@ virDomainMigrateVersion3Params(virDomainPtr domain, static int virDomainMigrateCheckNotLocal(const char *dconnuri) { - virURIPtr tempuri = NULL; - int ret = -1; + g_autoptr(virURI) tempuri = NULL; if (!(tempuri = virURIParse(dconnuri))) - goto cleanup; + return -1; if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) { virReportInvalidArg(dconnuri, "%s", _("Attempt to migrate guest to the same host")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virURIFree(tempuri); - return ret; + return 0; } -- 2.26.2
Use g_autofree on strings and remove the 'done' label since it's now unneeded. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt-domain.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0941caa67f..6adb58a9ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2726,9 +2726,8 @@ virDomainMigrateVersion1(virDomainPtr domain, const char *uri, unsigned long bandwidth) { - virDomainPtr ddomain = NULL; - char *uri_out = NULL; - char *cookie = NULL; + g_autofree char *uri_out = NULL; + g_autofree char *cookie = NULL; int cookielen = 0, ret; virDomainInfo info; unsigned int destflags; @@ -2758,12 +2757,12 @@ virDomainMigrateVersion1(virDomainPtr domain, if (dconn->driver->domainMigratePrepare (dconn, &cookie, &cookielen, uri, &uri_out, destflags, dname, bandwidth) == -1) - goto done; + return NULL; if (uri == NULL && uri_out == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare did not set uri")); - goto done; + return NULL; } if (uri_out) uri = uri_out; /* Did domainMigratePrepare change URI? */ @@ -2773,7 +2772,7 @@ virDomainMigrateVersion1(virDomainPtr domain, */ if (domain->conn->driver->domainMigratePerform (domain, cookie, cookielen, uri, flags, dname, bandwidth) == -1) - goto done; + return NULL; /* Get the destination domain and return it or error. * 'domain' no longer actually exists at this point @@ -2782,15 +2781,10 @@ virDomainMigrateVersion1(virDomainPtr domain, */ dname = dname ? dname : domain->name; if (dconn->driver->domainMigrateFinish) - ddomain = dconn->driver->domainMigrateFinish + return dconn->driver->domainMigrateFinish (dconn, dname, cookie, cookielen, uri, destflags); else - ddomain = virDomainLookupByName(dconn, dname); - - done: - VIR_FREE(uri_out); - VIR_FREE(cookie); - return ddomain; + return virDomainLookupByName(dconn, dname); } -- 2.26.2
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
Use g_autofree on strings and remove the 'done' label since it's now unneeded.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt-domain.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0941caa67f..6adb58a9ef 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c
@@ -2782,15 +2781,10 @@ virDomainMigrateVersion1(virDomainPtr domain, */ dname = dname ? dname : domain->name; if (dconn->driver->domainMigrateFinish) - ddomain = dconn->driver->domainMigrateFinish + return dconn->driver->domainMigrateFinish (dconn, dname, cookie, cookielen, uri, destflags); else - ddomain = virDomainLookupByName(dconn, dname);
Might as well remove this else statement and ...
- - done: - VIR_FREE(uri_out); - VIR_FREE(cookie); - return ddomain; + return virDomainLookupByName(dconn, dname);
.. do this return. Michal
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt-domain.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6adb58a9ef..e379623ce4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2816,9 +2816,9 @@ virDomainMigrateVersion2(virDomainPtr domain, unsigned long bandwidth) { virDomainPtr ddomain = NULL; - char *uri_out = NULL; - char *cookie = NULL; - char *dom_xml = NULL; + g_autofree char *uri_out = NULL; + g_autofree char *cookie = NULL; + g_autofree char *dom_xml = NULL; int cookielen = 0, ret; virDomainInfo info; virErrorPtr orig_err = NULL; @@ -2873,7 +2873,6 @@ virDomainMigrateVersion2(virDomainPtr domain, ret = dconn->driver->domainMigratePrepare2 (dconn, &cookie, &cookielen, uri, &uri_out, destflags, dname, bandwidth, dom_xml); - VIR_FREE(dom_xml); if (ret == -1) goto done; @@ -2918,8 +2917,6 @@ virDomainMigrateVersion2(virDomainPtr domain, done: virErrorRestore(&orig_err); - VIR_FREE(uri_out); - VIR_FREE(cookie); return ddomain; } @@ -2965,10 +2962,10 @@ virDomainMigrateVersion3Full(virDomainPtr domain, unsigned int flags) { virDomainPtr ddomain = NULL; - char *uri_out = NULL; - char *cookiein = NULL; - char *cookieout = NULL; - char *dom_xml = NULL; + g_autofree char *uri_out = NULL; + g_autofree char *cookiein = NULL; + g_autofree char *cookieout = NULL; + g_autofree char *dom_xml = NULL; int cookieinlen = 0; int cookieoutlen = 0; int ret; @@ -3242,10 +3239,6 @@ virDomainMigrateVersion3Full(virDomainPtr domain, done: virErrorRestore(&orig_err); - VIR_FREE(dom_xml); - VIR_FREE(uri_out); - VIR_FREE(cookiein); - VIR_FREE(cookieout); virTypedParamsFree(params, nparams); return ddomain; } -- 2.26.2
Use g_autofree and remove the unneeded 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ea4197d00..6b47e28e99 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12346,14 +12346,13 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; const char *dname = NULL; - const char **migrate_disks = NULL; + g_autofree const char **migrate_disks = NULL; int nmigrate_disks; - char *ret = NULL; virDomainObjPtr vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) - goto cleanup; + return NULL; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12361,30 +12360,26 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0) - goto cleanup; + return NULL; nmigrate_disks = virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, &migrate_disks); if (nmigrate_disks < 0) - goto cleanup; + return NULL; if (!(vm = qemuDomainObjFromDomain(domain))) - goto cleanup; + return NULL; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { virDomainObjEndAPI(&vm); - goto cleanup; + return NULL; } - ret = qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, - nmigrate_disks, migrate_disks, flags); - - cleanup: - VIR_FREE(migrate_disks); - return ret; + return qemuMigrationSrcBegin(domain->conn, vm, xmlin, dname, + cookieout, cookieoutlen, + nmigrate_disks, migrate_disks, flags); } -- 2.26.2
Use g_autoptr() on pointers and remove the unneeded 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b47e28e99..4fe122f8ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12397,10 +12397,9 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, const char *dom_xml) { virQEMUDriverPtr driver = dconn->privateData; - virDomainDefPtr def = NULL; + g_autoptr(virDomainDef) def = NULL; g_autofree char *origname = NULL; - qemuMigrationParamsPtr migParams = NULL; - int ret = -1; + g_autoptr(qemuMigrationParams) migParams = NULL; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -12411,30 +12410,25 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Tunnelled migration requested but invalid " "RPC method called")); - goto cleanup; + return -1; } if (!(migParams = qemuMigrationParamsFromFlags(NULL, 0, flags, QEMU_MIGRATION_DESTINATION))) - goto cleanup; + return -1; if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname))) - goto cleanup; + return -1; if (virDomainMigratePrepare3EnsureACL(dconn, def) < 0) - goto cleanup; - - ret = qemuMigrationDstPrepareDirect(driver, dconn, - cookiein, cookieinlen, - cookieout, cookieoutlen, - uri_in, uri_out, - &def, origname, NULL, 0, NULL, 0, - migParams, flags); + return -1; - cleanup: - qemuMigrationParamsFree(migParams); - virDomainDefFree(def); - return ret; + return qemuMigrationDstPrepareDirect(driver, dconn, + cookiein, cookieinlen, + cookieout, cookieoutlen, + uri_in, uri_out, + &def, origname, NULL, 0, NULL, 0, + migParams, flags); } static int -- 2.26.2
Use variable autocleanup and remove the now obsolete 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 43 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4fe122f8ae..d133793ee1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12444,21 +12444,20 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, { virQEMUDriverPtr driver = dconn->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virDomainDefPtr def = NULL; + g_autoptr(virDomainDef) def = NULL; const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; const char *listenAddress = cfg->migrationAddress; int nbdPort = 0; int nmigrate_disks; - const char **migrate_disks = NULL; + g_autofree const char **migrate_disks = NULL; g_autofree char *origname = NULL; - qemuMigrationParamsPtr migParams = NULL; - int ret = -1; + g_autoptr(qemuMigrationParams) migParams = NULL; - virCheckFlagsGoto(QEMU_MIGRATION_FLAGS, cleanup); + virCheckFlags(QEMU_MIGRATION_FLAGS, -1); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) - goto cleanup; + return -1; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12475,18 +12474,18 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, virTypedParamsGetInt(params, nparams, VIR_MIGRATE_PARAM_DISKS_PORT, &nbdPort) < 0) - goto cleanup; + return -1; nmigrate_disks = virTypedParamsGetStringList(params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, &migrate_disks); if (nmigrate_disks < 0) - goto cleanup; + return -1; if (!(migParams = qemuMigrationParamsFromFlags(params, nparams, flags, QEMU_MIGRATION_DESTINATION))) - goto cleanup; + return -1; if (flags & VIR_MIGRATE_TUNNELLED) { /* this is a logical error; we never should have gotten here with @@ -12495,28 +12494,22 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Tunnelled migration requested but invalid " "RPC method called")); - goto cleanup; + return -1; } if (!(def = qemuMigrationAnyPrepareDef(driver, NULL, dom_xml, dname, &origname))) - goto cleanup; + return -1; if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) - goto cleanup; - - ret = qemuMigrationDstPrepareDirect(driver, dconn, - cookiein, cookieinlen, - cookieout, cookieoutlen, - uri_in, uri_out, - &def, origname, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, - migParams, flags); + return -1; - cleanup: - qemuMigrationParamsFree(migParams); - VIR_FREE(migrate_disks); - virDomainDefFree(def); - return ret; + return qemuMigrationDstPrepareDirect(driver, dconn, + cookiein, cookieinlen, + cookieout, cookieoutlen, + uri_in, uri_out, + &def, origname, listenAddress, + nmigrate_disks, migrate_disks, nbdPort, + migParams, flags); } -- 2.26.2
Use g_autofree and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4ef3245c75..e0077d731b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2306,8 +2306,7 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, int fd) { qemuDomainObjPrivatePtr priv = vm->privateData; - qemuProcessIncomingDefPtr inc = NULL; - char *migrateFrom = NULL; + g_autofree char *migrateFrom = NULL; if (tunnel) { migrateFrom = g_strdup("stdio"); @@ -2329,7 +2328,7 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, if (!hostIPv6Capable) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("host isn't capable of IPv6")); - goto cleanup; + return NULL; } /* IPv6 address must be escaped in brackets on the cmd line */ encloseAddress = true; @@ -2358,12 +2357,8 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, migrateFrom = g_strdup_printf(incFormat, protocol, listenAddress, port); } - inc = qemuProcessIncomingDefNew(priv->qemuCaps, listenAddress, - migrateFrom, fd, NULL); - - cleanup: - VIR_FREE(migrateFrom); - return inc; + return qemuProcessIncomingDefNew(priv->qemuCaps, listenAddress, + migrateFrom, fd, NULL); } static int -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e0077d731b..83a25e7a8f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2389,7 +2389,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = NULL; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; - char *xmlout = NULL; + g_autofree char *xmlout = NULL; unsigned int cookieFlags; unsigned int startFlags; qemuProcessIncomingDefPtr incoming = NULL; @@ -2397,7 +2397,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, bool stopProcess = false; bool relabel = false; int rv; - char *tlsAlias = NULL; + g_autofree char *tlsAlias = NULL; virNWFilterReadLockFilterUpdates(); @@ -2447,7 +2447,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, /* Let migration hook filter domain XML */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { - char *xml; + g_autofree char *xml = NULL; int hookret; if (!(xml = qemuDomainDefFormatXML(driver, NULL, *def, @@ -2458,7 +2458,6 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, (*def)->name, VIR_HOOK_QEMU_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN, NULL, xml, &xmlout); - VIR_FREE(xml); if (hookret < 0) { goto cleanup; @@ -2467,7 +2466,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, VIR_DEBUG("Migrate hook filter returned nothing; using the" " original XML"); } else { - virDomainDefPtr newdef; + g_autoptr(virDomainDef) newdef = NULL; VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); newdef = virDomainDefParseString(xmlout, driver->xmlopt, NULL, @@ -2476,13 +2475,11 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, if (!newdef) goto cleanup; - if (!qemuDomainDefCheckABIStability(driver, NULL, *def, newdef)) { - virDomainDefFree(newdef); + if (!qemuDomainDefCheckABIStability(driver, NULL, *def, newdef)) goto cleanup; - } virDomainDefFree(*def); - *def = newdef; + *def = g_steal_pointer(&newdef); /* We should taint the domain here. However, @vm and therefore * privateData too are still NULL, so just notice the fact and * taint it later. */ @@ -2694,9 +2691,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, cleanup: virErrorPreserveLast(&origErr); - VIR_FREE(tlsAlias); qemuProcessIncomingDefFree(incoming); - VIR_FREE(xmlout); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); if (ret < 0 && priv) { -- 2.26.2
Use g_autoptr() and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration_params.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 6953badcfe..6273acd65a 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -1092,28 +1092,23 @@ qemuMigrationParamsFetch(virQEMUDriverPtr driver, qemuMigrationParamsPtr *migParams) { qemuDomainObjPrivatePtr priv = vm->privateData; - virJSONValuePtr jsonParams = NULL; - int ret = -1; + g_autoptr(virJSONValue) jsonParams = NULL; int rc; *migParams = NULL; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; + return -1; rc = qemuMonitorGetMigrationParams(priv->mon, &jsonParams); if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) - goto cleanup; + return -1; if (!(*migParams = qemuMigrationParamsFromJSON(jsonParams))) - goto cleanup; - - ret = 0; + return -1; - cleanup: - virJSONValueFree(jsonParams); - return ret; + return 0; } -- 2.26.2
Use g_autoptr() and remove both 'cleanup' and 'error' labels. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration_params.c | 34 +++++++++++--------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 6273acd65a..b1518f82c0 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -953,23 +953,22 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, qemuMigrationParamsPtr migParams) { qemuDomainObjPrivatePtr priv = vm->privateData; - virJSONValuePtr tlsProps = NULL; - virJSONValuePtr secProps = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virJSONValue) tlsProps = NULL; + g_autoptr(virJSONValue) secProps = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *secAlias = NULL; - int ret = -1; if (!cfg->migrateTLSx509certdir) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("host migration TLS directory not configured")); - goto error; + return -1; } if (!priv->job.migParams->params[QEMU_MIGRATION_PARAM_TLS_CREDS].set) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("TLS migration is not supported with this " "QEMU binary")); - goto error; + return -1; } /* If there's a secret, then grab/store it now using the connection */ @@ -977,18 +976,18 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, if (!(priv->migSecinfo = qemuDomainSecretInfoTLSNew(priv, QEMU_MIGRATION_TLS_ALIAS_BASE, cfg->migrateTLSx509secretUUID))) - goto error; + return -1; secAlias = priv->migSecinfo->s.aes.alias; } if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(QEMU_MIGRATION_TLS_ALIAS_BASE))) - goto error; + return -1; if (qemuDomainGetTLSObjects(priv->qemuCaps, priv->migSecinfo, cfg->migrateTLSx509certdir, tlsListen, cfg->migrateTLSx509verify, *tlsAlias, &tlsProps, &secProps) < 0) - goto error; + return -1; /* Ensure the domain doesn't already have the TLS objects defined... * This should prevent any issues just in case some cleanup wasn't @@ -997,29 +996,20 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, qemuDomainDelTLSObjects(driver, vm, asyncJob, secAlias, *tlsAlias); if (qemuDomainAddTLSObjects(driver, vm, asyncJob, &secProps, &tlsProps) < 0) - goto error; + return -1; if (qemuMigrationParamsSetString(migParams, QEMU_MIGRATION_PARAM_TLS_CREDS, *tlsAlias) < 0) - goto error; + return -1; if (!migParams->params[QEMU_MIGRATION_PARAM_TLS_HOSTNAME].set && qemuMigrationParamsSetString(migParams, QEMU_MIGRATION_PARAM_TLS_HOSTNAME, NULLSTR_EMPTY(hostname)) < 0) - goto error; - - ret = 0; - - cleanup: - virObjectUnref(cfg); - return ret; + return -1; - error: - virJSONValueFree(tlsProps); - virJSONValueFree(secProps); - goto cleanup; + return 0; } -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration_params.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index b1518f82c0..aea17b1ac8 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -836,8 +836,8 @@ qemuMigrationParamsApply(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; bool xbzrleCacheSize_old = false; - virJSONValuePtr params = NULL; - virJSONValuePtr caps = NULL; + g_autoptr(virJSONValue) params = NULL; + g_autoptr(virJSONValue) caps = NULL; qemuMigrationParam xbzrle = QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE; int ret = -1; int rc; @@ -896,9 +896,6 @@ qemuMigrationParamsApply(virQEMUDriverPtr driver, if (xbzrleCacheSize_old) migParams->params[xbzrle].set = true; - virJSONValueFree(params); - virJSONValueFree(caps); - return ret; } -- 2.26.2
Use g_autoptr() and remove the now obsolete 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_monitor_json.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3070c1e6b3..d808c4b55b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8405,9 +8405,8 @@ int qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon, const char *uri) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; if (!(cmd = qemuMonitorJSONMakeCommand("migrate-incoming", "s:uri", uri, @@ -8415,14 +8414,9 @@ qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; - - ret = qemuMonitorJSONCheckError(cmd, reply); + return -1; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return qemuMonitorJSONCheckError(cmd, reply); } -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 83a25e7a8f..28081c1b4f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2810,10 +2810,10 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, { unsigned short port = 0; bool autoPort = true; - char *hostname = NULL; + g_autofree char *hostname = NULL; int ret = -1; - virURIPtr uri = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virURI) uri = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *migrateHost = cfg->migrateHost; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " @@ -2928,9 +2928,6 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, nmigrate_disks, migrate_disks, nbdPort, migParams, flags); cleanup: - virURIFree(uri); - VIR_FREE(hostname); - virObjectUnref(cfg); if (ret != 0) { VIR_FREE(*uri_out); if (autoPort) -- 2.26.2
Next patch will use g_autoptr() in a qemuMigrationCookiePtr pointer to modernize qemuMigrationSrcBeginPhase(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration_cookie.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_migration_cookie.h b/src/qemu/qemu_migration_cookie.h index 1e88684589..95e7edb899 100644 --- a/src/qemu/qemu_migration_cookie.h +++ b/src/qemu/qemu_migration_cookie.h @@ -171,6 +171,7 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, void qemuMigrationCookieFree(qemuMigrationCookiePtr mig); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookie, qemuMigrationCookieFree); int qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig, -- 2.26.2
Use g_autoptr() and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 42 +++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 28081c1b4f..58c80533d8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2054,9 +2054,8 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, const char **migrate_disks, unsigned long flags) { - char *rv = NULL; - qemuMigrationCookiePtr mig = NULL; - virDomainDefPtr def = NULL; + g_autoptr(qemuMigrationCookie) mig = NULL; + g_autoptr(virDomainDef) def = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; unsigned int cookieFlags = QEMU_MIGRATION_COOKIE_LOCKSTATE; @@ -2075,12 +2074,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_BEGIN3); if (!qemuMigrationSrcIsAllowed(driver, vm, true, flags)) - goto cleanup; + return NULL; if (!(flags & (VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE)) && !qemuMigrationSrcIsSafe(vm->def, priv->qemuCaps, nmigrate_disks, migrate_disks, flags)) - goto cleanup; + return NULL; if (flags & VIR_MIGRATE_POSTCOPY && (!(flags & VIR_MIGRATE_LIVE) || @@ -2088,13 +2087,13 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("post-copy migration is not supported with non-live " "or paused migration")); - goto cleanup; + return NULL; } if (flags & VIR_MIGRATE_POSTCOPY && flags & VIR_MIGRATE_TUNNELLED) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("post-copy is not supported with tunnelled migration")); - goto cleanup; + return NULL; } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { @@ -2111,7 +2110,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, virReportError(VIR_ERR_INVALID_ARG, _("disk target %s not found"), migrate_disks[i]); - goto cleanup; + return NULL; } } @@ -2119,7 +2118,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Selecting disks to migrate is not " "implemented for tunnelled migration")); - goto cleanup; + return NULL; } } @@ -2152,13 +2151,13 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, if (!(mig = qemuMigrationEatCookie(driver, vm->def, priv->origname, priv, NULL, 0, 0))) - goto cleanup; + return NULL; if (qemuMigrationBakeCookie(mig, driver, vm, QEMU_MIGRATION_SOURCE, cookieout, cookieoutlen, cookieFlags) < 0) - goto cleanup; + return NULL; if (flags & VIR_MIGRATE_OFFLINE) { if (flags & (VIR_MIGRATE_NON_SHARED_DISK | @@ -2166,19 +2165,19 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("offline migration cannot handle " "non-shared storage")); - goto cleanup; + return NULL; } if (!(flags & VIR_MIGRATE_PERSIST_DEST)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("offline migration must be specified with " "the persistent flag set")); - goto cleanup; + return NULL; } if (flags & VIR_MIGRATE_TUNNELLED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("tunnelled offline migration does not " "make sense")); - goto cleanup; + return NULL; } } @@ -2186,21 +2185,16 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, if (!(def = virDomainDefParseString(xmlin, driver->xmlopt, priv->qemuCaps, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) - goto cleanup; + return NULL; if (!qemuDomainCheckABIStability(driver, vm, def)) - goto cleanup; + return NULL; - rv = qemuDomainDefFormatLive(driver, priv->qemuCaps, def, NULL, false, true); + return qemuDomainDefFormatLive(driver, priv->qemuCaps, def, NULL, false, true); } else { - rv = qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def, priv->origCPU, - false, true); + return qemuDomainDefFormatLive(driver, priv->qemuCaps, vm->def, priv->origCPU, + false, true); } - - cleanup: - qemuMigrationCookieFree(mig); - virDomainDefFree(def); - return rv; } char * -- 2.26.2
Use g_autoptr() and remove the obsolete 'error' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration_cookie.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 2e48d1b524..81b557e0a8 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1464,14 +1464,14 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, int cookieinlen, unsigned int flags) { - qemuMigrationCookiePtr mig = NULL; + g_autoptr(qemuMigrationCookie) mig = NULL; /* Parse & validate incoming cookie (if any) */ if (cookiein && cookieinlen && cookiein[cookieinlen-1] != '\0') { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Migration cookie was not NULL terminated")); - goto error; + return NULL; } VIR_DEBUG("cookielen=%d cookie='%s'", cookieinlen, NULLSTR(cookiein)); @@ -1485,7 +1485,7 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, priv ? priv->qemuCaps : NULL, cookiein, flags) < 0) - goto error; + return NULL; if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT && mig->persistent && @@ -1500,7 +1500,7 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing %s lock state for migration cookie"), virLockManagerPluginGetName(driver->lockManager)); - goto error; + return NULL; } } else if (STRNEQ(mig->lockDriver, virLockManagerPluginGetName(driver->lockManager))) { @@ -1508,16 +1508,12 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver, _("Source host lock driver %s different from target %s"), mig->lockDriver, virLockManagerPluginGetName(driver->lockManager)); - goto error; + return NULL; } } if (flags & QEMU_MIGRATION_COOKIE_STATS && mig->jobInfo) mig->jobInfo->operation = priv->job.current->operation; - return mig; - - error: - qemuMigrationCookieFree(mig); - return NULL; + return g_steal_pointer(&mig); } -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d133793ee1..d185666ed8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12634,7 +12634,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - qemuMigrationParamsPtr migParams = NULL; + g_autoptr(qemuMigrationParams) migParams = NULL; int ret = -1; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -12658,7 +12658,6 @@ qemuDomainMigratePerform3(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - qemuMigrationParamsFree(migParams); return ret; } @@ -12682,10 +12681,10 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, const char *graphicsuri = NULL; const char *listenAddress = NULL; int nmigrate_disks; - const char **migrate_disks = NULL; + g_autofree const char **migrate_disks = NULL; unsigned long long bandwidth = 0; int nbdPort = 0; - qemuMigrationParamsPtr migParams = NULL; + g_autoptr(qemuMigrationParams) migParams = NULL; int ret = -1; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -12743,8 +12742,6 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, flags, dname, bandwidth, true); cleanup: virDomainObjEndAPI(&vm); - qemuMigrationParamsFree(migParams); - VIR_FREE(migrate_disks); return ret; } -- 2.26.2
Use g_autoptr() and remove the unneeded 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 58c80533d8..884f1bc4a0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3844,7 +3844,7 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, qemuMigrationParamsPtr migParams) { qemuDomainObjPrivatePtr priv = vm->privateData; - virURIPtr uribits = NULL; + g_autoptr(virURI) uribits = NULL; int ret = -1; qemuMigrationSpec spec; @@ -3862,7 +3862,7 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, _("missing scheme in migration URI: %s"), uri); - goto cleanup; + return -1; } if (STREQ(uribits->scheme, "rdma")) { @@ -3870,13 +3870,13 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("outgoing RDMA migration is not supported " "with this QEMU binary")); - goto cleanup; + return -1; } if (!virMemoryLimitIsSet(vm->def->mem.hard_limit)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot start RDMA migration with no memory hard " "limit set")); - goto cleanup; + return -1; } } @@ -3900,9 +3900,6 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, if (spec.destType == MIGRATION_DEST_FD) VIR_FORCE_CLOSE(spec.dest.fd.qemu); - cleanup: - virURIFree(uribits); - return ret; } -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 884f1bc4a0..9fa08617a2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3467,10 +3467,10 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, int ret = -1; unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; qemuDomainObjPrivatePtr priv = vm->privateData; - qemuMigrationCookiePtr mig = NULL; - char *tlsAlias = NULL; + g_autoptr(qemuMigrationCookie) mig = NULL; + g_autofree char *tlsAlias = NULL; qemuMigrationIOThreadPtr iothread = NULL; - int fd = -1; + VIR_AUTOCLOSE fd = -1; unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; virErrorPtr orig_err = NULL; unsigned int cookieFlags = 0; @@ -3479,8 +3479,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, bool bwParam = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH); bool cancel = false; unsigned int waitFlags; - virDomainDefPtr persistDef = NULL; - char *timestamp; + g_autoptr(virDomainDef) persistDef = NULL; + g_autofree char *timestamp = NULL; int rc; VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " @@ -3642,10 +3642,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, } /* log start of migration */ - if ((timestamp = virTimeStringNow()) != NULL) { + if ((timestamp = virTimeStringNow()) != NULL) qemuDomainLogAppendMessage(driver, vm, "%s: initiating migration\n", timestamp); - VIR_FREE(timestamp); - } rc = -1; switch (spec->destType) { @@ -3779,11 +3777,6 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, ret = 0; cleanup: - VIR_FREE(tlsAlias); - VIR_FORCE_CLOSE(fd); - virDomainDefFree(persistDef); - qemuMigrationCookieFree(mig); - if (events) priv->signalIOError = false; -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 9fa08617a2..d8f2aeb9f0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4485,12 +4485,12 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, bool *v3proto) { int ret = -1; - virConnectPtr dconn = NULL; + g_autoptr(virConnect) dconn = NULL; bool p2p; virErrorPtr orig_err = NULL; bool offline = !!(flags & VIR_MIGRATE_OFFLINE); bool dstOffline = false; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); bool useParams; VIR_DEBUG("driver=%p, sconn=%p, vm=%p, xmlin=%s, dconnuri=%s, uri=%s, " @@ -4536,7 +4536,6 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to connect to remote libvirt URI %s: %s"), dconnuri, virGetLastErrorMessage()); - virObjectUnref(cfg); return -1; } @@ -4611,10 +4610,8 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, virErrorPreserveLast(&orig_err); qemuDomainObjEnterRemote(vm); virConnectUnregisterCloseCallback(dconn, qemuMigrationSrcConnectionClosed); - virObjectUnref(dconn); ignore_value(qemuDomainObjExitRemote(vm, false)); virErrorRestore(&orig_err); - virObjectUnref(cfg); return ret; } @@ -4650,7 +4647,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, virObjectEventPtr event = NULL; int ret = -1; virErrorPtr orig_err = NULL; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, @@ -4726,7 +4723,6 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, cleanup: virObjectEventStateQueue(driver->domainEventState, event); - virObjectUnref(cfg); return ret; } -- 2.26.2
Use g_autoptr() and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d8f2aeb9f0..c8bb0a6060 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4926,13 +4926,12 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver, qemuMigrationCookiePtr mig, bool ignoreSaveError) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr vmdef; - virDomainDefPtr oldDef = NULL; + g_autoptr(virDomainDef) oldDef = NULL; unsigned int oldPersist = vm->persistent; virObjectEventPtr event; - int ret = -1; vm->persistent = 1; oldDef = vm->newDef; @@ -4953,19 +4952,14 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver, VIR_DOMAIN_EVENT_DEFINED_ADDED); virObjectEventStateQueue(driver->domainEventState, event); - ret = 0; - - cleanup: - virDomainDefFree(oldDef); - virObjectUnref(cfg); - return ret; + return 0; error: virDomainDefFree(vm->newDef); vm->persistent = oldPersist; vm->newDef = oldDef; oldDef = NULL; - goto cleanup; + return -1; } -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c8bb0a6060..5d64e9df98 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4976,11 +4976,11 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, bool v3proto) { virDomainPtr dom = NULL; - qemuMigrationCookiePtr mig = NULL; + g_autoptr(qemuMigrationCookie) mig = NULL; virErrorPtr orig_err = NULL; int cookie_flags = 0; qemuDomainObjPrivatePtr priv = vm->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned short port; unsigned long long timeReceived = 0; virObjectEventPtr event; @@ -5246,9 +5246,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); VIR_FREE(priv->origname); virDomainObjEndAPI(&vm); - qemuMigrationCookieFree(mig); virErrorRestore(&orig_err); - virObjectUnref(cfg); /* Set a special error if Finish is expected to return NULL as a result of * successful call with retcode != 0 -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt-domain.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index e379623ce4..5b76f1ab66 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3553,7 +3553,7 @@ virDomainMigrate(virDomainPtr domain, if (flags & VIR_MIGRATE_PEER2PEER) { if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_P2P)) { - char *dstURI = NULL; + g_autofree char *dstURI = NULL; if (uri == NULL) { dstURI = virConnectGetURI(dconn); if (!dstURI) @@ -3562,11 +3562,8 @@ virDomainMigrate(virDomainPtr domain, VIR_DEBUG("Using peer2peer migration"); if (virDomainMigrateUnmanaged(domain, NULL, flags, dname, - uri ? uri : dstURI, NULL, bandwidth) < 0) { - VIR_FREE(dstURI); + uri ? uri : dstURI, NULL, bandwidth) < 0) goto error; - } - VIR_FREE(dstURI); ddomain = virDomainLookupByName(dconn, dname ? dname : domain->name); } else { @@ -3712,17 +3709,14 @@ virDomainMigrate2(virDomainPtr domain, if (flags & VIR_MIGRATE_PEER2PEER) { if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_P2P)) { - char *dstURI = virConnectGetURI(dconn); + g_autofree char *dstURI = virConnectGetURI(dconn); if (!dstURI) return NULL; VIR_DEBUG("Using peer2peer migration"); if (virDomainMigrateUnmanaged(domain, dxml, flags, dname, - dstURI, uri, bandwidth) < 0) { - VIR_FREE(dstURI); + dstURI, uri, bandwidth) < 0) goto error; - } - VIR_FREE(dstURI); ddomain = virDomainLookupByName(dconn, dname ? dname : domain->name); } else { -- 2.26.2
Use g_autoptr() and remove both 'error' and 'cleanup' labels. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_migration.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5d64e9df98..78e64344f6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2975,10 +2975,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, unsigned int flags, int retcode) { - qemuMigrationCookiePtr mig; + g_autoptr(qemuMigrationCookie) mig = NULL; virObjectEventPtr event; - int rv = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainJobInfoPtr jobInfo = NULL; @@ -2997,7 +2996,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, if (!(mig = qemuMigrationEatCookie(driver, vm->def, priv->origname, priv, cookiein, cookieinlen, QEMU_MIGRATION_COOKIE_STATS))) - goto cleanup; + return -1; if (retcode == 0) jobInfo = priv->job.completed; @@ -3026,7 +3025,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, } if (flags & VIR_MIGRATE_OFFLINE) - goto done; + return 0; /* Did the migration go as planned? If yes, kill off the domain object. * If something failed, resume CPUs, but only if we didn't use post-copy. @@ -3071,13 +3070,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, VIR_WARN("Failed to save status on vm %s", vm->def->name); } - done: - qemuMigrationCookieFree(mig); - rv = 0; - - cleanup: - virObjectUnref(cfg); - return rv; + return 0; } int -- 2.26.2
This document describes briefly how Libvirt migration internals works, complementing the info available in migration.html.in. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/kbase/migrationinternals.rst diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst new file mode 100644 index 0000000000..869ee99bd7 --- /dev/null +++ b/docs/kbase/migrationinternals.rst @@ -0,0 +1,174 @@ +=========================== +Libvirt migration internals +=========================== + +.. contents:: + +Migration is a multi-step operation with at least two distinct actors, +the source and the destination libvirtd daemons, and a lot of failure +points. This document describes the basic migration workflow in the +code level, as a way to complement `the base migration docs <migration.html>`_ +and help developers to get up to speed quicker with the code. + +In this document, unless stated otherwise, these conventions are followed: + +* 'user' refers to any entity that initiates a migration, regardless of being + an human using 'virsh' or a program consuming the Libvirt API; + +* 'source' refers to the source host of the migration, where the guest currently + exists; + +* 'destination' refers to the destination host of the migration. As of + Libvirt 6.5.0 local migration isn't supported, thus source and destination + refers to different hosts; + +* 'libvirt client' refers to the Libvirt client process that controls the + migration flow, e.g. virsh. Note that this client process can reside in + any host; + +* 'regular migration' refers to any migration operation where the libvirt + client co-ordinates the communication between the libvirtd instances in + the source and destination hosts. + +Migration protocol +================== + +Libvirt works with three migrations protocols. Preference is given to +protocol version 3, falling back to older versions if source and destination +can't handle version 3. Version 3 has been around since at least 2014, when +virDomainMigrate3 was moved to libvirt-domain.c by commit 67c08fccdcad, +meaning that it's safe to assume that users today are capable of always running +this protocol. + +Version 3 protocol sequence +--------------------------- + +The sequence of events in the migration protocol version 3, considering a +regular migration, is: + +1) in the source, generate the domain XML to pass to the destination. This +step is called "Begin"; + +2) in the destination, prepare the host to accept the incoming VM from the +source. This step is called "Prepare"; + +3) the source then starts the migration of the guest and waits for completion. +This is called "Perform"; + +4) destination waits for the migration to be completed, checking if it was successful +or not. The guest is killed in case of failure. This step is called "Finish"; + +5) the source checks the results of the migration process, killing the guest +if successful or resuming it if it failed. This is called "Confirm". + +In steps 1, 2, 3 and 4, an optional migration cookie can be generated and passed +to source or destination. This cookie contains extra information that informs +about extra settings or configuration required during the process. + +The name of each step and the version of the protocol is used to name the driver +interfaces that implements the logic. The steps above are implemented by the +following interfaces: + +1) Begin version 3: ``domainMigrateBegin3()`` and ``domainMigrateBegin3Params()`` +2) Prepare version 3: ``domainMigratePrepare3()`` and ``domainMigratePrepare3Params()`` +3) Perform version 3: ``domainMigratePerform3()`` and ``domainMigratePerform3Params()`` +4) Finish version 3: ``domainMigrateFinish3()`` and ``domainMigrateFinish3Params()`` +5) Confirm version 3: ``domainMigrateConfirm3()`` and ``domainMigrateConfirm3Params()`` + + +"virsh migrate" entry point +============================= + +When an user executes a "virsh migrate" command, virsh-domain.c calls ``cmdMigrate()``. +A virThread is created with the ``doMigrate`` worker. After validation of flags and +parameters, one of these functions will be executed: + +* if ``VIR_MIGRATE_PEER2PEER`` is set (i.e. --p2p was passed to virsh migrate), or + --direct was passed as parameter, ``virDomainMigrateToURI3()`` is called; + +* for all other cases, regular migration is assumed and execution goes + to ``virDomainMigrate3()``. + +virDomainMigrate3 function +-------------------------- + +``virDomainMigrate3()`` overall logic is: + +* if VIR_MIGRATE_PEER2PEER is set, error out and tell the user that this case must + be handled via ``virDomainMigrateToURI3()`` + +* if VIR_MIGRATE_OFFLINE is set, check if both source and destination supports it; + +* VIR_MIGRATE_CHANGE_PROTECTION is set, check if the source host supports it; + +* check if the source and the destination driver supports VIR_DRV_FEATURE_MIGRATION_PARAMS. + In this case, forward execution to ``virDomainMigrateVersion3Params()``; + +* proceed to check for a suitable migration protocol in both source and destination + drivers. The preference is to use migration protocol v3, via + ``virDomainMigrateVersion3()``, falling back to older versions if needed. + +Both ``virDomainMigrateVersion3()`` and ``virDomainMigrateVersion3Params()`` +are wrappers of ``virDomainMigrateVersion3Full()``, where the logic of the +regular migration is executed from step 1 (Begin) to 5 (Confirm). + +virDomainMigrateToURI3 function +------------------------------- + +While ``virDomainMigrate3()`` handles regular migration cases, ``virDomainMigrateToURI3()`` +takes care of peer-2-peer and direct migration scenarios. The function does flags +validation and then calls ``virDomainMigrateUnmanagedParams()``. At this point, +more checkings are made and then: + +* if VIR_MIGRATE_PEER2PEER is set and the source supports extensible parameters + (tested via VIR_DRV_FEATURE_MIGRATION_PARAMS support), ``domainMigratePerform3Params()`` + API of the hypervisor driver is called; + +* for all other cases, ``virDomainMigrateUnmanagedProto3()`` is called. This function does + additional checkings and then calls ``domainMigratePerform3()`` API of the hypervisor + driver. + +For both cases, the execution ends in the same API that handles the third step (Perform) +of the regular migration sequence. It's up for each hypervisor driver implementation to +differ when the API is being called from a regular or a peer-2-peer/direct migration. + +QEMU driver specifics +===================== + +The QEMU driver supports migration protocol version 2 and 3. Here's a list of +version 3 APIs that were discussed in this document that QEMU implements, +which can be found in src/qemu/qemu_driver.c: + +:: + + .domainMigrateBegin3 = qemuDomainMigrateBegin3, /* 0.9.2 */ + .domainMigratePrepare3 = qemuDomainMigratePrepare3, /* 0.9.2 */ + .domainMigratePerform3 = qemuDomainMigratePerform3, /* 0.9.2 */ + .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */ + .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */ + + .domainMigrateBegin3Params = qemuDomainMigrateBegin3Params, /* 1.1.0 */ + .domainMigratePrepare3Params = qemuDomainMigratePrepare3Params, /* 1.1.0 */ + .domainMigratePerform3Params = qemuDomainMigratePerform3Params, /* 1.1.0 */ + .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ + .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ + +All implementations have a 'Params' variation that handles the case where the +source and destationation can handle the extensible parameters API +(VIR_DRV_FEATURE_MIGRATION_PARAMS), but both versions calls out the same +inner function: + +* ``qemuDomainMigrateBegin3()`` and ``qemuDomainMigrateBegin3Params()`` use + ``qemuMigrationSrcBegin()``; + +* ``qemuDomainMigratePrepare3()`` and ``qemuDomainMigratePrepare3Params()`` use + ``qemuMigrationDstPrepareDirect()``; + +* ``qemuDomainMigratePerform3()`` and ``qemuDomainMigratePerform3Params()`` use + ``qemuMigrationSrcPerform()`` + +* ``qemuDomainMigrateFinish3()`` and ``qemuDomainMigrateFinish3Params()`` use + ``qemuMigrationDstFinish()`` + +* ``qemuDomainMigrateConfirm3()`` and ``qemuDomainMigrateConfirm3Params()`` use + ``qemuMigrationSrcConfirm()`` -- 2.26.2
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
This document describes briefly how Libvirt migration internals works, complementing the info available in migration.html.in.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/kbase/migrationinternals.rst
diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst new file mode 100644 index 0000000000..869ee99bd7 --- /dev/null +++ b/docs/kbase/migrationinternals.rst @@ -0,0 +1,174 @@ +=========================== +Libvirt migration internals +=========================== + +.. contents:: + +Migration is a multi-step operation with at least two distinct actors, +the source and the destination libvirtd daemons, and a lot of failure +points. This document describes the basic migration workflow in the +code level, as a way to complement `the base migration docs <migration.html>`_ +and help developers to get up to speed quicker with the code. + +In this document, unless stated otherwise, these conventions are followed: + +* 'user' refers to any entity that initiates a migration, regardless of being + an human using 'virsh' or a program consuming the Libvirt API; + +* 'source' refers to the source host of the migration, where the guest currently + exists; + +* 'destination' refers to the destination host of the migration. As of + Libvirt 6.5.0 local migration isn't supported, thus source and destination + refers to different hosts;
Is this right? What commit is reponsible for this change? Michal
On 7/13/20 10:20 AM, Michal Privoznik wrote:
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
This document describes briefly how Libvirt migration internals works, complementing the info available in migration.html.in.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/kbase/migrationinternals.rst
diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst new file mode 100644 index 0000000000..869ee99bd7 --- /dev/null +++ b/docs/kbase/migrationinternals.rst @@ -0,0 +1,174 @@ +=========================== +Libvirt migration internals +=========================== + +.. contents:: + +Migration is a multi-step operation with at least two distinct actors, +the source and the destination libvirtd daemons, and a lot of failure +points. This document describes the basic migration workflow in the +code level, as a way to complement `the base migration docs <migration.html>`_ +and help developers to get up to speed quicker with the code. + +In this document, unless stated otherwise, these conventions are followed: + +* 'user' refers to any entity that initiates a migration, regardless of being + an human using 'virsh' or a program consuming the Libvirt API; + +* 'source' refers to the source host of the migration, where the guest currently + exists; + +* 'destination' refers to the destination host of the migration. As of + Libvirt 6.5.0 local migration isn't supported, thus source and destination + refers to different hosts;
Is this right? What commit is reponsible for this change?
I guess my wording here is unclear. What I wanted to say is that, at least up to the current release we're at now (6.5.0), localhost migration (i.e. source and destination is the same host) isn't supported. I wanted to mention it this way because there's always the chance that Libvirt comes around and implements it. If you want a commit id, the error message warning about localhost migration appeared first here: commit 8654175c5b0c3db9e5f70907f102f0f900355d28 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Jan 24 18:06:16 2011 +0000 Introduce migration cookies to QEMU driver (...) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -46,0 +196,10 @@ + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing hostuuid element in migration data")); + goto error; + } + virUUIDFormat(mig->hostuuid, uuidstr); + if (STREQ(tmp, uuidstr)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Attempt to migrate guest to the same host %s"), + tmp); + goto error;
Michal
On 7/13/20 3:42 PM, Daniel Henrique Barboza wrote:
On 7/13/20 10:20 AM, Michal Privoznik wrote:
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
This document describes briefly how Libvirt migration internals works, complementing the info available in migration.html.in.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/kbase/migrationinternals.rst
diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst new file mode 100644 index 0000000000..869ee99bd7 --- /dev/null +++ b/docs/kbase/migrationinternals.rst @@ -0,0 +1,174 @@ +=========================== +Libvirt migration internals +=========================== + +.. contents:: + +Migration is a multi-step operation with at least two distinct actors, +the source and the destination libvirtd daemons, and a lot of failure +points. This document describes the basic migration workflow in the +code level, as a way to complement `the base migration docs <migration.html>`_ +and help developers to get up to speed quicker with the code. + +In this document, unless stated otherwise, these conventions are followed: + +* 'user' refers to any entity that initiates a migration, regardless of being + an human using 'virsh' or a program consuming the Libvirt API; + +* 'source' refers to the source host of the migration, where the guest currently + exists; + +* 'destination' refers to the destination host of the migration. As of + Libvirt 6.5.0 local migration isn't supported, thus source and destination + refers to different hosts;
Is this right? What commit is reponsible for this change?
I guess my wording here is unclear. What I wanted to say is that, at least up to the current release we're at now (6.5.0), localhost migration (i.e. source and destination is the same host) isn't supported. I wanted to mention it this way because there's always the chance that Libvirt comes around and implements it.
If you want a commit id, the error message warning about localhost migration appeared first here:
commit 8654175c5b0c3db9e5f70907f102f0f900355d28 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Jan 24 18:06:16 2011 +0000
Yeah, I was more interested whether we merged recently a patch that explicitly forbids same host migration. Speaking of which, to some extent we support same host migration (see v6.2.0-rc1~282) if both libvirts live in separate containers (and effectively think they run on different hosts). Can you post a diff that I can squash in before merging? Michal
On 7/13/20 11:32 AM, Michal Privoznik wrote:
On 7/13/20 3:42 PM, Daniel Henrique Barboza wrote:
On 7/13/20 10:20 AM, Michal Privoznik wrote:
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
This document describes briefly how Libvirt migration internals works, complementing the info available in migration.html.in.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/kbase/migrationinternals.rst
diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst new file mode 100644 index 0000000000..869ee99bd7 --- /dev/null +++ b/docs/kbase/migrationinternals.rst @@ -0,0 +1,174 @@ +=========================== +Libvirt migration internals +=========================== + +.. contents:: + +Migration is a multi-step operation with at least two distinct actors, +the source and the destination libvirtd daemons, and a lot of failure +points. This document describes the basic migration workflow in the +code level, as a way to complement `the base migration docs <migration.html>`_ +and help developers to get up to speed quicker with the code. + +In this document, unless stated otherwise, these conventions are followed: + +* 'user' refers to any entity that initiates a migration, regardless of being + an human using 'virsh' or a program consuming the Libvirt API; + +* 'source' refers to the source host of the migration, where the guest currently + exists; + +* 'destination' refers to the destination host of the migration. As of + Libvirt 6.5.0 local migration isn't supported, thus source and destination + refers to different hosts;
Is this right? What commit is reponsible for this change?
I guess my wording here is unclear. What I wanted to say is that, at least up to the current release we're at now (6.5.0), localhost migration (i.e. source and destination is the same host) isn't supported. I wanted to mention it this way because there's always the chance that Libvirt comes around and implements it.
If you want a commit id, the error message warning about localhost migration appeared first here:
commit 8654175c5b0c3db9e5f70907f102f0f900355d28 Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Jan 24 18:06:16 2011 +0000
Yeah, I was more interested whether we merged recently a patch that explicitly forbids same host migration. Speaking of which, to some extent we support same host migration (see v6.2.0-rc1~282) if both libvirts live in separate containers (and effectively think they run on different hosts).
Yeah, I believe k8s/kubevirt makes what I mentioned up there deprecated, since two nodes of the cluster are treated as complete different domains regardless of them being in the same host. This alone would make too much of a paragraph just to explain what I meant with 'destination'. Perhaps we should just stick it to the basics: $ git diff diff --git a/docs/kbase/migrationinternals.rst b/docs/kbase/migrationinternals.rst index 869ee99bd7..29bda0443b 100644 --- a/docs/kbase/migrationinternals.rst +++ b/docs/kbase/migrationinternals.rst @@ -18,9 +18,7 @@ In this document, unless stated otherwise, these conventions are followed: * 'source' refers to the source host of the migration, where the guest currently exists; -* 'destination' refers to the destination host of the migration. As of - Libvirt 6.5.0 local migration isn't supported, thus source and destination - refers to different hosts; +* 'destination' refers to the destination host of the migration; * 'libvirt client' refers to the Libvirt client process that controls the migration flow, e.g. virsh. Note that this client process can reside in Thanks, DHB
Can you post a diff that I can squash in before merging?
Michal
On 7/13/20 11:49 AM, Daniel Henrique Barboza wrote:
Hi,
My intention was to better understand how Libvirt migration works in the code level, aside from the documentation we already have in migration.html.in. During my reading I ended up doing some trivial cleanups use g_auto* functions here and there.
Last patch is a new kbase document with a summary of what I've learned. Figured it might be a good complement for migration.html for developers.
Daniel Henrique Barboza (25): libvirt-domain.c: modernize virDomainMigrateCheckNotLocal() libvirt-domain.c: modernize virDomainMigrateVersion1 libvirt-domain.c: use g_autofree in virDomainMigrateVersion* functions qemu_driver.c: modernize qemuDomainMigrateBegin3Params() qemu_driver.c: modernize qemuDomainMigratePrepare3() qemu_driver.c: modernize qemuDomainMigratePrepare3Params() qemu_migration.c: modernize qemuMigrationDstPrepare() qemu_migration.c: use g_auto* in qemuMigrationDstPrepareAny() qemu_migration_params.c: modernize qemuMigrationParamsFetch() qemu_migration_params.c: modernize qemuMigrationParamsEnableTLS() qemu_migration_params.c: use g_autofree in qemuMigrationParamsApply() qemu_monitor_json.c: modernize qemuMonitorJSONMigrateIncoming() qemu_migration.c: use g_auto* in qemuMigrationDstPrepareDirect() qemu_migration_cookie.h: register AUTOPTR_CLEANUP_FUNC for qemuMigrationCookiePtr qemu_migration.c: modernize qemuMigrationSrcBeginPhase() qemu_migration_cookie.c: modernize qemuMigrationEatCookie() qemu_driver.c: use g_auto* in qemuDomainMigratePerform3* functions qemu_migration.c: modernize qemuMigrationSrcPerformNative() qemu_migration.c: use auto* in qemuMigrationSrcRun() qemu_migration.c: use g_auto* in qemuMigrationSrcPerformJob and Peer2Peer qemu_migration.c: modernize qemuMigrationDstPersist() qemu_migration.c: use g_auto* in qemuMigrationDstFinish() libvirt-domain.c: g_autofree in virDomainMigrate() and virDomainMigrate2() qemu_migration.c: modernize qemuMigrationSrcConfirmPhase() docs/kbase: introduce migrationinternals.rst
docs/kbase/migrationinternals.rst | 174 ++++++++++++++++++++++++++++++ src/libvirt-domain.c | 68 ++++-------- src/qemu/qemu_driver.c | 105 ++++++++---------- src/qemu/qemu_migration.c | 158 ++++++++++----------------- src/qemu/qemu_migration_cookie.c | 16 ++- src/qemu/qemu_migration_cookie.h | 1 + src/qemu/qemu_migration_params.c | 56 ++++------ src/qemu/qemu_monitor_json.c | 14 +-- 8 files changed, 323 insertions(+), 269 deletions(-) create mode 100644 docs/kbase/migrationinternals.rst
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Daniel Henrique Barboza -
Michal Privoznik