All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] libxl: add p2p migration
@ 2015-11-10 15:32 Joao Martins
  2015-12-02 23:27 ` Jim Fehlig
       [not found] ` <565F7E5F.2080808@suse.com>
  0 siblings, 2 replies; 4+ messages in thread
From: Joao Martins @ 2015-11-10 15:32 UTC (permalink / raw)
  To: libvir-list; +Cc: jfehlig, Joao Martins, xen-devel

Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
for supporting migration in Openstack. Most of the changes
occur at the source and no modifications at the receiver.

In P2P mode there is only the Perform phase so we must handle
the connection with the destination and actually perform the
migration. libxlDomainPerformP2P implements the connection to
the destination and let libxlDoMigrateP2P implements the actual
migration logic with virConnectPtr. In this function we do
the migration steps in the destination similar to
virDomainMigrateVersion3Full. We appropriately save the last
error reported in each of the phases to provide proper
reporting. We don't yet support VIR_MIGRATE_TUNNELED and
we always use V3 with extensible params, making the
implementation simpler.

It is worth noting that the receiver didn't have any changes,
and because it's still the v3 sequence thus it is possible to
migrate from a P2P to non-P2P host.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
Changes since v1:
 - Move Begin step to libxlDoMigrateP2P to have all 4 steps
 together.
 - Remove if before VIR_FREE(dom_xml)
---
 src/libxl/libxl_driver.c    |  13 ++-
 src/libxl/libxl_migration.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
 src/libxl/libxl_migration.h |  11 +++
 3 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fcdcbdb..da98265 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
     switch (feature) {
     case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
     case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+    case VIR_DRV_FEATURE_MIGRATION_P2P:
         return 1;
     default:
         return 0;
@@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
     if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
-                                    uri, dname, flags) < 0)
-        goto cleanup;
+    if (flags & VIR_MIGRATE_PEER2PEER) {
+        if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
+                                           dconnuri, uri, dname, flags) < 0)
+            goto cleanup;
+    } else {
+        if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
+                                        uri, dname, flags) < 0)
+            goto cleanup;
+    }
 
     ret = 0;
 
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 0d23e5f..a1c7b55 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -42,6 +42,7 @@
 #include "libxl_conf.h"
 #include "libxl_migration.h"
 #include "locking/domain_lock.h"
+#include "virtypedparam.h"
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
@@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
     return ret;
 }
 
