9:16 a.m.
$subj: "migration: refactor parameter compatibility checks
On 09/10/2015 09:20 AM, Nikolay Shirokovskiy wrote:
Checks for migration's parameter set and support by protocol are
slightly
s/migration's parameter set/migration parameter sets
scattered in code. Let's put it in one place, namely every
protocol function
should check it's parameter set.
Perhaps more simply said - "Use the common virTypedParamsCheck for each
virDomainMigrate* function to check its expected params list prior to
the API making the driver call. Thi"
Signed-off-by: Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
---
src/libvirt-domain.c | 56 ++++++++++++++++++++-----------------------------
1 files changed, 23 insertions(+), 33 deletions(-)
I think the "one" (perhaps naive) mental note I make is that you add a
virTypedParamsCheck to virDomainMigrateUnmanagedProto3; however, it
seems it's unnecessary for virDomainMigratePeer2PeerParams... Yes, I see
one calls Perform3 and one calls Perform3Params
Still seems sane to me
John
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 55efd49..8b4171e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3345,30 +3345,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",
@@ -3391,11 +3392,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,
@@ -4439,12 +4451,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);
@@ -4459,9 +4465,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)) {
@@ -4477,17 +4480,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,
@@ -4499,13 +4496,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)