Re: [PATCH] Report error when both live and offline flags are used for migration.

[Nitesh Konkar <niteshkonkar.libvirt@xxxxxxxxx>]

Hello Jiri,

On Tue, Mar 8, 2016 at 8:31 PM, Jiri Denemark <jdenemar@xxxxxxxxxx> 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.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list