+/* This function is a simplification of virDomainMigrateVersion3Full
+ * excluding tunnel support and restricting it to migration v3
+ * with params since it was the first to be introduced in libxl.
+ */
+static int
+libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
+                  virDomainObjPtr vm,
+                  virConnectPtr sconn,
+                  const char *xmlin,
+                  virConnectPtr dconn,
+                  const char *dconnuri ATTRIBUTE_UNUSED,
+                  const char *dname,
+                  const char *uri,
+                  unsigned int flags)
+{
+    virDomainPtr ddomain = NULL;
+    virTypedParameterPtr params = NULL;
+    int nparams = 0;
+    int maxparams = 0;
+    char *uri_out = NULL;
+    char *dom_xml = NULL;
+    unsigned long destflags;
+    bool cancelled = true;
+    virErrorPtr orig_err = NULL;
+    int ret = -1;
+
+    dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
+    if (!dom_xml)
+        goto cleanup;
+
+    if (virTypedParamsAddString(&params, &nparams, &maxparams,
+                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
+        goto cleanup;
+
+    if (dname &&
+        virTypedParamsAddString(&params, &nparams, &maxparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
+        goto cleanup;
+
+    if (uri &&
+        virTypedParamsAddString(&params, &nparams, &maxparams,
+                                VIR_MIGRATE_PARAM_URI, uri) < 0)
+        goto cleanup;
+
+    /* We don't require the destination to have P2P support
+     * as it looks to be normal migration from the receiver perpective.
+     */
+    destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
+
+    VIR_DEBUG("Prepare3");
+    virObjectUnlock(vm);
+    ret = dconn->driver->domainMigratePrepare3Params
+        (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
+    virObjectLock(vm);
+
+    if (ret == -1)
+        goto cleanup;
+
+    if (uri_out) {
+        if (virTypedParamsReplaceString(&params, &nparams,
+                                        VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
+            orig_err = virSaveLastError();
+            goto finish;
+        }
+    } else {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("domainMigratePrepare3 did not set uri"));
+        goto finish;
+    }
+
+    VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out));
+    ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL,
+                                      uri_out, NULL, flags);
+
+    if (ret < 0)
+        orig_err = virSaveLastError();
+
+    cancelled = (ret < 0);
+
+ finish:
+    VIR_DEBUG("Finish3 ret=%d", ret);
+    if (virTypedParamsGetString(params, nparams,
+                                VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 &&
+        virTypedParamsReplaceString(&params, &nparams,
+                                    VIR_MIGRATE_PARAM_DEST_NAME,
+                                    vm->def->name) < 0) {
+        ddomain = NULL;
+    } else {
+        virObjectUnlock(vm);
+        ddomain = dconn->driver->domainMigrateFinish3Params
+            (dconn, params, nparams, NULL, 0, NULL, NULL,
+             destflags, cancelled);
+        virObjectLock(vm);
+    }
+
+    cancelled = (ddomain == NULL);
+
+    /* If Finish3Params set an error, and we don't have an earlier
+     * one we need to preserve it in case confirm3 overwrites
+     */
+    if (!orig_err)
+        orig_err = virSaveLastError();
+
+    VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
+    ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
+
+    if (ret < 0)
+        VIR_WARN("Guest %s probably left in 'paused' state on source",
+                 vm->def->name);
+
+ cleanup:
+    if (ddomain) {
+        virObjectUnref(ddomain);
+        ret = 0;
+    } else {
+        ret = -1;
+    }
+
+    if (orig_err) {
+        virSetError(orig_err);
+        virFreeError(orig_err);
+    }
+
+    VIR_FREE(dom_xml);
+    VIR_FREE(uri_out);
+    virTypedParamsFree(params, nparams);
+    return ret;
+}
+
+static int virConnectCredType[] = {
+    VIR_CRED_AUTHNAME,
+    VIR_CRED_PASSPHRASE,
+};
+
+static virConnectAuth virConnectAuthConfig = {
+    .credtype = virConnectCredType,
+    .ncredtype = ARRAY_CARDINALITY(virConnectCredType),
+};
+
+static void
+libxlMigrationConnectionClosed(virConnectPtr conn,
+                               int reason,
+                               void *opaque)
+{
+    virDomainObjPtr vm = opaque;
+
+    VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name);
+    virDomainObjBroadcast(vm);
+}
+
+/* On P2P mode there is only the Perform3 phase and we need to handle
+ * the connection with the destination libvirtd and perform the migration.
+ * Here we first tackle the first part of it, and libxlDoMigrationP2P handles
+ * the migration process with an established virConnectPtr to the destination.
+ */
+int
+libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
+                               virDomainObjPtr vm,
+                               virConnectPtr sconn,
+                               const char *xmlin,
+                               const char *dconnuri,
+                               const char *uri_str ATTRIBUTE_UNUSED,
+                               const char *dname,
+                               unsigned int flags)
+{
+    int ret = -1;
+    int keepAliveInterval = 5;
+    int keepAliveCount = 5;
+    bool useParams;
+    virConnectPtr dconn = NULL;
+    virErrorPtr orig_err = NULL;
+
+    virObjectUnlock(vm);
+    dconn = virConnectOpenAuth(dconnuri, &virConnectAuthConfig, 0);
+    virObjectLock(vm);
+
+    if (dconn == NULL) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("Failed to connect to remote libvirt URI %s: %s"),
+                       dconnuri, virGetLastErrorMessage());
+        return ret;
+    }
+
+    if (virConnectSetKeepAlive(dconn, keepAliveInterval,
+                               keepAliveCount) < 0)
+        goto cleanup;
+
+    if (virConnectRegisterCloseCallback(dconn, libxlMigrationConnectionClosed,
+                                        vm, NULL) < 0) {
+        goto cleanup;
+    }
+
+    virObjectUnlock(vm);
+    useParams = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
+                                         VIR_DRV_FEATURE_MIGRATION_PARAMS);
+    virObjectLock(vm);
+
+    if (!useParams) {
+        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                       _("Destination libvirt does not support migration with extensible parameters"));
+        goto cleanup;
+    }
+
+    ret = libxlDoMigrateP2P(driver, vm, sconn, xmlin, dconn, dconnuri,
+                            dname, uri_str, flags);
+
+ cleanup:
+    orig_err = virSaveLastError();
+    virObjectUnlock(vm);
+    virConnectUnregisterCloseCallback(dconn, libxlMigrationConnectionClosed);
+    virObjectUnref(dconn);
+    virObjectLock(vm);
+    if (orig_err) {
+        virSetError(orig_err);
+        virFreeError(orig_err);
+    }
+    return ret;
+}
+
 int
 libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
                             virDomainObjPtr vm,
diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h
index 20b45d8..0f83bb4 100644
--- a/src/libxl/libxl_migration.h
+++ b/src/libxl/libxl_migration.h
@@ -28,6 +28,7 @@
 
 # define LIBXL_MIGRATION_FLAGS                  \
     (VIR_MIGRATE_LIVE |                         \
+     VIR_MIGRATE_PEER2PEER |                    \
      VIR_MIGRATE_UNDEFINE_SOURCE |              \
      VIR_MIGRATE_PAUSED)
 
@@ -56,6 +57,16 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
                             unsigned int flags);
 
 int
+libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
+                               virDomainObjPtr vm,
+                               virConnectPtr sconn,
+                               const char *dom_xml,
+                               const char *dconnuri,
+                               const char *uri_str,
+                               const char *dname,
+                               unsigned int flags);
+
+int
 libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
                             virDomainObjPtr vm,
                             const char *dom_xml,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] libxl: add p2p migration
  2015-11-10 15:32 [PATCH v2] libxl: add p2p migration Joao Martins
