9:52 a.m.
Current implementation of 'toURI' migration interfaces does not support all
combinations of interface versions and protocol versions. For example 'toURI2'
with p2p flag will not migrate if driver supports only v3params proto.
This is not convinient as drivers that starts to support migration have to
manually support older versions of protocol. I guess this should be done in
one place, namely here.
Another issue is that there are a lot of code duplication in implementation of
toURI interfaces and it is not obvious from code how are they related.
This implementation uses extensible parameters as intermediate parameters
representation. This is possible as interfaces are done backward compatible in
terms of parameters and later versions supports all parameters of former
versions.
= Changes from version 3
* fix spellings
* make error messages more specific as suggested
* in 'migration: refactor: introduce params version of unmanaged':
fix conversion for null values of explicit string params into virTypedParameterPtr.
* squash 'migration: reuse parameters check in toURI2 and toURI3' into
'migration: introduce parameter checking function'
* 'migration: refactor: one return in forURI family functions' is dropped as
suggested
src/libvirt-domain.c | 509 +++++++++++++++++++++++---------------------------
1 files changed, 237 insertions(+), 272 deletions(-)
9:52 a.m.
New subject: [libvirt] [PATCH 01/12] migration: refactor: get rid of use_params p2p_full
'useParams' parameter usage is an example of control coupling. Most of the work
inside the function is done differently except for the uri check. Lets split
this function into two, one with extensible parameters set and one with hardcoded
parameter set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 103 +++++++++++++++++++++++--------------------------
1 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 1edd73e..697d58d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3301,37 +3301,26 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
* only talks to the source libvirtd instance. The source libvirtd
* then opens its own connection to the destination and co-ordinates
* migration itself.
- *
- * If useParams is true, params and nparams contain migration parameters and
- * we know it's safe to call the API which supports extensible parameters.
- * Otherwise, we have to use xmlin, dname, uri, and bandwidth and pass them
- * to the old-style APIs.
*/
static int
-virDomainMigratePeer2PeerFull(virDomainPtr domain,
- const char *dconnuri,
- const char *xmlin,
- const char *dname,
- const char *uri,
- unsigned long long bandwidth,
- virTypedParameterPtr params,
- int nparams,
- bool useParams,
- unsigned int flags)
+virDomainMigratePeer2PeerPlain(virDomainPtr domain,
+ const char *xmlin,
+ unsigned int flags,
+ const char *dname,
+ const char *dconnuri,
+ const char *uri,
+ unsigned long long bandwidth)
{
virURIPtr tempuri = NULL;
VIR_DOMAIN_DEBUG(domain,
"dconnuri=%s, xmlin=%s, dname=%s, uri=%s, bandwidth=%llu
"
- "params=%p, nparams=%d, useParams=%d, flags=%x",
+ "flags=%x",
dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri),
- bandwidth, params, nparams, useParams, flags);
- VIR_TYPED_PARAMS_DEBUG(params, nparams);
+ bandwidth, flags);
- if ((useParams &&
!domain->conn->driver->domainMigratePerform3Params) ||
- (!useParams &&
- !domain->conn->driver->domainMigratePerform &&
- !domain->conn->driver->domainMigratePerform3)) {
+ if (!domain->conn->driver->domainMigratePerform &&
+ !domain->conn->driver->domainMigratePerform3) {
virReportUnsupportedError();
return -1;
}
@@ -3346,13 +3335,8 @@ virDomainMigratePeer2PeerFull(virDomainPtr domain,
}
virURIFree(tempuri);
- if (useParams) {
- VIR_DEBUG("Using migration protocol 3 with extensible parameters");
- return domain->conn->driver->domainMigratePerform3Params
- (domain, dconnuri, params, nparams,
- NULL, 0, NULL, NULL, flags);
- } else if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_V3)) {
+ if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_V3)) {
VIR_DEBUG("Using migration protocol 3");
return domain->conn->driver->domainMigratePerform3
(domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
@@ -3377,28 +3361,37 @@ virDomainMigratePeer2PeerFull(virDomainPtr domain,
static int
-virDomainMigratePeer2Peer(virDomainPtr domain,
- const char *xmlin,
- unsigned long flags,
- const char *dname,
- const char *dconnuri,
- const char *uri,
- unsigned long bandwidth)
-{
- return virDomainMigratePeer2PeerFull(domain, dconnuri, xmlin, dname, uri,
- bandwidth, NULL, 0, false, flags);
-}
-
-
-static int
virDomainMigratePeer2PeerParams(virDomainPtr domain,
const char *dconnuri,
virTypedParameterPtr params,
int nparams,
unsigned int flags)
{
- return virDomainMigratePeer2PeerFull(domain, dconnuri, NULL, NULL, NULL, 0,
- params, nparams, true, flags);
+ virURIPtr tempuri = NULL;
+
+ VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
+ dconnuri, params, nparams, flags);
+ VIR_TYPED_PARAMS_DEBUG(params, nparams);
+
+ if (!domain->conn->driver->domainMigratePerform3Params) {
+ virReportUnsupportedError();
+ return -1;
+ }
+
+ if (!(tempuri = virURIParse(dconnuri)))
+ return -1;
+ if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
+ virReportInvalidArg(dconnuri, "%s",
+ _("unable to parse server from dconnuri"));
+ virURIFree(tempuri);
+ return -1;
+ }
+ virURIFree(tempuri);
+
+ VIR_DEBUG("Using migration protocol 3 with extensible parameters");
+ return domain->conn->driver->domainMigratePerform3Params
+ (domain, dconnuri, params, nparams,
+ NULL, 0, NULL, NULL, flags);
}
@@ -3605,8 +3598,8 @@ virDomainMigrate(virDomainPtr domain,
}
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2Peer(domain, NULL, flags, dname,
- uri ? uri : dstURI, NULL, bandwidth) < 0) {
+ if (virDomainMigratePeer2PeerPlain(domain, NULL, flags, dname,
+ uri ? uri : dstURI, NULL, bandwidth) <
0) {
VIR_FREE(dstURI);
goto error;
}
@@ -3826,8 +3819,8 @@ virDomainMigrate2(virDomainPtr domain,
return NULL;
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2Peer(domain, dxml, flags, dname,
- dstURI, uri, bandwidth) < 0) {
+ if (virDomainMigratePeer2PeerPlain(domain, dxml, flags, dname,
+ dstURI, uri, bandwidth) < 0) {
VIR_FREE(dstURI);
goto error;
}
@@ -4207,8 +4200,8 @@ virDomainMigrateToURI(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2Peer(domain, NULL, flags,
- dname, duri, NULL, bandwidth) < 0)
+ if (virDomainMigratePeer2PeerPlain(domain, NULL, flags,
+ dname, duri, NULL, bandwidth) < 0)
goto error;
} else {
/* No peer to peer migration supported */
@@ -4353,8 +4346,8 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2Peer(domain, dxml, flags,
- dname, dconnuri, miguri, bandwidth) < 0)
+ if (virDomainMigratePeer2PeerPlain(domain, dxml, flags,
+ dname, dconnuri, miguri, bandwidth) <
0)
goto error;
} else {
/* No peer to peer migration supported */
@@ -4484,8 +4477,8 @@ virDomainMigrateToURI3(virDomainPtr domain,
goto error;
} else if (compat) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2Peer(domain, dxml, flags, dname,
- dconnuri, uri, bandwidth) < 0)
+ if (virDomainMigratePeer2PeerPlain(domain, dxml, flags, dname,
+ dconnuri, uri, bandwidth) < 0)
goto error;
} else {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 02/12] migration: refactor: reuse p2p url check
Refactor dconnuri local server URI check to common API.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 44 ++++++++++++++++++++++++--------------------
1 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 697d58d..2e43062 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3293,6 +3293,28 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
}
+static int ATTRIBUTE_NONNULL(1)
+virDomainMigrateCheckNotLocal(const char *dconnuri)
+{
+ virURIPtr tempuri = NULL;
+ int ret = -1;
+
+ if (!(tempuri = virURIParse(dconnuri)))
+ goto cleanup;
+ if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
+ virReportInvalidArg(dconnuri, "%s",
+ _("Attempt to migrate guest to the same host
%s"));
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+
+ virURIFree(tempuri);
+ return ret;
+}
+
+
/*
* In normal migration, the libvirt client co-ordinates communication
* between the 2 libvirtd instances on source & dest hosts.
@@ -3311,8 +3333,6 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
const char *uri,
unsigned long long bandwidth)
{
- virURIPtr tempuri = NULL;
-
VIR_DOMAIN_DEBUG(domain,
"dconnuri=%s, xmlin=%s, dname=%s, uri=%s, bandwidth=%llu
"
"flags=%x",
@@ -3325,15 +3345,8 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
return -1;
}
- if (!(tempuri = virURIParse(dconnuri)))
+ if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
- if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
- virReportInvalidArg(dconnuri, "%s",
- _("unable to parse server from dconnuri"));
- virURIFree(tempuri);
- return -1;
- }
- virURIFree(tempuri);
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_V3)) {
@@ -3367,8 +3380,6 @@ virDomainMigratePeer2PeerParams(virDomainPtr domain,
int nparams,
unsigned int flags)
{
- virURIPtr tempuri = NULL;
-
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
dconnuri, params, nparams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams);
@@ -3378,15 +3389,8 @@ virDomainMigratePeer2PeerParams(virDomainPtr domain,
return -1;
}
- if (!(tempuri = virURIParse(dconnuri)))
- return -1;
- if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
- virReportInvalidArg(dconnuri, "%s",
- _("unable to parse server from dconnuri"));
- virURIFree(tempuri);
+ if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
- }
- virURIFree(tempuri);
VIR_DEBUG("Using migration protocol 3 with extensible parameters");
return domain->conn->driver->domainMigratePerform3Params
--
1.7.1
1:32 p.m.
New subject: [libvirt] [PATCH 02/12] migration: refactor: reuse p2p url check
On Fri, Oct 02, 2015 at 10:52:42 +0300, Nikolay Shirokovskiy wrote:
Refactor dconnuri local server URI check to common API.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 44 ++++++++++++++++++++++++--------------------
1 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 697d58d..2e43062 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3293,6 +3293,28 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
}
+static int ATTRIBUTE_NONNULL(1)
+virDomainMigrateCheckNotLocal(const char *dconnuri)
+{
+ virURIPtr tempuri = NULL;
+ int ret = -1;
+
+ if (!(tempuri = virURIParse(dconnuri)))
+ goto cleanup;
+ if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
+ virReportInvalidArg(dconnuri, "%s",
+ _("Attempt to migrate guest to the same host
%s"));
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+
We usually put an empty line before the label, not after it.
+ virURIFree(tempuri);
+ return ret;
+}
Jirka
9:52 a.m.
New subject: [libvirt] [PATCH 03/12] migration: move implementation check to branches in p2p
This is more structured code so it will be easier to add branch for _PARAMS
protocol here. It is not a pure refactoring strictly speaking as we remove
scenarios for broken cases when driver defines V3 feature and implements
perform function. So it is additionally a more solid code.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2e43062..a7d3fbd 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3339,23 +3339,25 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri),
bandwidth, flags);
- if (!domain->conn->driver->domainMigratePerform &&
- !domain->conn->driver->domainMigratePerform3) {
- virReportUnsupportedError();
- return -1;
- }
-
if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_V3)) {
VIR_DEBUG("Using migration protocol 3");
+ if (!domain->conn->driver->domainMigratePerform3) {
+ virReportUnsupportedError();
+ return -1;
+ }
return domain->conn->driver->domainMigratePerform3
(domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
uri, flags, dname, bandwidth);
} else {
VIR_DEBUG("Using migration protocol 2");
+ if (!domain->conn->driver->domainMigratePerform) {
+ virReportUnsupportedError();
+ return -1;
+ }
if (xmlin) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Unable to change target guest XML during "
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 04/12] migration: remove direct migration dependency on version1 of driver
From: Michal Privoznik <mprivozn(a)redhat.com>
Direct migration should work if *perform3 is present but *perform
is not. This is situation when driver migration is implemented
after new version of driver function is introduced. We should not
be forced to support old version too as its parameter space is
subspace of newer one.
---
This patch has been already sent to mailist and has form
that finally suggested by Michal Privoznik. I just include
it in the set as it is not merged yet.
---
src/libvirt-domain.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a7d3fbd..1c6e27b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3425,16 +3425,15 @@ virDomainMigrateDirect(virDomainPtr domain,
NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(uri),
bandwidth);
- if (!domain->conn->driver->domainMigratePerform) {
- virReportUnsupportedError();
- return -1;
- }
-
/* Perform the migration. The driver isn't supposed to return
* until the migration is complete.
*/
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_V3)) {
+ if (!domain->conn->driver->domainMigratePerform3) {
+ virReportUnsupportedError();
+ return -1;
+ }
VIR_DEBUG("Using migration protocol 3");
/* dconn URI not relevant in direct migration, since no
* target libvirtd is involved */
@@ -3450,6 +3449,10 @@ virDomainMigrateDirect(virDomainPtr domain,
dname,
bandwidth);
} else {
+ if (!domain->conn->driver->domainMigratePerform) {
+ virReportUnsupportedError();
+ return -1;
+ }
VIR_DEBUG("Using migration protocol 2");
if (xmlin) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 05/12] migration: refactor: rename uri parameter to miguri
We use miguri name for this parameter in other places. So
make naming more consitent.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 1c6e27b..d164782 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3330,13 +3330,13 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
unsigned int flags,
const char *dname,
const char *dconnuri,
- const char *uri,
+ const char *miguri,
unsigned long long bandwidth)
{
VIR_DOMAIN_DEBUG(domain,
- "dconnuri=%s, xmlin=%s, dname=%s, uri=%s, bandwidth=%llu
"
- "flags=%x",
- dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri),
+ "dconnuri=%s, xmlin=%s, dname=%s, migrui=%s, "
+ "bandwidth=%llu, flags=%x",
+ dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(miguri),
bandwidth, flags);
if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
@@ -3351,7 +3351,7 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
}
return domain->conn->driver->domainMigratePerform3
(domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
- uri, flags, dname, bandwidth);
+ miguri, flags, dname, bandwidth);
} else {
VIR_DEBUG("Using migration protocol 2");
if (!domain->conn->driver->domainMigratePerform) {
@@ -3364,7 +3364,7 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
"migration"));
return -1;
}
- if (uri) {
+ if (miguri) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to override peer2peer migration URI"));
return -1;
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 06/12] migration: refactor: merge direct and p2p into unmanaged
p2p plain and direct function are good candidates for code reuse. Their main
function is same - to branch among different versions of migration protocol and
implementation of this function is also same. Also they have other common
functionality in lesser aspects. So let's merge them.
But as they have different signatures we have to get to convention on how to
pass direct migration 'uri' in 'dconnuri' and 'miguri'.
Fortunately we alreay
have such convention in parameters passed to toURI2 function, just let's follow
it. 'uri' is passed in miguri and dconnuri is ignored.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 148 ++++++++++++++++----------------------------------
1 files changed, 46 insertions(+), 102 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index d164782..36db29c 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3319,27 +3319,35 @@ virDomainMigrateCheckNotLocal(const char *dconnuri)
* In normal migration, the libvirt client co-ordinates communication
* between the 2 libvirtd instances on source & dest hosts.
*
- * In this peer-2-peer migration alternative, the libvirt client
- * only talks to the source libvirtd instance. The source libvirtd
- * then opens its own connection to the destination and co-ordinates
- * migration itself.
+ * This function encapsulates 2 alternatives to the above case.
+ *
+ * 1. peer-2-peer migration, the libvirt client only talks to the source
+ * libvirtd instance. The source libvirtd then opens its own
+ * connection to the destination and co-ordinates migration itself.
+ *
+ * 2. direct migration, where there is no requirement for a libvirtd instance
+ * on the dest host. Eg, XenD can talk direct to XenD, so libvirtd on dest
+ * does not need to be involved at all, or even running.
*/
static int
-virDomainMigratePeer2PeerPlain(virDomainPtr domain,
- const char *xmlin,
- unsigned int flags,
- const char *dname,
- const char *dconnuri,
- const char *miguri,
- unsigned long long bandwidth)
+virDomainMigrateUnmanaged(virDomainPtr domain,
+ const char *xmlin,
+ unsigned int flags,
+ const char *dname,
+ const char *dconnuri,
+ const char *miguri,
+ unsigned long long bandwidth)
{
+ const char *uri = NULL;
+
VIR_DOMAIN_DEBUG(domain,
"dconnuri=%s, xmlin=%s, dname=%s, migrui=%s, "
"bandwidth=%llu, flags=%x",
dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(miguri),
bandwidth, flags);
- if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
+ if ((flags & VIR_MIGRATE_PEER2PEER) &&
+ virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -3364,13 +3372,18 @@ virDomainMigratePeer2PeerPlain(virDomainPtr domain,
"migration"));
return -1;
}
- if (miguri) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to override peer2peer migration URI"));
- return -1;
+ if (flags & VIR_MIGRATE_PEER2PEER) {
+ if (miguri) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to override peer2peer migration
URI"));
+ return -1;
+ }
+ uri = dconnuri;
+ } else {
+ uri = miguri;
}
return domain->conn->driver->domainMigratePerform
- (domain, NULL, 0, dconnuri, flags, dname, bandwidth);
+ (domain, NULL, 0, uri, flags, dname, bandwidth);
}
}
@@ -3401,75 +3414,6 @@ virDomainMigratePeer2PeerParams(virDomainPtr domain,
}
-/*
- * In normal migration, the libvirt client co-ordinates communication
- * between the 2 libvirtd instances on source & dest hosts.
- *
- * Some hypervisors support an alternative, direct migration where
- * there is no requirement for a libvirtd instance on the dest host.
- * In this case
- *
- * eg, XenD can talk direct to XenD, so libvirtd on dest does not
- * need to be involved at all, or even running
- */
-static int
-virDomainMigrateDirect(virDomainPtr domain,
- const char *xmlin,
- unsigned long flags,
- const char *dname,
- const char *uri,
- unsigned long bandwidth)
-{
- VIR_DOMAIN_DEBUG(domain,
- "xmlin=%s, flags=%lx, dname=%s, uri=%s, bandwidth=%lu",
- NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(uri),
- bandwidth);
-
- /* Perform the migration. The driver isn't supposed to return
- * until the migration is complete.
- */
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_V3)) {
- if (!domain->conn->driver->domainMigratePerform3) {
- virReportUnsupportedError();
- return -1;
- }
- VIR_DEBUG("Using migration protocol 3");
- /* dconn URI not relevant in direct migration, since no
- * target libvirtd is involved */
- return domain->conn->driver->domainMigratePerform3(domain,
- xmlin,
- NULL, /* cookiein */
- 0, /* cookieinlen */
- NULL, /* cookieoutlen */
- NULL, /* cookieoutlen */
- NULL, /* dconnuri */
- uri,
- flags,
- dname,
- bandwidth);
- } else {
- if (!domain->conn->driver->domainMigratePerform) {
- virReportUnsupportedError();
- return -1;
- }
- VIR_DEBUG("Using migration protocol 2");
- if (xmlin) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("Unable to change target guest XML during
migration"));
- return -1;
- }
- return domain->conn->driver->domainMigratePerform(domain,
- NULL, /* cookie */
- 0, /* cookielen */
- uri,
- flags,
- dname,
- bandwidth);
- }
-}
-
-
/**
* virDomainMigrate:
* @domain: a domain object
@@ -3607,8 +3551,8 @@ virDomainMigrate(virDomainPtr domain,
}
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2PeerPlain(domain, NULL, flags, dname,
- uri ? uri : dstURI, NULL, bandwidth) <
0) {
+ if (virDomainMigrateUnmanaged(domain, NULL, flags, dname,
+ uri ? uri : dstURI, NULL, bandwidth) < 0) {
VIR_FREE(dstURI);
goto error;
}
@@ -3828,8 +3772,8 @@ virDomainMigrate2(virDomainPtr domain,
return NULL;
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2PeerPlain(domain, dxml, flags, dname,
- dstURI, uri, bandwidth) < 0) {
+ if (virDomainMigrateUnmanaged(domain, dxml, flags, dname,
+ dstURI, uri, bandwidth) < 0) {
VIR_FREE(dstURI);
goto error;
}
@@ -4209,8 +4153,8 @@ virDomainMigrateToURI(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2PeerPlain(domain, NULL, flags,
- dname, duri, NULL, bandwidth) < 0)
+ if (virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, duri, NULL, bandwidth) < 0)
goto error;
} else {
/* No peer to peer migration supported */
@@ -4221,8 +4165,8 @@ virDomainMigrateToURI(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
VIR_DEBUG("Using direct migration");
- if (virDomainMigrateDirect(domain, NULL, flags,
- dname, duri, bandwidth) < 0)
+ if (virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, NULL, duri, bandwidth) < 0)
goto error;
} else {
/* Cannot do a migration with only the perform step */
@@ -4355,8 +4299,8 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2PeerPlain(domain, dxml, flags,
- dname, dconnuri, miguri, bandwidth) <
0)
+ if (virDomainMigrateUnmanaged(domain, dxml, flags,
+ dname, dconnuri, miguri, bandwidth) < 0)
goto error;
} else {
/* No peer to peer migration supported */
@@ -4367,8 +4311,8 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
VIR_DEBUG("Using direct migration");
- if (virDomainMigrateDirect(domain, dxml, flags,
- dname, miguri, bandwidth) < 0)
+ if (virDomainMigrateUnmanaged(domain, dxml, flags,
+ dname, NULL, miguri, bandwidth) < 0)
goto error;
} else {
/* Cannot do a migration with only the perform step */
@@ -4486,8 +4430,8 @@ virDomainMigrateToURI3(virDomainPtr domain,
goto error;
} else if (compat) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigratePeer2PeerPlain(domain, dxml, flags, dname,
- dconnuri, uri, bandwidth) < 0)
+ if (virDomainMigrateUnmanaged(domain, dxml, flags, dname,
+ dconnuri, uri, bandwidth) < 0)
goto error;
} else {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -4514,8 +4458,8 @@ virDomainMigrateToURI3(virDomainPtr domain,
}
VIR_DEBUG("Using direct migration");
- if (virDomainMigrateDirect(domain, dxml, flags,
- dname, uri, bandwidth) < 0)
+ if (virDomainMigrateUnmanaged(domain, dxml, flags,
+ dname, NULL, uri, bandwidth) < 0)
goto error;
}
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 07/12] migration: refactor: introduce params version of unmanaged
Let's put main functionality into params version of virDomainMigrateUnmanaged
as a preparation step for merging it with virDomainMigratePeer2PeerParams.
virDomainMigrateUnmanaged then does nothing more then just adapting arguments.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 98 ++++++++++++++++++++++++++++++++++----------------
1 files changed, 67 insertions(+), 31 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 36db29c..3f2bb4a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3330,21 +3330,32 @@ virDomainMigrateCheckNotLocal(const char *dconnuri)
* does not need to be involved at all, or even running.
*/
static int
-virDomainMigrateUnmanaged(virDomainPtr domain,
- const char *xmlin,
- unsigned int flags,
- const char *dname,
- const char *dconnuri,
- const char *miguri,
- unsigned long long bandwidth)
+virDomainMigrateUnmanagedParams(virDomainPtr domain,
+ const char *dconnuri,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
{
const char *uri = NULL;
+ const char *miguri = NULL;
+ const char *dname = NULL;
+ const char *xmlin = NULL;
+ unsigned long long bandwidth = 0;
- VIR_DOMAIN_DEBUG(domain,
- "dconnuri=%s, xmlin=%s, dname=%s, migrui=%s, "
- "bandwidth=%llu, flags=%x",
- dconnuri, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(miguri),
- bandwidth, flags);
+ VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
+ dconnuri, params, nparams, flags);
+ VIR_TYPED_PARAMS_DEBUG(params, nparams);
+
+ if (virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
+ virTypedParamsGetULLong(params, nparams,
+ VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
+ return -1;
+ }
if ((flags & VIR_MIGRATE_PEER2PEER) &&
virDomainMigrateCheckNotLocal(dconnuri) < 0)
@@ -3414,6 +3425,46 @@ virDomainMigratePeer2PeerParams(virDomainPtr domain,
}
+static int
+virDomainMigrateUnmanaged(virDomainPtr domain,
+ const char *xmlin,
+ unsigned int flags,
+ const char *dname,
+ const char *dconnuri,
+ const char *miguri,
+ unsigned long long bandwidth)
+{
+ int ret = -1;
+ virTypedParameterPtr params = NULL;
+ int nparams = 0;
+ int maxparams = 0;
+
+ if (miguri &&
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_URI, miguri) < 0)
+ goto cleanup;
+ if (dname &&
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0)
+ goto cleanup;
+ if (xmlin &&
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_XML, xmlin) < 0)
+ goto cleanup;
+ if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_BANDWIDTH, bandwidth) < 0)
+ goto cleanup;
+
+ ret = virDomainMigrateUnmanagedParams(domain, dconnuri, params,
+ nparams, flags);
+
+ cleanup:
+ virTypedParamsFree(params, nparams);
+
+ return ret;
+}
+
+
/**
* virDomainMigrate:
* @domain: a domain object
@@ -4380,10 +4431,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_DEST_XML,
VIR_MIGRATE_PARAM_BANDWIDTH };
- const char *uri = NULL;
- const char *dname = NULL;
- const char *dxml = NULL;
- unsigned long long bandwidth = 0;
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparms=%u flags=%x",
NULLSTR(dconnuri), params, nparams, flags);
@@ -4402,17 +4449,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
compat = virTypedParamsCheck(params, nparams, compatParams,
ARRAY_CARDINALITY(compatParams));
- if (virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_URI, &uri) < 0 ||
- virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
- virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_DEST_XML, &dxml) < 0 ||
- virTypedParamsGetULLong(params, nparams,
- VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
- goto error;
- }
-
if (flags & VIR_MIGRATE_PEER2PEER) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
@@ -4430,8 +4466,8 @@ virDomainMigrateToURI3(virDomainPtr domain,
goto error;
} else if (compat) {
VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigrateUnmanaged(domain, dxml, flags, dname,
- dconnuri, uri, bandwidth) < 0)
+ if (virDomainMigrateUnmanagedParams(domain, dconnuri, params,
+ nparams, flags) < 0)
goto error;
} else {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -4458,8 +4494,8 @@ virDomainMigrateToURI3(virDomainPtr domain,
}
VIR_DEBUG("Using direct migration");
- if (virDomainMigrateUnmanaged(domain, dxml, flags,
- dname, NULL, uri, bandwidth) < 0)
+ if (virDomainMigrateUnmanagedParams(domain, NULL, params,
+ nparams, flags) < 0)
goto error;
}
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 08/12] migration: refactor: extract parameter adaption functions
Extract parameter adaptation and checking which is protocol dependent into
designated functions. Leave only branching and common checks in
virDomainMigrateUnmanagedParams.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 118 ++++++++++++++++++++++++++++++++++----------------
1 files changed, 80 insertions(+), 38 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3f2bb4a..fab9564 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3315,6 +3315,82 @@ virDomainMigrateCheckNotLocal(const char *dconnuri)
}
+static int
+virDomainMigrateUnmanagedProto2(virDomainPtr domain,
+ const char *dconnuri,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+ const char *uri = NULL;
+ const char *miguri = NULL;
+ const char *dname = NULL;
+ const char *xmlin = NULL;
+ unsigned long long bandwidth = 0;
+
+ if (virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
+ virTypedParamsGetULLong(params, nparams,
+ VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
+ return -1;
+ }
+
+ if (xmlin) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Unable to change target guest XML during "
+ "migration"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_PEER2PEER) {
+ if (miguri) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to override peer2peer migration URI"));
+ return -1;
+ }
+ uri = dconnuri;
+ } else {
+ uri = miguri;
+ }
+
+ return domain->conn->driver->domainMigratePerform
+ (domain, NULL, 0, uri, flags, dname, bandwidth);
+}
+
+
+static int
+virDomainMigrateUnmanagedProto3(virDomainPtr domain,
+ const char *dconnuri,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+ const char *miguri = NULL;
+ const char *dname = NULL;
+ const char *xmlin = NULL;
+ unsigned long long bandwidth = 0;
+
+ if (virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
+ virTypedParamsGetULLong(params, nparams,
+ VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
+ return -1;
+ }
+
+ return domain->conn->driver->domainMigratePerform3
+ (domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
+ miguri, flags, dname, bandwidth);
+}
+
+
/*
* In normal migration, the libvirt client co-ordinates communication
* between the 2 libvirtd instances on source & dest hosts.
@@ -3336,27 +3412,10 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
int nparams,
unsigned int flags)
{
- const char *uri = NULL;
- const char *miguri = NULL;
- const char *dname = NULL;
- const char *xmlin = NULL;
- unsigned long long bandwidth = 0;
-
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
dconnuri, params, nparams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams);
- if (virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
- virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
- virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
- virTypedParamsGetULLong(params, nparams,
- VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
- return -1;
- }
-
if ((flags & VIR_MIGRATE_PEER2PEER) &&
virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
@@ -3368,33 +3427,16 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
virReportUnsupportedError();
return -1;
}
- return domain->conn->driver->domainMigratePerform3
- (domain, xmlin, NULL, 0, NULL, NULL, dconnuri,
- miguri, flags, dname, bandwidth);
+ return virDomainMigrateUnmanagedProto3(domain, dconnuri,
+ params, nparams, flags);
} else {
VIR_DEBUG("Using migration protocol 2");
if (!domain->conn->driver->domainMigratePerform) {
virReportUnsupportedError();
return -1;
}
- if (xmlin) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("Unable to change target guest XML during "
- "migration"));
- return -1;
- }
- if (flags & VIR_MIGRATE_PEER2PEER) {
- if (miguri) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to override peer2peer migration
URI"));
- return -1;
- }
- uri = dconnuri;
- } else {
- uri = miguri;
- }
- return domain->conn->driver->domainMigratePerform
- (domain, NULL, 0, uri, flags, dname, bandwidth);
+ return virDomainMigrateUnmanagedProto2(domain, dconnuri,
+ params, nparams, flags);
}
}
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 09/12] migration: refactor: refactor parameter compatibility checks
Move virDomainMigrateUnmanagedProto* expected params list check into
function itself and use common virTypedParamsCheck for this purpose.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 58 +++++++++++++++++++++----------------------------
1 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fab9564..6261f6b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3322,30 +3322,32 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
int nparams,
unsigned int flags)
{
+ /* uri parameter is added for direct case */
+ const char *compatParams[] = { VIR_MIGRATE_PARAM_DEST_NAME,
+ VIR_MIGRATE_PARAM_BANDWIDTH,
+ VIR_MIGRATE_PARAM_URI };
const char *uri = NULL;
const char *miguri = NULL;
const char *dname = NULL;
- const char *xmlin = NULL;
unsigned long long bandwidth = 0;
+ if (!virTypedParamsCheck(params, nparams, compatParams,
+ ARRAY_CARDINALITY(compatParams))) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Some parameters are not supported by migration "
+ "protocol 2."));
+ return -1;
+ }
+
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
- virTypedParamsGetString(params, nparams,
- VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
virTypedParamsGetULLong(params, nparams,
VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
return -1;
}
- if (xmlin) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("Unable to change target guest XML during "
- "migration"));
- return -1;
- }
-
if (flags & VIR_MIGRATE_PEER2PEER) {
if (miguri) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -3369,11 +3371,23 @@ virDomainMigrateUnmanagedProto3(virDomainPtr domain,
int nparams,
unsigned int flags)
{
+ const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
+ VIR_MIGRATE_PARAM_DEST_NAME,
+ VIR_MIGRATE_PARAM_DEST_XML,
+ VIR_MIGRATE_PARAM_BANDWIDTH };
const char *miguri = NULL;
const char *dname = NULL;
const char *xmlin = NULL;
unsigned long long bandwidth = 0;
+ if (!virTypedParamsCheck(params, nparams, compatParams,
+ ARRAY_CARDINALITY(compatParams))) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Some parameters are not supported by migration "
+ "protocol 3"));
+ return -1;
+ }
+
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
virTypedParamsGetString(params, nparams,
@@ -4468,12 +4482,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
unsigned int nparams,
unsigned int flags)
{
- bool compat;
- const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
- VIR_MIGRATE_PARAM_DEST_NAME,
- VIR_MIGRATE_PARAM_DEST_XML,
- VIR_MIGRATE_PARAM_BANDWIDTH };
-
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparms=%u flags=%x",
NULLSTR(dconnuri), params, nparams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams);
@@ -4488,9 +4496,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
VIR_MIGRATE_NON_SHARED_INC,
error);
- compat = virTypedParamsCheck(params, nparams, compatParams,
- ARRAY_CARDINALITY(compatParams));
-
if (flags & VIR_MIGRATE_PEER2PEER) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_P2P)) {
@@ -4506,17 +4511,11 @@ virDomainMigrateToURI3(virDomainPtr domain,
if (virDomainMigratePeer2PeerParams(domain, dconnuri, params,
nparams, flags) < 0)
goto error;
- } else if (compat) {
+ } else {
VIR_DEBUG("Using peer2peer migration");
if (virDomainMigrateUnmanagedParams(domain, dconnuri, params,
nparams, flags) < 0)
goto error;
- } else {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("Peer-to-peer migration with extensible "
- "parameters is not supported but extended "
- "parameters were passed"));
- goto error;
}
} else {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -4528,13 +4527,6 @@ virDomainMigrateToURI3(virDomainPtr domain,
goto error;
}
- if (!compat) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("Direct migration does not support extensible "
- "parameters"));
- goto error;
- }
-
VIR_DEBUG("Using direct migration");
if (virDomainMigrateUnmanagedParams(domain, NULL, params,
nparams, flags) < 0)
--
1.7.1
1:32 p.m.
New subject: [libvirt] [PATCH 09/12] migration: refactor: refactor parameter compatibility checks
On Fri, Oct 02, 2015 at 10:52:49 +0300, Nikolay Shirokovskiy wrote:
Move virDomainMigrateUnmanagedProto* expected params list check into
function itself and use common virTypedParamsCheck for this purpose.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 58 +++++++++++++++++++++----------------------------
1 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fab9564..6261f6b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3322,30 +3322,32 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
int nparams,
unsigned int flags)
{
+ /* uri parameter is added for direct case */
+ const char *compatParams[] = { VIR_MIGRATE_PARAM_DEST_NAME,
+ VIR_MIGRATE_PARAM_BANDWIDTH,
+ VIR_MIGRATE_PARAM_URI };
const char *uri = NULL;
const char *miguri = NULL;
const char *dname = NULL;
- const char *xmlin = NULL;
unsigned long long bandwidth = 0;
+ if (!virTypedParamsCheck(params, nparams, compatParams,
+ ARRAY_CARDINALITY(compatParams))) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("Some parameters are not supported by migration "
+ "protocol 2."));
Drop '.' at the end of the message.
Jirka
9:52 a.m.
New subject: [libvirt] [PATCH 10/12] migration: merge all proto branches into single function
Finally on this step we get what we were aimed for - toURI{1, 2} (and
migration{*} APIs too) now can work thru V3_PARAMS protocol. Execution path
goes thru unchanged virDomainMigrateUnmanaged adapter function which is called
by all target places.
Note that we keep the fact that direct migration never works
thru V3_PARAMS proto. We can't change this aspect without
further investigation.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 56 ++++++++++++++-----------------------------------
1 files changed, 16 insertions(+), 40 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 6261f6b..2778a15 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3434,8 +3434,19 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
virDomainMigrateCheckNotLocal(dconnuri) < 0)
return -1;
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_V3)) {
+ if ((flags & VIR_MIGRATE_PEER2PEER) &&
+ VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_PARAMS)) {
+ VIR_DEBUG("Using migration protocol 3 with extensible parameters");
+ if (!domain->conn->driver->domainMigratePerform3Params) {
+ virReportUnsupportedError();
+ return -1;
+ }
+ return domain->conn->driver->domainMigratePerform3Params
+ (domain, dconnuri, params, nparams,
+ NULL, 0, NULL, NULL, flags);
+ } else if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_V3)) {
VIR_DEBUG("Using migration protocol 3");
if (!domain->conn->driver->domainMigratePerform3) {
virReportUnsupportedError();
@@ -3456,32 +3467,6 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
static int
-virDomainMigratePeer2PeerParams(virDomainPtr domain,
- const char *dconnuri,
- virTypedParameterPtr params,
- int nparams,
- unsigned int flags)
-{
- VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
- dconnuri, params, nparams, flags);
- VIR_TYPED_PARAMS_DEBUG(params, nparams);
-
- if (!domain->conn->driver->domainMigratePerform3Params) {
- virReportUnsupportedError();
- return -1;
- }
-
- if (virDomainMigrateCheckNotLocal(dconnuri) < 0)
- return -1;
-
- VIR_DEBUG("Using migration protocol 3 with extensible parameters");
- return domain->conn->driver->domainMigratePerform3Params
- (domain, dconnuri, params, nparams,
- NULL, 0, NULL, NULL, flags);
-}
-
-
-static int
virDomainMigrateUnmanaged(virDomainPtr domain,
const char *xmlin,
unsigned int flags,
@@ -4505,18 +4490,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
goto error;
}
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_PARAMS)) {
- VIR_DEBUG("Using peer2peer migration with extensible parameters");
- if (virDomainMigratePeer2PeerParams(domain, dconnuri, params,
- nparams, flags) < 0)
- goto error;
- } else {
- VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigrateUnmanagedParams(domain, dconnuri, params,
- nparams, flags) < 0)
- goto error;
- }
+ VIR_DEBUG("Using peer2peer migration");
+ if (virDomainMigrateUnmanagedParams(domain, dconnuri, params, nparams, flags)
< 0)
+ goto error;
} else {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
--
1.7.1
9:52 a.m.
New subject: [libvirt] [PATCH 11/12] migration: refactor: introduce parameter checking function
virDomainMigrateUnmanagedParams is not a good candidate for this functionality
as it is used by migrate family functions too and its have its own checks that
are superset of extracted and we don't need to check twice.
Actually name of the function is slightly misleading as there is also a check
for consistensy of flags parameter alone. So it could be refactored further and
reused by all migrate functions but for now let it be a matter of a different
patchset.
It is *not* a pure refactoring patch as it introduces offline check for older
versions. Looks like it must be done that way and no one will be broken too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 165 ++++++++++++++++++++-----------------------------
1 files changed, 67 insertions(+), 98 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2778a15..35dfa3f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4139,6 +4139,46 @@ virDomainMigrate3(virDomainPtr domain,
}
+static
+int virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain,
+ unsigned int flags)
+{
+ VIR_EXCLUSIVE_FLAGS_RET(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ -1);
+
+ if (flags & VIR_MIGRATE_OFFLINE &&
+ !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("offline migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_PEER2PEER) {
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_P2P)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("p2p migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+ } else {
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("direct migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+ }
+
+
+ return 0;
+}
+
+
/**
* virDomainMigrateToURI:
* @domain: a domain object
@@ -4217,6 +4257,9 @@ virDomainMigrateToURI(virDomainPtr domain,
const char *dname,
unsigned long bandwidth)
{
+ const char *dconnuri = NULL;
+ const char *miguri = NULL;
+
VIR_DOMAIN_DEBUG(domain, "duri=%p, flags=%lx, dname=%s, bandwidth=%lu",
NULLSTR(duri), flags, NULLSTR(dname), bandwidth);
@@ -4225,49 +4268,19 @@ virDomainMigrateToURI(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
-
virCheckNonNullArgGoto(duri, error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
- VIR_MIGRATE_NON_SHARED_INC,
- error);
-
- if (flags & VIR_MIGRATE_OFFLINE &&
- !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("offline migration is not supported by "
- "the source host"));
+ if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto error;
- }
- if (flags & VIR_MIGRATE_PEER2PEER) {
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_P2P)) {
- VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigrateUnmanaged(domain, NULL, flags,
- dname, duri, NULL, bandwidth) < 0)
- goto error;
- } else {
- /* No peer to peer migration supported */
- virReportUnsupportedError();
- goto error;
- }
- } else {
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
- VIR_DEBUG("Using direct migration");
- if (virDomainMigrateUnmanaged(domain, NULL, flags,
- dname, NULL, duri, bandwidth) < 0)
- goto error;
- } else {
- /* Cannot do a migration with only the perform step */
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("direct migration is not supported by the"
- " connection driver"));
- goto error;
- }
- }
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ dconnuri = duri;
+ else
+ miguri = duri;
+
+ if (virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, dconnuri, miguri, bandwidth) < 0)
+ goto error;
return 0;
@@ -4383,37 +4396,15 @@ virDomainMigrateToURI2(virDomainPtr domain,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
- VIR_MIGRATE_NON_SHARED_INC,
- error);
+ if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
+ goto error;
- if (flags & VIR_MIGRATE_PEER2PEER) {
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_P2P)) {
- VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigrateUnmanaged(domain, dxml, flags,
- dname, dconnuri, miguri, bandwidth) < 0)
- goto error;
- } else {
- /* No peer to peer migration supported */
- virReportUnsupportedError();
- goto error;
- }
- } else {
- if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
- VIR_DEBUG("Using direct migration");
- if (virDomainMigrateUnmanaged(domain, dxml, flags,
- dname, NULL, miguri, bandwidth) < 0)
- goto error;
- } else {
- /* Cannot do a migration with only the perform step */
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("direct migration is not supported by the"
- " connection driver"));
- goto error;
- }
- }
+ if (!(flags & VIR_MIGRATE_PEER2PEER))
+ dconnuri = NULL;
+
+ if (virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, dconnuri, miguri, bandwidth) < 0)
+ goto error;
return 0;
@@ -4477,37 +4468,15 @@ virDomainMigrateToURI3(virDomainPtr domain,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
- VIR_MIGRATE_NON_SHARED_INC,
- error);
-
- if (flags & VIR_MIGRATE_PEER2PEER) {
- if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_P2P)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("Peer-to-peer migration is not supported by "
- "the connection driver"));
- goto error;
- }
+ if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
+ goto error;
- VIR_DEBUG("Using peer2peer migration");
- if (virDomainMigrateUnmanagedParams(domain, dconnuri, params, nparams, flags)
< 0)
- goto error;
- } else {
- if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
- /* Cannot do a migration with only the perform step */
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("Direct migration is not supported by the"
- " connection driver"));
- goto error;
- }
+ if (!(flags & VIR_MIGRATE_PEER2PEER))
+ dconnuri = NULL;
- VIR_DEBUG("Using direct migration");
- if (virDomainMigrateUnmanagedParams(domain, NULL, params,
- nparams, flags) < 0)
- goto error;
- }
+ if (virDomainMigrateUnmanagedParams(domain, dconnuri,
+ params, nparams, flags) < -1)
+ goto error;
return 0;
--
1.7.1
1:32 p.m.
New subject: [libvirt] [PATCH 11/12] migration: refactor: introduce parameter checking function
On Fri, Oct 02, 2015 at 10:52:51 +0300, Nikolay Shirokovskiy wrote:
virDomainMigrateUnmanagedParams is not a good candidate for this
functionality
as it is used by migrate family functions too and its have its own checks that
are superset of extracted and we don't need to check twice.
Actually name of the function is slightly misleading as there is also a check
for consistensy of flags parameter alone. So it could be refactored further and
reused by all migrate functions but for now let it be a matter of a different
patchset.
It is *not* a pure refactoring patch as it introduces offline check for older
versions. Looks like it must be done that way and no one will be broken too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 165 ++++++++++++++++++++-----------------------------
1 files changed, 67 insertions(+), 98 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2778a15..35dfa3f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4139,6 +4139,46 @@ virDomainMigrate3(virDomainPtr domain,
}
+static
+int virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain,
+ unsigned int flags)
+{
+ VIR_EXCLUSIVE_FLAGS_RET(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ -1);
+
+ if (flags & VIR_MIGRATE_OFFLINE &&
+ !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("offline migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_PEER2PEER) {
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_P2P)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("p2p migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+ } else {
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_DIRECT)) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("direct migration is not supported by "
+ "the source host"));
+ return -1;
+ }
+ }
+
+
One empty line would be enough :-)
+ return 0;
+}
Jirka
9:52 a.m.
New subject: [libvirt] [PATCH 12/12] migration: check dconnuri in p2p mode
Check dconnuri is not null or we will catch nullpointer later.
I hope this makes Coverity happy.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 35dfa3f..b9e6ac9 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4399,7 +4399,9 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto error;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, error);
+ else
dconnuri = NULL;
if (virDomainMigrateUnmanaged(domain, NULL, flags,
@@ -4471,7 +4473,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto error;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, error);
+ else
dconnuri = NULL;
if (virDomainMigrateUnmanagedParams(domain, dconnuri,
--
1.7.1
1:34 p.m.
On Fri, Oct 02, 2015 at 10:52:40 +0300, Nikolay Shirokovskiy wrote:
Current implementation of 'toURI' migration interfaces does
not support all
combinations of interface versions and protocol versions. For example 'toURI2'
with p2p flag will not migrate if driver supports only v3params proto.
This is not convinient as drivers that starts to support migration have to
manually support older versions of protocol. I guess this should be done in
one place, namely here.
Another issue is that there are a lot of code duplication in implementation of
toURI interfaces and it is not obvious from code how are they related.
This implementation uses extensible parameters as intermediate parameters
representation. This is possible as interfaces are done backward compatible in
terms of parameters and later versions supports all parameters of former
versions.
= Changes from version 3
* fix spellings
* make error messages more specific as suggested
* in 'migration: refactor: introduce params version of unmanaged':
fix conversion for null values of explicit string params into virTypedParameterPtr.
* squash 'migration: reuse parameters check in toURI2 and toURI3' into
'migration: introduce parameter checking function'
* 'migration: refactor: one return in forURI family functions' is dropped as
suggested
src/libvirt-domain.c | 509 +++++++++++++++++++++++---------------------------
1 files changed, 237 insertions(+), 272 deletions(-)
ACK series and pushed with the three cosmetics issues fixed.
Thanks for your work.
Jirka
3262
days inactive
3272
days old
18 comments
2 participants
participants (2)
-
Jiri Denemark -
Nikolay Shirokovskiy