[libvirt] [PATCH] Report error when both live and offline flags are used for migration.
--- src/libvirt-domain.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..dc11945 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain, error); + if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -3841,6 +3850,15 @@ virDomainMigrate2(virDomainPtr domain, error); + if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -4014,6 +4032,15 @@ virDomainMigrate3(virDomainPtr domain, VIR_MIGRATE_NON_SHARED_INC, error); + if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_PEER2PEER) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " -- 1.8.3.1
Hello All, Any comments on this small patch? Warm Regards, Nitesh Konkar. On Thu, Mar 3, 2016 at 4:38 PM, Nitesh Konkar < niteshkonkar.libvirt@gmail.com> wrote:
--- src/libvirt-domain.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..dc11945 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain, error);
+ if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -3841,6 +3850,15 @@ virDomainMigrate2(virDomainPtr domain, error);
+ if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) { if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -4014,6 +4032,15 @@ virDomainMigrate3(virDomainPtr domain, VIR_MIGRATE_NON_SHARED_INC, error);
+ if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_PEER2PEER) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " -- 1.8.3.1
On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
--- src/libvirt-domain.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..dc11945 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain, error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
I asked you to move the following if () {...} block...
+ if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) {
... here.
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_OFFLINE)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
I wanted to do that and push the patch, but I realized the patch is incomplete (it doesn't cover MigrateToURI* APIs) and there is even a better place to add these checks... Something like the following (untested): diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c index 64cbffa..36a939d 100644 --- i/src/qemu/qemu_migration.c +++ w/src/qemu/qemu_migration.c @@ -3081,6 +3081,12 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, goto cleanup; if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("live and offline migration flags are " + "mutually exclusive")); + goto cleanup; + } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3335,6 +3341,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virNWFilterReadLockFilterUpdates(); if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("live and offline migration flags are " + "mutually exclusive")); + goto cleanup; + } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s",
Hello Jiri, On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
--- src/libvirt-domain.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..dc11945 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain, error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
I asked you to move the following if () {...} block...
Sorry, I missed this out. Will rectify it in the next patch.
+ if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) {
... here.
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
I wanted to do that and push the patch, but I realized the patch is incomplete (it doesn't cover MigrateToURI* APIs) and there is even a better place to add these checks...
Something like the following (untested):
But, changes in qemuMigrationBeginPhase and qemuMigrationPrepareAny would necessitate changes to all drivers domainMigrate* specific implementations right ? I feel what we have is better. What is your opinion on it ? :-)
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c index 64cbffa..36a939d 100644 --- i/src/qemu/qemu_migration.c +++ w/src/qemu/qemu_migration.c @@ -3081,6 +3081,12 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, goto cleanup;
if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("live and offline migration flags are " + "mutually exclusive")); + goto cleanup; + } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3335,6 +3341,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virNWFilterReadLockFilterUpdates();
if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("live and offline migration flags are " + "mutually exclusive")); + goto cleanup; + } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s",
Looking forward to your reply. Warm Regards, Nitesh Konkar.
On Wed, Mar 09, 2016 at 20:41:45 +0530, Nitesh Konkar wrote:
Hello Jiri,
On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Mar 03, 2016 at 06:08:20 -0500, Nitesh Konkar wrote:
--- src/libvirt-domain.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9491845..dc11945 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3617,6 +3617,15 @@ virDomainMigrate(virDomainPtr domain, error);
+ if (flags & VIR_MIGRATE_OFFLINE) {
I asked you to move the following if () {...} block...
Sorry, I missed this out. Will rectify it in the next patch.
+ if (flags & VIR_MIGRATE_LIVE) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Live and offline migration flags are " + "mutually exclusive")); + goto error; + } + } + if (flags & VIR_MIGRATE_OFFLINE) {
... here.
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver,
domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
I wanted to do that and push the patch, but I realized the patch is incomplete (it doesn't cover MigrateToURI* APIs) and there is even a better place to add these checks...
Something like the following (untested):
But, changes in qemuMigrationBeginPhase and qemuMigrationPrepareAny would necessitate changes to all drivers domainMigrate* specific implementations right ?
Yes, but the problem with your approach is that it isn't complete (more APIs would need to be updated) and it's client-side (these high level migration APIs only run on the client). It doesn't seem to be the right place either since these high level APIs use flags only to distinguish what RPC calls they need to make. My approach is better since you only need to add the check to source and destination entry points (even though you need to do it for each driver, but not really) and the checks are next to the other similar checks for flags. Drivers with no support for either of the flags don't need to be updated at all. Jirka
participants (2)
-
Jiri Denemark -
Nitesh Konkar