11:05 p.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 version2
1. fix misc typos and overall english in commit messages
2. fix misc style issues
3. add 2 new patches
1. migration: refactor: rename uri parameter to miguri
2. migration: check dconnuri in p2p mode
4. rework succession of 2 patches
1. migration: refactor: prepare to reuse flag vs feature checks
2. migration: reuse flags vs features checks in toURI family
into
1. migration: refactor: introduce parameter checking function
2. migration: reuse parameters check in toURI2 and toURI3
5. rearrange functions as suggested by reviwer to make better diffs
The overall diff as compared to the previous patchset is really small.
src/libvirt-domain.c | 531 ++++++++++++++++++++++---------------------------
1 files changed, 238 insertions(+), 293 deletions(-)
11:05 p.m.
New subject: [libvirt] [PATCH v3 01/14] 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
10:12 p.m.
New subject: [libvirt] [PATCH v3 01/14] migration: refactor: get rid of use_params p2p_full
On Fri, Sep 18, 2015 at 18:05:39 +0300, Nikolay Shirokovskiy wrote:
'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(-)
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 02/14] 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
10:12 p.m.
New subject: [libvirt] [PATCH v3 02/14] migration: refactor: reuse p2p url check
On Fri, Sep 18, 2015 at 18:05:40 +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"));
Hmm, I don't think this new error message is better than the old one.
But anyway, we shouldn't need this code at all, checking dconnuri on a
client and then passing it to libvirtd which will have to check it again
does not make a lot of sense.
In other words, I think we should just completely remove the dconuri
checks from virDomainMigratePeer2Peer*.
Jirka
3:20 p.m.
New subject: [libvirt] [PATCH v3 02/14] migration: refactor: reuse p2p url check
On 25.09.2015 17:12, Jiri Denemark wrote:
On Fri, Sep 18, 2015 at 18:05:40 +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"));
Hmm, I don't think this new error message is better than the old one.
But anyway, we shouldn't need this code at all, checking dconnuri on a
client and then passing it to libvirtd which will have to check it again
does not make a lot of sense.
In other words, I think we should just completely remove the dconuri
checks from virDomainMigratePeer2Peer*.
Jirka
Sorry, for delay. I hoped you'll finish series review.
The meaning of this check is unclear, so i found it origins, see
https://www.redhat.com/archives/libvir-list/2011-January/msg00437.html.
This message states that without this check we can get a deadlock, thus
I leave it.
Also I investigate if this check had been moved to qemu code as reviewer suggested.
Looks likes it is not.
10:12 p.m.
New subject: [libvirt] [PATCH v3 02/14] migration: refactor: reuse p2p url check
On Wed, Sep 30, 2015 at 10:20:23 +0300, Nikolay Shirokovskiy wrote:
On 25.09.2015 17:12, Jiri Denemark wrote:
> On Fri, Sep 18, 2015 at 18:05:40 +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"));
>
> Hmm, I don't think this new error message is better than the old one.
> But anyway, we shouldn't need this code at all, checking dconnuri on a
> client and then passing it to libvirtd which will have to check it again
> does not make a lot of sense.
>
> In other words, I think we should just completely remove the dconuri
> checks from virDomainMigratePeer2Peer*.
>
> Jirka
>
Sorry, for delay. I hoped you'll finish series review.
Yeah, sorry about that. A long weekend came into my review session :-)
The meaning of this check is unclear, so i found it origins, see
https://www.redhat.com/archives/libvir-list/2011-January/msg00437.html.
This message states that without this check we can get a deadlock, thus
I leave it.
Also I investigate if this check had been moved to qemu code as reviewer suggested.
Looks likes it is not.
I see, unless we have the check in the qemu driver since a long time
ago, I'm afraid we need to keep it in libvirt-domain.c.
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 03/14] 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
10:12 p.m.
New subject: [libvirt] [PATCH v3 03/14] migration: move implementation check to branches in p2p
On Fri, Sep 18, 2015 at 18:05:41 +0300, Nikolay Shirokovskiy wrote:
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(-)
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 04/14] 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
10:12 p.m.
New subject: [libvirt] [PATCH v3 04/14] migration: remove direct migration dependency on version1 of driver
On Fri, Sep 18, 2015 at 18:05:42 +0300, Nikolay Shirokovskiy wrote:
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(-)
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 05/14] 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
10:12 p.m.
New subject: [libvirt] [PATCH v3 05/14] migration: refactor: rename uri parameter to miguri
On Fri, Sep 18, 2015 at 18:05:43 +0300, Nikolay Shirokovskiy wrote:
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(-)
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 06/14] 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..f52c3bf 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 incapsulates 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
10:13 p.m.
New subject: [libvirt] [PATCH v3 06/14] migration: refactor: merge direct and p2p into unmanaged
On Fri, Sep 18, 2015 at 18:05:44 +0300, Nikolay Shirokovskiy wrote:
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..f52c3bf 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 incapsulates 2 alternatives to the above case.
s/incapsulates/encapsulates/
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 07/14] 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 | 92 +++++++++++++++++++++++++++++++++-----------------
1 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f52c3bf..330ccab 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,40 @@ 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 (virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_URI, miguri) < 0 ||
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0 ||
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_XML, xmlin) < 0 ||
+ 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 +4425,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 +4443,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 +4460,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 +4488,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
10:13 p.m.
New subject: [libvirt] [PATCH v3 07/14] migration: refactor: introduce params version of unmanaged
On Fri, Sep 18, 2015 at 18:05:45 +0300, Nikolay Shirokovskiy wrote:
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 | 92 +++++++++++++++++++++++++++++++++-----------------
1 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f52c3bf..330ccab 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
...
+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 (virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_URI, miguri) < 0 ||
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_NAME, dname) < 0 ||
+ virTypedParamsAddString(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_DEST_XML, xmlin) < 0 ||
+ virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
+ VIR_MIGRATE_PARAM_BANDWIDTH, bandwidth) < 0)
+ goto cleanup;
All the *AddString calls should only be done when the appropriate value
is not NULL, otherwise *AddString will actually add "" into params and
thus you will get "" rather than NULL in virDomainMigrateUnmanagedParams.
In other words, something like
if (miguri &&
virTypedParamsAddString(¶ms, &nparams, &maxparams,
VIR_MIGRATE_PARAM_URI, miguri) < 0)
goto cleanup;
...
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 08/14] 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 330ccab..f87a22d 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:38 p.m.
New subject: [libvirt] [PATCH v3 08/14] migration: refactor: extract parameter adaption functions
On Fri, Sep 18, 2015 at 18:05:46 +0300, Nikolay Shirokovskiy wrote:
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 330ccab..f87a22d 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",
While at it, you could also change VIR_ERR_INTERNAL_ERROR to a more
appropriate VIR_ERR_ARGUMENT_UNSUPPORTED. But it can be done in a
separate patch.
...
At first sight I was thinking this patch makes the code even worse since
a new parameter would need to be added in more places, but that's
addressed in the next patch.
ACK
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 09/14] 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 | 56 ++++++++++++++++++++-----------------------------
1 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f87a22d..abed9d6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3322,30 +3322,31 @@ 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",
+ _("Migration does not support some of parameters."));
+ 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 +3370,22 @@ 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",
+ _("Migration does not support some of parameters."));
+ return -1;
+ }
+
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
virTypedParamsGetString(params, nparams,
@@ -4462,12 +4474,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);
@@ -4482,9 +4488,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)) {
@@ -4500,17 +4503,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,
@@ -4522,13 +4519,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
9:38 p.m.
New subject: [libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks
On Fri, Sep 18, 2015 at 18:05:47 +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 | 56 ++++++++++++++++++++-----------------------------
1 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f87a22d..abed9d6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3322,30 +3322,31 @@ 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",
+ _("Migration does not support some of parameters."));
How about "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",
_("Unable to override peer2peer migration URI"));
return -1;
}
Can we merge this check with with the others by adding
VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
passed? I think it would be a bit cleaner, although it depends on how
dirty way of doing that you choose :-)
@@ -3369,11 +3370,22 @@ 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",
+ _("Migration does not support some of parameters."));
How about "Some parameters are not supported by migration protocol 3"?
+ return -1;
+ }
+
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
virTypedParamsGetString(params, nparams,
Jirka
3:34 p.m.
New subject: [libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks
On 30.09.2015 16:38, Jiri Denemark wrote:
On Fri, Sep 18, 2015 at 18:05:47 +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 | 56 ++++++++++++++++++++-----------------------------
> 1 files changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index f87a22d..abed9d6 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3322,30 +3322,31 @@ 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",
> + _("Migration does not support some of
parameters."));
How about "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",
_("Unable to override peer2peer migration
URI"));
return -1;
}
Can we merge this check with with the others by adding
VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
passed? I think it would be a bit cleaner, although it depends on how
dirty way of doing that you choose :-)
I'm not sure is this considered dirty?
1. char** cast (could be fixed by changing function signature to const char * const *
2. unchecked array accessing - safe until somebody will edit this code
const char *compatParams[3] = { VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH,
NULL};
size_t ncompatParams = 0;
ncompatParams = virStringListLength((char**)compatParams);
if (!(flags & VIR_MIGRATE_PEER2PEER))
compatParams[ncompatParams++] = VIR_MIGRATE_PARAM_URI;
> @@ -3369,11 +3370,22 @@ 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",
> + _("Migration does not support some of
parameters."));
How about "Some parameters are not supported by migration protocol 3"?
> + return -1;
> + }
> +
> if (virTypedParamsGetString(params, nparams,
> VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
> virTypedParamsGetString(params, nparams,
Jirka
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:30 p.m.
New subject: [libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks
On Thu, Oct 01, 2015 at 10:34:49 +0300, Nikolay Shirokovskiy wrote:
On 30.09.2015 16:38, Jiri Denemark wrote:
> On Fri, Sep 18, 2015 at 18:05:47 +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 | 56 ++++++++++++++++++++-----------------------------
>> 1 files changed, 23 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index f87a22d..abed9d6 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -3322,30 +3322,31 @@ 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",
>> + _("Migration does not support some of
parameters."));
>
> How about "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",
> _("Unable to override peer2peer migration
URI"));
> return -1;
> }
>
> Can we merge this check with with the others by adding
> VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
> passed? I think it would be a bit cleaner, although it depends on how
> dirty way of doing that you choose :-)
I'm not sure is this considered dirty?
1. char** cast (could be fixed by changing function signature to const char * const *
2. unchecked array accessing - safe until somebody will edit this code
const char *compatParams[3] = { VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH,
NULL};
size_t ncompatParams = 0;
ncompatParams = virStringListLength((char**)compatParams);
if (!(flags & VIR_MIGRATE_PEER2PEER))
compatParams[ncompatParams++] = VIR_MIGRATE_PARAM_URI;
I don't know, I was thinking about something similar:
const char *compatParams[] = {VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH,
VIR_MIGRATE_PARAM_URI};
size_t ncompatParams = ARRAY_CARDINALITY(compatParams);
if (flags & VIR_MIGRATE_PEER2PEER)
ncompatParams--;
if (!virTypedParamsCheck(params, nparams, compatParams,
ncompatParams) ...
but it looked similarly fragile. Maybe defining two arrays, one for p2p
and one for direct migration would be the right clean way of doing
this...
Jirka
9:48 p.m.
New subject: [libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks
On 01.10.2015 12:30, Jiri Denemark wrote:
On Thu, Oct 01, 2015 at 10:34:49 +0300, Nikolay Shirokovskiy wrote:
>
>
> On 30.09.2015 16:38, Jiri Denemark wrote:
>> On Fri, Sep 18, 2015 at 18:05:47 +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 | 56
++++++++++++++++++++-----------------------------
>>> 1 files changed, 23 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>>> index f87a22d..abed9d6 100644
>>> --- a/src/libvirt-domain.c
>>> +++ b/src/libvirt-domain.c
>>> @@ -3322,30 +3322,31 @@ 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",
>>> + _("Migration does not support some of
parameters."));
>>
>> How about "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",
>> _("Unable to override peer2peer migration
URI"));
>> return -1;
>> }
>>
>> Can we merge this check with with the others by adding
>> VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
>> passed? I think it would be a bit cleaner, although it depends on how
>> dirty way of doing that you choose :-)
> I'm not sure is this considered dirty?
>
> 1. char** cast (could be fixed by changing function signature to const char * const
*
> 2. unchecked array accessing - safe until somebody will edit this code
>
>
> const char *compatParams[3] = { VIR_MIGRATE_PARAM_DEST_NAME,
> VIR_MIGRATE_PARAM_BANDWIDTH,
> NULL};
> size_t ncompatParams = 0;
>
> ncompatParams = virStringListLength((char**)compatParams);
> if (!(flags & VIR_MIGRATE_PEER2PEER))
> compatParams[ncompatParams++] = VIR_MIGRATE_PARAM_URI;
I don't know, I was thinking about something similar:
const char *compatParams[] = {VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH,
VIR_MIGRATE_PARAM_URI};
size_t ncompatParams = ARRAY_CARDINALITY(compatParams);
if (flags & VIR_MIGRATE_PEER2PEER)
ncompatParams--;
if (!virTypedParamsCheck(params, nparams, compatParams,
ncompatParams) ...
but it looked similarly fragile. Maybe defining two arrays, one for p2p
and one for direct migration would be the right clean way of doing
this...
then it became bulky, like
const char *compatParamsP2P[] = { VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH };
const char *compatParamsDirect[] = { 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;
unsigned long long bandwidth = 0;
if (flags & VIR_MIGRATE_PEER2PEER) {
if (!virTypedParamsCheck(params, nparams, compatParamsP2P,
ARRAY_CARDINALITY(compatParamsP2P))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Some parameters are not supported by migration "
"protocol 2."));
return -1;
}
} else {
if (!virTypedParamsCheck(params, nparams, compatParamsDirect,
ARRAY_CARDINALITY(compatParamsDirect))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Some parameters are not supported by migration "
"protocol 2."));
return -1;
}
}
May be return to the first variant. Anyway uri parameter is special case.
Jirka
11:09 p.m.
New subject: [libvirt] [PATCH v3 09/14] migration: refactor: refactor parameter compatibility checks
On Thu, Oct 01, 2015 at 16:48:44 +0300, Nikolay Shirokovskiy wrote:
On 01.10.2015 12:30, Jiri Denemark wrote:
> On Thu, Oct 01, 2015 at 10:34:49 +0300, Nikolay Shirokovskiy wrote:
>>
>>
>> On 30.09.2015 16:38, Jiri Denemark wrote:
>>> On Fri, Sep 18, 2015 at 18:05:47 +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 | 56
++++++++++++++++++++-----------------------------
>>>> 1 files changed, 23 insertions(+), 33 deletions(-)
>>>>
>>>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>>>> index f87a22d..abed9d6 100644
>>>> --- a/src/libvirt-domain.c
>>>> +++ b/src/libvirt-domain.c
>>>> @@ -3322,30 +3322,31 @@ 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",
>>>> + _("Migration does not support some of
parameters."));
>>>
>>> How about "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",
>>> _("Unable to override peer2peer migration
URI"));
>>> return -1;
>>> }
>>>
>>> Can we merge this check with with the others by adding
>>> VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
>>> passed? I think it would be a bit cleaner, although it depends on how
>>> dirty way of doing that you choose :-)
>> I'm not sure is this considered dirty?
>>
>> 1. char** cast (could be fixed by changing function signature to const char *
const *
>> 2. unchecked array accessing - safe until somebody will edit this code
>>
>>
>> const char *compatParams[3] = { VIR_MIGRATE_PARAM_DEST_NAME,
>> VIR_MIGRATE_PARAM_BANDWIDTH,
>> NULL};
>> size_t ncompatParams = 0;
>>
>> ncompatParams = virStringListLength((char**)compatParams);
>> if (!(flags & VIR_MIGRATE_PEER2PEER))
>> compatParams[ncompatParams++] = VIR_MIGRATE_PARAM_URI;
>
> I don't know, I was thinking about something similar:
>
> const char *compatParams[] = {VIR_MIGRATE_PARAM_DEST_NAME,
> VIR_MIGRATE_PARAM_BANDWIDTH,
> VIR_MIGRATE_PARAM_URI};
> size_t ncompatParams = ARRAY_CARDINALITY(compatParams);
>
> if (flags & VIR_MIGRATE_PEER2PEER)
> ncompatParams--;
>
> if (!virTypedParamsCheck(params, nparams, compatParams,
> ncompatParams) ...
>
> but it looked similarly fragile. Maybe defining two arrays, one for p2p
> and one for direct migration would be the right clean way of doing
> this...
then it became bulky, like
const char *compatParamsP2P[] = { VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH };
const char *compatParamsDirect[] = { 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;
unsigned long long bandwidth = 0;
if (flags & VIR_MIGRATE_PEER2PEER) {
if (!virTypedParamsCheck(params, nparams, compatParamsP2P,
ARRAY_CARDINALITY(compatParamsP2P))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Some parameters are not supported by migration "
"protocol 2."));
return -1;
}
} else {
if (!virTypedParamsCheck(params, nparams, compatParamsDirect,
ARRAY_CARDINALITY(compatParamsDirect))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Some parameters are not supported by migration "
"protocol 2."));
return -1;
}
}
Well, you could make it a bit shorter:
const char **compat;
size_t ncompat;
if (flags & VIR_MIGRATE_PEER2PEER) {
compat = compatParamsP2P;
ncompat = ARRAY_CARDINALITY(compatParamsP2P);
} else {
compat = compatParamsDirect;
ncompat = ARRAY_CARDINALITY(compatParamsDirect);
}
if (!virTypedParamsCheck(params, nparams, compat, ncompat) ...
May be return to the first variant. Anyway uri parameter is special
case.
Yeah, looking at all other options I think your original code was the
best :-) So just keep it unchanged.
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 10/14] 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 abed9d6..5c22460 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3432,8 +3432,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();
@@ -3454,32 +3465,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,
@@ -4497,18 +4482,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:38 p.m.
New subject: [libvirt] [PATCH v3 10/14] migration: merge all proto branches into single function
On Fri, Sep 18, 2015 at 18:05:48 +0300, Nikolay Shirokovskiy wrote:
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 abed9d6..5c22460 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3432,8 +3432,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) &&
I though one of the reason you're doing all this is to remove the need
to implement older API for direct migration so I was expecting this part
of the condition to go away...
+ 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();
Jirka
3:43 p.m.
New subject: [libvirt] [PATCH v3 10/14] migration: merge all proto branches into single function
On 30.09.2015 16:38, Jiri Denemark wrote:
On Fri, Sep 18, 2015 at 18:05:48 +0300, Nikolay Shirokovskiy wrote:
> 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 abed9d6..5c22460 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3432,8 +3432,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) &&
I though one of the reason you're doing all this is to remove the need
to implement older API for direct migration so I was expecting this part
of the condition to go away...
This patchset is large enough) I'd better leave the answer to the question
is this check could be safely deleted to different patch series.
> + 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();
Jirka
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:05 p.m.
New subject: [libvirt] [PATCH v3 11/14] 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.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 83 +++++++++++++++++++++++++++-----------------------
1 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5c22460..eec45bd 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4131,6 +4131,39 @@ virDomainMigrate3(virDomainPtr domain,
}
+static
+int virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain,
+ unsigned int flags)
+{
+ int feat;
+
+ 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)
+ feat = VIR_DRV_FEATURE_MIGRATION_P2P;
+ else
+ feat = VIR_DRV_FEATURE_MIGRATION_DIRECT;
+
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, feat)) {
+ virReportUnsupportedError();
+ return -1;
+ }
+
+ return 0;
+}
+
+
/**
* virDomainMigrateToURI:
* @domain: a domain object
@@ -4209,6 +4242,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);
@@ -4220,46 +4256,17 @@ virDomainMigrateToURI(virDomainPtr domain,
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;
--
1.7.1
9:38 p.m.
New subject: [libvirt] [PATCH v3 11/14] migration: refactor: introduce parameter checking function
On Fri, Sep 18, 2015 at 18:05:49 +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.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 83 +++++++++++++++++++++++++++-----------------------
1 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5c22460..eec45bd 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4131,6 +4131,39 @@ virDomainMigrate3(virDomainPtr domain,
}
+static
+int virDomainMigrateUnmanagedCheckCompat(virDomainPtr domain,
+ unsigned int flags)
+{
+ int feat;
+
+ 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)
+ feat = VIR_DRV_FEATURE_MIGRATION_P2P;
+ else
+ feat = VIR_DRV_FEATURE_MIGRATION_DIRECT;
+
+ if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, feat)) {
+ virReportUnsupportedError();
Use virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, ...),
virReportUnsupportedError is for unsupported APIs while
VIR_ERR_ARGUMENT_UNSUPPORTED says the API is supported but some of its
arguments are not. While at it, I think it's better to keep the code a
bit verbose and separate checks for p2p and direct migration to be able
to provide a usable error message.
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 12/14] migration: reuse parameters check in toURI2 and toURI3
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 | 75 ++++++++++----------------------------------------
1 files changed, 15 insertions(+), 60 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index eec45bd..fc61830 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4253,7 +4253,6 @@ virDomainMigrateToURI(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
-
virCheckNonNullArgGoto(duri, error);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
@@ -4382,37 +4381,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;
@@ -4476,37 +4453,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
9:38 p.m.
New subject: [libvirt] [PATCH v3 12/14] migration: reuse parameters check in toURI2 and toURI3
On Fri, Sep 18, 2015 at 18:05:50 +0300, Nikolay Shirokovskiy wrote:
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 | 75 ++++++++++----------------------------------------
1 files changed, 15 insertions(+), 60 deletions(-)
I think you could squash this patch into the previous one which does
similar thing in *ToURI.
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index eec45bd..fc61830 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4253,7 +4253,6 @@ virDomainMigrateToURI(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
-
virCheckNonNullArgGoto(duri, error);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
And this hunk definitely doesn't belong to this patch... one more reason
to squash patch 12 and 11 :-)
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 13/14] migration: refactor: one return in forURI family functions
May be a matter of a taste but this version with one return point in every
function looks simplier to understand and to extend too. Anyway after such
a heavy refactoring a little cleanup will not hurt.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 61 +++++++++++++++++++++++---------------------------
1 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fc61830..f012f4b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4242,6 +4242,7 @@ virDomainMigrateToURI(virDomainPtr domain,
const char *dname,
unsigned long bandwidth)
{
+ int ret = -1;
const char *dconnuri = NULL;
const char *miguri = NULL;
@@ -4252,26 +4253,24 @@ virDomainMigrateToURI(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
- virCheckReadOnlyGoto(domain->conn->flags, error);
- virCheckNonNullArgGoto(duri, error);
+ virCheckReadOnlyGoto(domain->conn->flags, cleanup);
+ virCheckNonNullArgGoto(duri, cleanup);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
- goto error;
+ goto cleanup;
if (flags & VIR_MIGRATE_PEER2PEER)
dconnuri = duri;
else
miguri = duri;
- if (virDomainMigrateUnmanaged(domain, NULL, flags,
- dname, dconnuri, miguri, bandwidth) < 0)
- goto error;
-
- return 0;
+ ret = virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, dconnuri, miguri, bandwidth);
+ cleanup:
+ if (ret < 0)
+ virDispatchError(domain->conn);
- error:
- virDispatchError(domain->conn);
- return -1;
+ return ret;
}
@@ -4370,6 +4369,7 @@ virDomainMigrateToURI2(virDomainPtr domain,
const char *dname,
unsigned long bandwidth)
{
+ int ret = -1;
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, miguri=%s, dxml=%s, "
"flags=%lx, dname=%s, bandwidth=%lu",
NULLSTR(dconnuri), NULLSTR(miguri), NULLSTR(dxml),
@@ -4379,23 +4379,20 @@ virDomainMigrateToURI2(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
- virCheckReadOnlyGoto(domain->conn->flags, error);
+ virCheckReadOnlyGoto(domain->conn->flags, cleanup);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
- goto error;
+ goto cleanup;
if (!(flags & VIR_MIGRATE_PEER2PEER))
dconnuri = NULL;
- if (virDomainMigrateUnmanaged(domain, NULL, flags,
- dname, dconnuri, miguri, bandwidth) < 0)
- goto error;
-
- return 0;
-
- error:
- virDispatchError(domain->conn);
- return -1;
+ ret = virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, dconnuri, miguri, bandwidth);
+ cleanup:
+ if (ret < 0)
+ virDispatchError(domain->conn);
+ return ret;
}
@@ -4443,6 +4440,7 @@ virDomainMigrateToURI3(virDomainPtr domain,
unsigned int nparams,
unsigned int flags)
{
+ int ret = -1;
VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparms=%u flags=%x",
NULLSTR(dconnuri), params, nparams, flags);
VIR_TYPED_PARAMS_DEBUG(params, nparams);
@@ -4451,23 +4449,20 @@ virDomainMigrateToURI3(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
- virCheckReadOnlyGoto(domain->conn->flags, error);
+ virCheckReadOnlyGoto(domain->conn->flags, cleanup);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
- goto error;
+ goto cleanup;
if (!(flags & VIR_MIGRATE_PEER2PEER))
dconnuri = NULL;
- if (virDomainMigrateUnmanagedParams(domain, dconnuri,
- params, nparams, flags) < -1)
- goto error;
-
- return 0;
-
- error:
- virDispatchError(domain->conn);
- return -1;
+ ret = virDomainMigrateUnmanagedParams(domain, dconnuri,
+ params, nparams, flags);
+ cleanup:
+ if (ret < 0)
+ virDispatchError(domain->conn);
+ return ret;
}
--
1.7.1
9:46 p.m.
New subject: [libvirt] [PATCH v3 13/14] migration: refactor: one return in forURI family functions
On Fri, Sep 18, 2015 at 18:05:51 +0300, Nikolay Shirokovskiy wrote:
May be a matter of a taste but this version with one return point in
every
function looks simplier to understand and to extend too. Anyway after such
a heavy refactoring a little cleanup will not hurt.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 61 +++++++++++++++++++++++---------------------------
1 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fc61830..f012f4b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4242,6 +4242,7 @@ virDomainMigrateToURI(virDomainPtr domain,
const char *dname,
unsigned long bandwidth)
{
+ int ret = -1;
const char *dconnuri = NULL;
const char *miguri = NULL;
@@ -4252,26 +4253,24 @@ virDomainMigrateToURI(virDomainPtr domain,
/* First checkout the source */
virCheckDomainReturn(domain, -1);
- virCheckReadOnlyGoto(domain->conn->flags, error);
- virCheckNonNullArgGoto(duri, error);
+ virCheckReadOnlyGoto(domain->conn->flags, cleanup);
+ virCheckNonNullArgGoto(duri, cleanup);
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
- goto error;
+ goto cleanup;
if (flags & VIR_MIGRATE_PEER2PEER)
dconnuri = duri;
else
miguri = duri;
- if (virDomainMigrateUnmanaged(domain, NULL, flags,
- dname, dconnuri, miguri, bandwidth) < 0)
- goto error;
-
- return 0;
+ ret = virDomainMigrateUnmanaged(domain, NULL, flags,
+ dname, dconnuri, miguri, bandwidth);
+ cleanup:
+ if (ret < 0)
+ virDispatchError(domain->conn);
- error:
- virDispatchError(domain->conn);
- return -1;
+ return ret;
}
Well, we tend to use cleanup labels when there's some common cleanup to
be done for both successful and error case, which is not the case here.
And by applying this patch, all *ToURI* functions will be quite
different from any other function in this file. That said, I think you
could just drop this patch :-)
Jirka
11:05 p.m.
New subject: [libvirt] [PATCH v3 14/14] 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 f012f4b..380f374 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4384,7 +4384,9 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto cleanup;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, cleanup);
+ else
dconnuri = NULL;
ret = virDomainMigrateUnmanaged(domain, NULL, flags,
@@ -4454,7 +4456,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto cleanup;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, cleanup);
+ else
dconnuri = NULL;
ret = virDomainMigrateUnmanagedParams(domain, dconnuri,
--
1.7.1
9:47 p.m.
New subject: [libvirt] [PATCH v3 14/14] migration: check dconnuri in p2p mode
On Fri, Sep 18, 2015 at 18:05:52 +0300, Nikolay Shirokovskiy wrote:
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 f012f4b..380f374 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4384,7 +4384,9 @@ virDomainMigrateToURI2(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto cleanup;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, cleanup);
+ else
dconnuri = NULL;
ret = virDomainMigrateUnmanaged(domain, NULL, flags,
@@ -4454,7 +4456,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
if (virDomainMigrateUnmanagedCheckCompat(domain, flags) < 0)
goto cleanup;
- if (!(flags & VIR_MIGRATE_PEER2PEER))
+ if (flags & VIR_MIGRATE_PEER2PEER)
+ virCheckNonNullArgGoto(dconnuri, cleanup);
+ else
dconnuri = NULL;
ret = virDomainMigrateUnmanagedParams(domain, dconnuri,
ACK, although you may need to update the labels after dropping the
previous patch.
Jirka
3264
days inactive
3277
days old
35 comments
3 participants
participants (3)
-
Jiri Denemark -
Nikolay Shirokovskiy -
Nikolay Shirokovskiy