@ 2015-12-02 23:27 ` Jim Fehlig
       [not found] ` <565F7E5F.2080808@suse.com>
  1 sibling, 0 replies; 4+ messages in thread
From: Jim Fehlig @ 2015-12-02 23:27 UTC (permalink / raw)
  To: Joao Martins, libvir-list; +Cc: xen-devel

On 11/10/2015 08:32 AM, Joao Martins wrote:
> Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
> for supporting migration in Openstack. Most of the changes
> occur at the source and no modifications at the receiver.
>
> In P2P mode there is only the Perform phase so we must handle
> the connection with the destination and actually perform the
> migration. libxlDomainPerformP2P implements the connection to
> the destination and let libxlDoMigrateP2P implements the actual
> migration logic with virConnectPtr. In this function we do
> the migration steps in the destination similar to
> virDomainMigrateVersion3Full. We appropriately save the last
> error reported in each of the phases to provide proper
> reporting. We don't yet support VIR_MIGRATE_TUNNELED and
> we always use V3 with extensible params, making the
> implementation simpler.
>
> It is worth noting that the receiver didn't have any changes,
> and because it's still the v3 sequence thus it is possible to
> migrate from a P2P to non-P2P host.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
> Changes since v1:
>  - Move Begin step to libxlDoMigrateP2P to have all 4 steps
>  together.
>  - Remove if before VIR_FREE(dom_xml)
> ---
>  src/libxl/libxl_driver.c    |  13 ++-
>  src/libxl/libxl_migration.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
>  src/libxl/libxl_migration.h |  11 +++
>  3 files changed, 241 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index fcdcbdb..da98265 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
>      switch (feature) {
>      case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>      case VIR_DRV_FEATURE_MIGRATION_PARAMS:
> +    case VIR_DRV_FEATURE_MIGRATION_P2P:
>          return 1;
>      default:
>          return 0;
> @@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>      if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
>  
> -    if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
> -                                    uri, dname, flags) < 0)
> -        goto cleanup;
> +    if (flags & VIR_MIGRATE_PEER2PEER) {
> +        if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
> +                                           dconnuri, uri, dname, flags) < 0)
> +            goto cleanup;
> +    } else {
> +        if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
> +                                        uri, dname, flags) < 0)
> +            goto cleanup;
> +    }
>  
>      ret = 0;
>  
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index 0d23e5f..a1c7b55 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -42,6 +42,7 @@
>  #include "libxl_conf.h"
>  #include "libxl_migration.h"
>  #include "locking/domain_lock.h"
> +#include "virtypedparam.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>  
> @@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>      return ret;
>  }
>  
> +/* This function is a simplification of virDomainMigrateVersion3Full
> + * excluding tunnel support and restricting it to migration v3
> + * with params since it was the first to be introduced in libxl.
> + */
> +static int
> +libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
> +                  virDomainObjPtr vm,
> +                  virConnectPtr sconn,
> +                  const char *xmlin,
> +                  virConnectPtr dconn,
> +                  const char *dconnuri ATTRIBUTE_UNUSED,
> +                  const char *dname,
> +                  const char *uri,
> +                  unsigned int flags)
> +{
> +    virDomainPtr ddomain = NULL;
> +    virTypedParameterPtr params = NULL;
> +    int nparams = 0;
> +    int maxparams = 0;
> +    char *uri_out = NULL;
> +    char *dom_xml = NULL;
> +    unsigned long destflags;
> +    bool cancelled = true;
> +    virErrorPtr orig_err = NULL;
> +    int ret = -1;
> +
> +    dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
> +    if (!dom_xml)
> +        goto cleanup;
> +
> +    if (virTypedParamsAddString(&params, &nparams, &maxparams,
> +                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
> +        goto cleanup;
> +
> +    if (dname &&
> +        virTypedParamsAddString(&params, &nparams, &maxparams,
> +                                VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
> +        goto cleanup;
> +
> +    if (uri &&
> +        virTypedParamsAddString(&params, &nparams, &maxparams,
> +                                VIR_MIGRATE_PARAM_URI, uri) < 0)
> +        goto cleanup;
> +
> +    /* We don't require the destination to have P2P support
> +     * as it looks to be normal migration from the receiver perpective.
> +     */
> +    destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
> +
> +    VIR_DEBUG("Prepare3");
> +    virObjectUnlock(vm);
> +    ret = dconn->driver->domainMigratePrepare3Params
> +        (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
> +    virObjectLock(vm);

Is it necessary to unlock and lock the vm while calling prepare and finish on
the destination libvirtd?

> +
> +    if (ret == -1)
> +        goto cleanup;
> +
> +    if (uri_out) {
> +        if (virTypedParamsReplaceString(&params, &nparams,
> +                                        VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
> +            orig_err = virSaveLastError();
> +            goto finish;
> +        }
> +    } else {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("domainMigratePrepare3 did not set uri"));
> +        goto finish;
> +    }
> +
> +    VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out));
> +    ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL,
> +                                      uri_out, NULL, flags);
> +
> +    if (ret < 0)
> +        orig_err = virSaveLastError();
> +
> +    cancelled = (ret < 0);
> +
> + finish:
> +    VIR_DEBUG("Finish3 ret=%d", ret);
> +    if (virTypedParamsGetString(params, nparams,
> +                                VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 &&
> +        virTypedParamsReplaceString(&params, &nparams,
> +                                    VIR_MIGRATE_PARAM_DEST_NAME,
> +                                    vm->def->name) < 0) {
> +        ddomain = NULL;
> +    } else {
> +        virObjectUnlock(vm);
> +        ddomain = dconn->driver->domainMigrateFinish3Params
> +            (dconn, params, nparams, NULL, 0, NULL, NULL,
> +             destflags, cancelled);
> +        virObjectLock(vm);
> +    }
> +
> +    cancelled = (ddomain == NULL);
> +
> +    /* If Finish3Params set an error, and we don't have an earlier
> +     * one we need to preserve it in case confirm3 overwrites
> +     */
> +    if (!orig_err)
> +        orig_err = virSaveLastError();
> +
> +    VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
> +    ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
> +
> +    if (ret < 0)
> +        VIR_WARN("Guest %s probably left in 'paused' state on source",
> +                 vm->def->name);
> +
> + cleanup:
> +    if (ddomain) {
> +        virObjectUnref(ddomain);
> +        ret = 0;
> +    } else {
> +        ret = -1;
> +    }
> +
> +    if (orig_err) {
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
> +    }
> +
> +    VIR_FREE(dom_xml);
> +    VIR_FREE(uri_out);
> +    virTypedParamsFree(params, nparams);
> +    return ret;
> +}
> +
> +static int virConnectCredType[] = {
> +    VIR_CRED_AUTHNAME,
> +    VIR_CRED_PASSPHRASE,
> +};
> +
> +static virConnectAuth virConnectAuthConfig = {
> +    .credtype = virConnectCredType,
> +    .ncredtype = ARRAY_CARDINALITY(virConnectCredType),
> +};
> +
> +static void
> +libxlMigrationConnectionClosed(virConnectPtr conn,
> +                               int reason,
> +                               void *opaque)
> +{
> +    virDomainObjPtr vm = opaque;
> +
> +    VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name);
> +    virDomainObjBroadcast(vm);

I would expect to see a corresponding virDomainObjWait for this call to
virDomainObjBroadcast. But with the current migration code, I don't think the
connect close callback is needed. Is there anything to do when the connection to
destination libvirtd closes?

Regards,
Jim

> +}
> +
> +/* On P2P mode there is only the Perform3 phase and we need to handle
> + * the connection with the destination libvirtd and perform the migration.
> + * Here we first tackle the first part of it, and libxlDoMigrationP2P handles
> + * the migration process with an established virConnectPtr to the destination.
> + */
> +int
> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
> +                               virDomainObjPtr vm,
> +                               virConnectPtr sconn,
> +                               const char *xmlin,
> +                               const char *dconnuri,
> +                               const char *uri_str ATTRIBUTE_UNUSED,
> +                               const char *dname,
> +                               unsigned int flags)
> +{
> +    int ret = -1;
> +    int keepAliveInterval = 5;
> +    int keepAliveCount = 5;
> +    bool useParams;
> +    virConnectPtr dconn = NULL;
> +    virErrorPtr orig_err = NULL;
> +
> +    virObjectUnlock(vm);
> +    dconn = virConnectOpenAuth(dconnuri, &virConnectAuthConfig, 0);
> +    virObjectLock(vm);
> +
> +    if (dconn == NULL) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("Failed to connect to remote libvirt URI %s: %s"),
> +                       dconnuri, virGetLastErrorMessage());
> +        return ret;
> +    }
> +
> +    if (virConnectSetKeepAlive(dconn, keepAliveInterval,
> +                               keepAliveCount) < 0)
> +        goto cleanup;
> +
> +    if (virConnectRegisterCloseCallback(dconn, libxlMigrationConnectionClosed,
> +                                        vm, NULL) < 0) {
> +        goto cleanup;
> +    }
> +
> +    virObjectUnlock(vm);
> +    useParams = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
> +                                         VIR_DRV_FEATURE_MIGRATION_PARAMS);
> +    virObjectLock(vm);
> +
> +    if (!useParams) {
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("Destination libvirt does not support migration with extensible parameters"));
> +        goto cleanup;
> +    }
> +
> +    ret = libxlDoMigrateP2P(driver, vm, sconn, xmlin, dconn, dconnuri,
> +                            dname, uri_str, flags);
> +
> + cleanup:
> +    orig_err = virSaveLastError();
> +    virObjectUnlock(vm);
> +    virConnectUnregisterCloseCallback(dconn, libxlMigrationConnectionClosed);
> +    virObjectUnref(dconn);
> +    virObjectLock(vm);
> +    if (orig_err) {
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
> +    }
> +    return ret;
> +}
> +
>  int
>  libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>                              virDomainObjPtr vm,
> diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h
> index 20b45d8..0f83bb4 100644
> --- a/src/libxl/libxl_migration.h
> +++ b/src/libxl/libxl_migration.h
> @@ -28,6 +28,7 @@
>  
>  # define LIBXL_MIGRATION_FLAGS                  \
>      (VIR_MIGRATE_LIVE |                         \
> +     VIR_MIGRATE_PEER2PEER |                    \
>       VIR_MIGRATE_UNDEFINE_SOURCE |              \
>       VIR_MIGRATE_PAUSED)
>  
> @@ -56,6 +57,16 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>                              unsigned int flags);
>  
>  int
> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
> +                               virDomainObjPtr vm,
> +                               virConnectPtr sconn,
> +                               const char *dom_xml,
> +                               const char *dconnuri,
> +                               const char *uri_str,
> +                               const char *dname,
> +                               unsigned int flags);
> +
> +int
>  libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>                              virDomainObjPtr vm,
>                              const char *dom_xml,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] libxl: add p2p migration
       [not found] ` <565F7E5F.2080808@suse.com>
@ 2015-12-03 19:18   ` Joao Martins
       [not found]   ` <56609569.5090903@oracle.com>
  1 sibling, 0 replies; 4+ messages in thread
From: Joao Martins @ 2015-12-03 19:18 UTC (permalink / raw)
  To: Jim Fehlig; +Cc: libvir-list, xen-devel



On 12/02/2015 11:27 PM, Jim Fehlig wrote:
> On 11/10/2015 08:32 AM, Joao Martins wrote:
>> Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
>> for supporting migration in Openstack. Most of the changes
>> occur at the source and no modifications at the receiver.
>>
>> In P2P mode there is only the Perform phase so we must handle
>> the connection with the destination and actually perform the
>> migration. libxlDomainPerformP2P implements the connection to
>> the destination and let libxlDoMigrateP2P implements the actual
>> migration logic with virConnectPtr. In this function we do
>> the migration steps in the destination similar to
>> virDomainMigrateVersion3Full. We appropriately save the last
>> error reported in each of the phases to provide proper
>> reporting. We don't yet support VIR_MIGRATE_TUNNELED and
>> we always use V3 with extensible params, making the
>> implementation simpler.
>>
>> It is worth noting that the receiver didn't have any changes,
>> and because it's still the v3 sequence thus it is possible to
>> migrate from a P2P to non-P2P host.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> Changes since v1:
>>  - Move Begin step to libxlDoMigrateP2P to have all 4 steps
>>  together.
>>  - Remove if before VIR_FREE(dom_xml)
>> ---
>>  src/libxl/libxl_driver.c    |  13 ++-
>>  src/libxl/libxl_migration.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
>>  src/libxl/libxl_migration.h |  11 +++
>>  3 files changed, 241 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index fcdcbdb..da98265 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
>>      switch (feature) {
>>      case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>>      case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>> +    case VIR_DRV_FEATURE_MIGRATION_P2P:
>>          return 1;
>>      default:
>>          return 0;
>> @@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>>      if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>>          goto cleanup;
>>  
>> -    if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>> -                                    uri, dname, flags) < 0)
>> -        goto cleanup;
>> +    if (flags & VIR_MIGRATE_PEER2PEER) {
>> +        if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
>> +                                           dconnuri, uri, dname, flags) < 0)
>> +            goto cleanup;
>> +    } else {
>> +        if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>> +                                        uri, dname, flags) < 0)
>> +            goto cleanup;
>> +    }
>>  
>>      ret = 0;
>>  
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> index 0d23e5f..a1c7b55 100644
>> --- a/src/libxl/libxl_migration.c
>> +++ b/src/libxl/libxl_migration.c
>> @@ -42,6 +42,7 @@
>>  #include "libxl_conf.h"
>>  #include "libxl_migration.h"
>>  #include "locking/domain_lock.h"
>> +#include "virtypedparam.h"
>>  
>>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>>  
>> @@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>>      return ret;
>>  }
>>  
>> +/* This function is a simplification of virDomainMigrateVersion3Full
>> + * excluding tunnel support and restricting it to migration v3
>> + * with params since it was the first to be introduced in libxl.
>> + */
>> +static int
>> +libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
>> +                  virDomainObjPtr vm,
>> +                  virConnectPtr sconn,
>> +                  const char *xmlin,
>> +                  virConnectPtr dconn,
>> +                  const char *dconnuri ATTRIBUTE_UNUSED,
>> +                  const char *dname,
>> +                  const char *uri,
>> +                  unsigned int flags)
>> +{
>> +    virDomainPtr ddomain = NULL;
>> +    virTypedParameterPtr params = NULL;
>> +    int nparams = 0;
>> +    int maxparams = 0;
>> +    char *uri_out = NULL;
>> +    char *dom_xml = NULL;
>> +    unsigned long destflags;
>> +    bool cancelled = true;
>> +    virErrorPtr orig_err = NULL;
>> +    int ret = -1;
>> +
>> +    dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
>> +    if (!dom_xml)
>> +        goto cleanup;
>> +
>> +    if (virTypedParamsAddString(&params, &nparams, &maxparams,
>> +                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
>> +        goto cleanup;
>> +
>> +    if (dname &&
>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>> +                                VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
>> +        goto cleanup;
>> +
>> +    if (uri &&
>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>> +                                VIR_MIGRATE_PARAM_URI, uri) < 0)
>> +        goto cleanup;
>> +
>> +    /* We don't require the destination to have P2P support
>> +     * as it looks to be normal migration from the receiver perpective.
>> +     */
>> +    destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
>> +
>> +    VIR_DEBUG("Prepare3");
>> +    virObjectUnlock(vm);
>> +    ret = dconn->driver->domainMigratePrepare3Params
>> +        (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
>> +    virObjectLock(vm);
> 
> Is it necessary to unlock and lock the vm while calling prepare and finish on
> the destination libvirtd?
> 
They can be removed, but then we leave the virDomainObj locked for a "long" time
when no operations are being done on his context. Plus no API calls could be
done there in the meantime (and migration can take a rather long time). Prepare,
Finish, ConnectOpenAuth and ConnectSupportsFeature  are done on the remote
driver (RPC calls to the target libvirtd) so they have a rather high "cost".
What do you think?

>> +
>> +    if (ret == -1)
>> +        goto cleanup;
>> +
>> +    if (uri_out) {
>> +        if (virTypedParamsReplaceString(&params, &nparams,
>> +                                        VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
>> +            orig_err = virSaveLastError();
>> +            goto finish;
>> +        }
>> +    } else {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("domainMigratePrepare3 did not set uri"));
>> +        goto finish;
>> +    }
>> +
>> +    VIR_DEBUG("Perform3 uri=%s", NULLSTR(uri_out));
>> +    ret = libxlDomainMigrationPerform(driver, vm, NULL, NULL,
>> +                                      uri_out, NULL, flags);
>> +
>> +    if (ret < 0)
>> +        orig_err = virSaveLastError();
>> +
>> +    cancelled = (ret < 0);
>> +
>> + finish:
>> +    VIR_DEBUG("Finish3 ret=%d", ret);
>> +    if (virTypedParamsGetString(params, nparams,
>> +                                VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 &&
>> +        virTypedParamsReplaceString(&params, &nparams,
>> +                                    VIR_MIGRATE_PARAM_DEST_NAME,
>> +                                    vm->def->name) < 0) {
>> +        ddomain = NULL;
>> +    } else {
>> +        virObjectUnlock(vm);
>> +        ddomain = dconn->driver->domainMigrateFinish3Params
>> +            (dconn, params, nparams, NULL, 0, NULL, NULL,
>> +             destflags, cancelled);
>> +        virObjectLock(vm);
>> +    }
>> +
>> +    cancelled = (ddomain == NULL);
>> +
>> +    /* If Finish3Params set an error, and we don't have an earlier
>> +     * one we need to preserve it in case confirm3 overwrites
>> +     */
>> +    if (!orig_err)
>> +        orig_err = virSaveLastError();
>> +
>> +    VIR_DEBUG("Confirm3 cancelled=%d vm=%p", cancelled, vm);
>> +    ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
>> +
>> +    if (ret < 0)
>> +        VIR_WARN("Guest %s probably left in 'paused' state on source",
>> +                 vm->def->name);
>> +
>> + cleanup:
>> +    if (ddomain) {
>> +        virObjectUnref(ddomain);
>> +        ret = 0;
>> +    } else {
>> +        ret = -1;
>> +    }
>> +
>> +    if (orig_err) {
>> +        virSetError(orig_err);
>> +        virFreeError(orig_err);
>> +    }
>> +
>> +    VIR_FREE(dom_xml);
>> +    VIR_FREE(uri_out);
>> +    virTypedParamsFree(params, nparams);
>> +    return ret;
>> +}
>> +
>> +static int virConnectCredType[] = {
>> +    VIR_CRED_AUTHNAME,
>> +    VIR_CRED_PASSPHRASE,
>> +};
>> +
>> +static virConnectAuth virConnectAuthConfig = {
>> +    .credtype = virConnectCredType,
>> +    .ncredtype = ARRAY_CARDINALITY(virConnectCredType),
>> +};
>> +
>> +static void
>> +libxlMigrationConnectionClosed(virConnectPtr conn,
>> +                               int reason,
>> +                               void *opaque)
>> +{
>> +    virDomainObjPtr vm = opaque;
>> +
>> +    VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name);
>> +    virDomainObjBroadcast(vm);
> 
> I would expect to see a corresponding virDomainObjWait for this call to
> virDomainObjBroadcast. But with the current migration code, I don't think the
> connect close callback is needed. Is there anything to do when the connection to
> destination libvirtd closes?
> 
You're right, it's my mistake. There is no one waiting for migration to complete
so this part isn't needed. I will remove the close callback on v3.

Regards,
Joao

>> +}
>> +
>> +/* On P2P mode there is only the Perform3 phase and we need to handle
>> + * the connection with the destination libvirtd and perform the migration.
>> + * Here we first tackle the first part of it, and libxlDoMigrationP2P handles
>> + * the migration process with an established virConnectPtr to the destination.
>> + */
>> +int
>> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
>> +                               virDomainObjPtr vm,
>> +                               virConnectPtr sconn,
>> +                               const char *xmlin,
>> +                               const char *dconnuri,
>> +                               const char *uri_str ATTRIBUTE_UNUSED,
>> +                               const char *dname,
>> +                               unsigned int flags)
>> +{
>> +    int ret = -1;
>> +    int keepAliveInterval = 5;
>> +    int keepAliveCount = 5;
>> +    bool useParams;
>> +    virConnectPtr dconn = NULL;
>> +    virErrorPtr orig_err = NULL;
>> +
>> +    virObjectUnlock(vm);
>> +    dconn = virConnectOpenAuth(dconnuri, &virConnectAuthConfig, 0);
>> +    virObjectLock(vm);
>> +
>> +    if (dconn == NULL) {
>> +        virReportError(VIR_ERR_OPERATION_FAILED,
>> +                       _("Failed to connect to remote libvirt URI %s: %s"),
>> +                       dconnuri, virGetLastErrorMessage());
>> +        return ret;
>> +    }
>> +
>> +    if (virConnectSetKeepAlive(dconn, keepAliveInterval,
>> +                               keepAliveCount) < 0)
>> +        goto cleanup;
>> +
>> +    if (virConnectRegisterCloseCallback(dconn, libxlMigrationConnectionClosed,
>> +                                        vm, NULL) < 0) {
>> +        goto cleanup;
>> +    }
>> +
>> +    virObjectUnlock(vm);
>> +    useParams = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
>> +                                         VIR_DRV_FEATURE_MIGRATION_PARAMS);
>> +    virObjectLock(vm);
>> +
>> +    if (!useParams) {
>> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> +                       _("Destination libvirt does not support migration with extensible parameters"));
>> +        goto cleanup;
>> +    }
>> +
>> +    ret = libxlDoMigrateP2P(driver, vm, sconn, xmlin, dconn, dconnuri,
>> +                            dname, uri_str, flags);
>> +
>> + cleanup:
>> +    orig_err = virSaveLastError();
>> +    virObjectUnlock(vm);
>> +    virConnectUnregisterCloseCallback(dconn, libxlMigrationConnectionClosed);
>> +    virObjectUnref(dconn);
>> +    virObjectLock(vm);
>> +    if (orig_err) {
>> +        virSetError(orig_err);
>> +        virFreeError(orig_err);
>> +    }
>> +    return ret;
>> +}
>> +
>>  int
>>  libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>>                              virDomainObjPtr vm,
>> diff --git a/src/libxl/libxl_migration.h b/src/libxl/libxl_migration.h
>> index 20b45d8..0f83bb4 100644
>> --- a/src/libxl/libxl_migration.h
>> +++ b/src/libxl/libxl_migration.h
>> @@ -28,6 +28,7 @@
>>  
>>  # define LIBXL_MIGRATION_FLAGS                  \
>>      (VIR_MIGRATE_LIVE |                         \
>> +     VIR_MIGRATE_PEER2PEER |                    \
>>       VIR_MIGRATE_UNDEFINE_SOURCE |              \
>>       VIR_MIGRATE_PAUSED)
>>  
>> @@ -56,6 +57,16 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>>                              unsigned int flags);
>>  
>>  int
>> +libxlDomainMigrationPerformP2P(libxlDriverPrivatePtr driver,
>> +                               virDomainObjPtr vm,
>> +                               virConnectPtr sconn,
>> +                               const char *dom_xml,
>> +                               const char *dconnuri,
>> +                               const char *uri_str,
>> +                               const char *dname,
>> +                               unsigned int flags);
>> +
>> +int
>>  libxlDomainMigrationPerform(libxlDriverPrivatePtr driver,
>>                              virDomainObjPtr vm,
>>                              const char *dom_xml,
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] libxl: add p2p migration
       [not found]   ` <56609569.5090903@oracle.com>
