
On 31.08.2015 10:42, Nikolay Shirokovskiy wrote:
On 28.08.2015 19:04, Michal Privoznik wrote:
On 28.08.2015 11:29, Nikolay Shirokovskiy wrote:
On 28.08.2015 08:54, Michal Privoznik wrote:
On 27.08.2015 12:23, Nikolay Shirokovskiy wrote:
From: Nikolay Shirokovskiy <Nikolay Shirokovskiy nshirokovskiy@virtuozzo.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.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/libvirt-domain.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6ab50ba..c89775b 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3427,7 +3427,8 @@ virDomainMigrateDirect(virDomainPtr domain, NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(dconnuri), NULLSTR(miguri), bandwidth);
- if (!domain->conn->driver->domainMigratePerform) { + if (!domain->conn->driver->domainMigratePerform && + !domain->conn->driver->domainMigratePerform3) { virReportUnsupportedError(); return -1; }
Hm.. domainMigratePerform3 will be used iff connection driver has VIR_DRV_FEATURE_MIGRATION_V3 feature. But this check will require that regardless. What if we check the presence of implementation with respect to that? I see you mean actual driver could be behind remote one and checking for perform3 always gives true so we need to check for feature instead?
Sort of. domainMigratePerform3 is used only if the feature is present. But after your patch, the function implementation would be needed always, even if the feature is not present.
Why? You can implement version1 or version3 or both for this check to pass.
Unfortunately no. I mean, the current code does not have any fallback. Whenever the feature is detected, domainMigratePerform3 is used and that's it. Therefore I think we are aiming on something among the following lines: diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbf08fc..218efe8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3425,16 +3425,16 @@ 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 +3450,11 @@ 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", The other approach could be: diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbf08fc..9d4827a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3420,12 +3420,18 @@ virDomainMigrateDirect(virDomainPtr domain, const char *uri, unsigned long bandwidth) { + bool v3migration; + VIR_DOMAIN_DEBUG(domain, "xmlin=%s, flags=%lx, dname=%s, uri=%s, bandwidth=%lu", NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(uri), bandwidth); - if (!domain->conn->driver->domainMigratePerform) { + v3migration = VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_MIGRATION_V3); + + if ((!v3migration && !domain->conn->driver->domainMigratePerform) || + (v3migration && !domain->conn->driver->domainMigratePerform3)) { virReportUnsupportedError(); return -1; } @@ -3433,8 +3439,7 @@ virDomainMigrateDirect(virDomainPtr domain, /* 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 (v3migration) { VIR_DEBUG("Using migration protocol 3"); /* dconn URI not relevant in direct migration, since no * target libvirtd is involved */ Michal