Re: [libvirt] [PATCH 3/5] vz: implement managed migration

19 May 2016


      On 19.05.2016 13:38, Maxim Nestratov wrote:
...
18.05.2016 11:11, Nikolay Shirokovskiy пишет:
...
The newest version of migration protocol - version 3 with parameters is implemented.
Supported flags is VIR_MIGRATE_PAUSED only. Supported parameters are
VIR_MIGRATE_PARAM_URI and VIR_MIGRATE_PARAM_DEST_NAME. VIR_MIGRATE_PARAM_DEST_XML
is in VZ_MIGRATION_PARAMETERS for technical onyl reasons.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
  src/vz/vz_driver.c | 443 +++++++++++++++++++++++++++++++++++++++++++++++++++++
  src/vz/vz_sdk.c    |  42 +++++
  src/vz/vz_sdk.h    |   6 +
  3 files changed, 491 insertions(+)
...
...
...
+static int
+vzDomainMigratePrepare3Params(virConnectPtr conn,
+                              virTypedParameterPtr params,
+                              int nparams,
+                              const char *cookiein,
+                              int cookieinlen,
+                              char **cookieout,
+                              int *cookieoutlen,
+                              char **uri_out,
+                              unsigned int flags)
+{
+    vzConnPtr privconn = conn->privateData;
+    const char *miguri = NULL;
+    const char *dname = NULL;
+    virDomainObjPtr dom = NULL;
+    vzMigrationCookiePtr mig = NULL;
+    int ret = -1;
+
+    virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+    if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
+        goto cleanup;
+
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
+        virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0)
+        goto cleanup;
+
+    /* We must set uri_out if miguri is not set. This is direct
+     * managed migration requirement */
+    if (!miguri && !(*uri_out = vzMigrationCreateURI()))
+        goto cleanup;
+
+    if (!(mig = vzEatCookie(cookiein, cookieinlen)))
+        goto cleanup;
+
+    memcpy(mig->session_uuid, privconn->driver->session_uuid, VIR_UUID_BUFLEN);
+
+    if (vzBakeCookie(mig, cookieout, cookieoutlen) < 0)
+        goto cleanup;
+
+    if (!(dom = vzNewDomain(privconn->driver,
+                            dname ? dname : mig->name, mig->uuid)))
+        goto cleanup;
Just realized that since virDomainObjListAdd returns a valid and existing domain if its name and uuid is the same this call can cause some problems.
For instance, if a user specified the same source and destination uri, the migration will fail and Finish code will remove the domain from list.
Thus what about the following chunk instead existing?:
virObjectLock(privconn->driver);
    dom = virDomainObjListFindByUUID(privconn->driver->domains, mig->uuid);
    if (dom) {
        char uuidstr[VIR_UUID_STRING_BUFLEN];
        virUUIDFormat(mig->uuid, uuidstr);
        virReportError(VIR_ERR_INTERNAL_ERROR,
                       _("A domain with uuid '%s' already exists"),
                       uuidstr);
        goto unlock;
    }
if (!(dom = vzNewDomain(privconn->driver,
                            dname ? dname : mig->name, mig->uuid)))
        goto unlock;
ret = 0;
unlock:
    virObjectUnlock(privconn->driver);
Looks reasonable.
...
...
+
+    ret = 0;
+
+ cleanup:
+    vzMigrationCookieFree(mig);
+    if (dom)
+        virObjectUnlock(dom);
+    return ret;
+}
+