@ 2015-12-04 17:00     ` Jim Fehlig
  0 siblings, 0 replies; 4+ messages in thread
From: Jim Fehlig @ 2015-12-04 17:00 UTC (permalink / raw)
  To: Joao Martins; +Cc: libvir-list, xen-devel

On 12/03/2015 12:18 PM, Joao Martins wrote:
>
> On 12/02/2015 11:27 PM, Jim Fehlig wrote:
>> On 11/10/2015 08:32 AM, Joao Martins wrote:
>>> Introduce support for VIR_MIGRATE_PEER2PEER in libxl driver
>>> for supporting migration in Openstack. Most of the changes
>>> occur at the source and no modifications at the receiver.
>>>
>>> In P2P mode there is only the Perform phase so we must handle
>>> the connection with the destination and actually perform the
>>> migration. libxlDomainPerformP2P implements the connection to
>>> the destination and let libxlDoMigrateP2P implements the actual
>>> migration logic with virConnectPtr. In this function we do
>>> the migration steps in the destination similar to
>>> virDomainMigrateVersion3Full. We appropriately save the last
>>> error reported in each of the phases to provide proper
>>> reporting. We don't yet support VIR_MIGRATE_TUNNELED and
>>> we always use V3 with extensible params, making the
>>> implementation simpler.
>>>
>>> It is worth noting that the receiver didn't have any changes,
>>> and because it's still the v3 sequence thus it is possible to
>>> migrate from a P2P to non-P2P host.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> Changes since v1:
>>>  - Move Begin step to libxlDoMigrateP2P to have all 4 steps
>>>  together.
>>>  - Remove if before VIR_FREE(dom_xml)
>>> ---
>>>  src/libxl/libxl_driver.c    |  13 ++-
>>>  src/libxl/libxl_migration.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
>>>  src/libxl/libxl_migration.h |  11 +++
>>>  3 files changed, 241 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index fcdcbdb..da98265 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -4713,6 +4713,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature)
>>>      switch (feature) {
>>>      case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>>>      case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>>> +    case VIR_DRV_FEATURE_MIGRATION_P2P:
>>>          return 1;
>>>      default:
>>>          return 0;
>>> @@ -5039,9 +5040,15 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>>>      if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>>>          goto cleanup;
>>>  
>>> -    if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>>> -                                    uri, dname, flags) < 0)
>>> -        goto cleanup;
>>> +    if (flags & VIR_MIGRATE_PEER2PEER) {
>>> +        if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
>>> +                                           dconnuri, uri, dname, flags) < 0)
>>> +            goto cleanup;
>>> +    } else {
>>> +        if (libxlDomainMigrationPerform(driver, vm, dom_xml, dconnuri,
>>> +                                        uri, dname, flags) < 0)
>>> +            goto cleanup;
>>> +    }
>>>  
>>>      ret = 0;
>>>  
>>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>>> index 0d23e5f..a1c7b55 100644
>>> --- a/src/libxl/libxl_migration.c
>>> +++ b/src/libxl/libxl_migration.c
>>> @@ -42,6 +42,7 @@
>>>  #include "libxl_conf.h"
>>>  #include "libxl_migration.h"
>>>  #include "locking/domain_lock.h"
>>> +#include "virtypedparam.h"
>>>  
>>>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>>>  
>>> @@ -456,6 +457,225 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>>>      return ret;
>>>  }
>>>  
>>> +/* This function is a simplification of virDomainMigrateVersion3Full
>>> + * excluding tunnel support and restricting it to migration v3
>>> + * with params since it was the first to be introduced in libxl.
>>> + */
>>> +static int
>>> +libxlDoMigrateP2P(libxlDriverPrivatePtr driver,
>>> +                  virDomainObjPtr vm,
>>> +                  virConnectPtr sconn,
>>> +                  const char *xmlin,
>>> +                  virConnectPtr dconn,
>>> +                  const char *dconnuri ATTRIBUTE_UNUSED,
>>> +                  const char *dname,
>>> +                  const char *uri,
>>> +                  unsigned int flags)
>>> +{
>>> +    virDomainPtr ddomain = NULL;
>>> +    virTypedParameterPtr params = NULL;
>>> +    int nparams = 0;
>>> +    int maxparams = 0;
>>> +    char *uri_out = NULL;
>>> +    char *dom_xml = NULL;
>>> +    unsigned long destflags;
>>> +    bool cancelled = true;
>>> +    virErrorPtr orig_err = NULL;
>>> +    int ret = -1;
>>> +
>>> +    dom_xml = libxlDomainMigrationBegin(sconn, vm, xmlin);
>>> +    if (!dom_xml)
>>> +        goto cleanup;
>>> +
>>> +    if (virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (dname &&
>>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
>>> +        goto cleanup;
>>> +
>>> +    if (uri &&
>>> +        virTypedParamsAddString(&params, &nparams, &maxparams,
>>> +                                VIR_MIGRATE_PARAM_URI, uri) < 0)
>>> +        goto cleanup;
>>> +
>>> +    /* We don't require the destination to have P2P support
>>> +     * as it looks to be normal migration from the receiver perpective.
>>> +     */
>>> +    destflags = flags & ~(VIR_MIGRATE_PEER2PEER);
>>> +
>>> +    VIR_DEBUG("Prepare3");
>>> +    virObjectUnlock(vm);
>>> +    ret = dconn->driver->domainMigratePrepare3Params
>>> +        (dconn, params, nparams, NULL, 0, NULL, NULL, &uri_out, destflags);
>>> +    virObjectLock(vm);
>> Is it necessary to unlock and lock the vm while calling prepare and finish on
>> the destination libvirtd?
>>
> They can be removed, but then we leave the virDomainObj locked for a "long" time
> when no operations are being done on his context. Plus no API calls could be
> done there in the meantime (and migration can take a rather long time).

Ah, right. Dropping the lock is fine, but IMO we'll need a modify job to protect
the domain from modification by another thread. I think the existing migration
code needs improvement wrt job handling too :-/.

Regards,
Jim

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-04 17:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-10 15:32 [PATCH v2] libxl: add p2p migration Joao Martins
2015-12-02 23:27 ` Jim Fehlig
     [not found] ` <565F7E5F.2080808@suse.com>
2015-12-03 19:18   ` Joao Martins
     [not found]   ` <56609569.5090903@oracle.com>
2015-12-04 17:00     ` Jim Fehlig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.