[Qemu-devel] [PATCH v2 4/4] migration: Fail migration blocker for --only-migratble

[Ashijeet Acharya <ashijeetacharya@gmail.com>]

migrate_add_blocker should rightly fail if the '--only-migratable'
option was specified and the device in use should not be able to
perform the action which results in an unmigratable VM.

Make migrate_add_blocker return -EACCES in this case.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
 block/qcow.c                |  7 +++++--
 block/vdi.c                 |  7 +++++--
 block/vhdx.c                |  8 +++++---
 block/vmdk.c                |  7 +++++--
 block/vpc.c                 |  7 +++++--
 block/vvfat.c               |  8 +++++---
 hw/9pfs/9p.c                |  9 +++++++--
 hw/display/virtio-gpu.c     |  9 +++++++--
 hw/intc/arm_gic_kvm.c       |  7 +++++--
 hw/intc/arm_gicv3_its_kvm.c |  5 ++++-
 hw/intc/arm_gicv3_kvm.c     |  5 ++++-
 hw/misc/ivshmem.c           | 10 ++++++++--
 hw/scsi/vhost-scsi.c        | 10 ++++++----
 hw/virtio/vhost.c           |  8 +++++---
 migration/migration.c       |  7 +++++++
 target-i386/kvm.c           |  7 +++++--
 16 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 11526a1..7e5003ac 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -253,8 +253,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        error_free(s->migration_blocker);
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use a node with qcow format as "
+                              "it does not support live migration");
+        }
         goto fail;
     }
 
diff --git a/block/vdi.c b/block/vdi.c
index 2b56f52..df45df4 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -472,8 +472,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        error_free(s->migration_blocker);
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use a node with vdi format as "
+                              "it does not support live migration");
+        }
         goto fail_free_bmap;
     }
 
diff --git a/block/vhdx.c b/block/vhdx.c
index d81941b..118134e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -996,11 +996,13 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        error_free(s->migration_blocker);
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use a node with vhdx format as "
+                              "it does not support live migration");
+        }
         goto fail;
     }
-
     if (flags & BDRV_O_RDWR) {
         ret = vhdx_update_headers(bs, s, false, NULL);
         if (ret < 0) {
diff --git a/block/vmdk.c b/block/vmdk.c
index 4a45a83..cd6a641 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -977,8 +977,11 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        error_free(s->migration_blocker);
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use a node with vmdk format as "
+                              "it does not support live migration");
+        }
         goto fail;
     }
 
diff --git a/block/vpc.c b/block/vpc.c
index 299a8c8..c935962 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -427,8 +427,11 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        error_free(s->migration_blocker);
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use a node with vpc format as "
+                              "it does not support live migration");
+        }
         goto fail;
     }
 
diff --git a/block/vvfat.c b/block/vvfat.c
index 3de3320..f9a5f9b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1192,8 +1192,11 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
                    "does not support live migration",
                    bdrv_get_device_or_node_name(bs));
         ret = migrate_add_blocker(s->migration_blocker, errp);
-        if (ret < 0) {
-            error_free(s->migration_blocker);
+        if (ret) {
+            if (ret == -EACCES) {
+                error_append_hint(errp, "Cannot use a node with vvfat format "
+                                  "as it does not support live migration");
+            }
             goto fail;
         }
     }
@@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
         init_mbr(s, cyls, heads, secs);
     }
 
-    //    assert(is_consistent(s));
     qemu_co_mutex_init(&s->lock);
 
     ret = 0;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e72ef43..01dd67f 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1020,12 +1020,17 @@ static void coroutine_fn v9fs_attach(void *opaque)
      */
     if (!s->migration_blocker) {
         int ret;
+        Error *local_err;
         s->root_fid = fid;
         error_setg(&s->migration_blocker,
                    "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
                    s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
-        ret = migrate_add_blocker(s->migration_blocker, NULL);
-        if (ret < 0) {
+        ret = migrate_add_blocker(s->migration_blocker, &local_err);
+        if (ret) {
+            if (ret == -EACCES) {
+                error_append_hint(&local_err, "Failed to mount VirtFS as it "
+                                  "does not support live migration");
+            }
             err = ret;
             clunk_fid(s, fid);
             goto out;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6e60b8c..1b02f4b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1102,6 +1102,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     VirtIOGPU *g = VIRTIO_GPU(qdev);
     bool have_virgl;
     int i;
+    int ret;
 
     if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
         error_setg(errp, "invalid max_outputs > %d", VIRTIO_GPU_MAX_SCANOUTS);
@@ -1120,8 +1121,12 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 
     if (virtio_gpu_virgl_enabled(g->conf)) {
         error_setg(&g->migration_blocker, "virgl is not yet migratable");
-        if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
-            error_free(g->migration_blocker);
+        ret = migrate_add_blocker(g->migration_blocker, errp);
+        if (ret) {
+            if (ret == -EACCES) {
+                error_append_hint(errp, "Cannot use virgl as it does not "
+                                  "support live migration yet");
+            }
             return;
         }
     }
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 3614daa..b40ad5f 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -514,8 +514,11 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
         error_setg(&s->migration_blocker, "This operating system kernel does "
                                           "not support vGICv2 migration");
         ret = migrate_add_blocker(s->migration_blocker, errp);
-        if (ret < 0) {
-            error_free(s->migration_blocker);
+        if (ret) {
+            if (ret == -EACCES) {
+                error_append_hint(errp, "Failed to realize vGICv2 as its"
+                                  " migrataion is not implemented yet");
+            }
             return;
         }
     }
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index 950a02f..3474642 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -70,7 +70,10 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
     error_setg(&s->migration_blocker, "vITS migration is not implemented");
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
-        error_free(s->migration_blocker);
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Failed to realize vITS as its migration "
+                              "is not implemented yet");
+        }
         return;
     }
 
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 06f6f30..be7b97c 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -118,7 +118,10 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
     error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
-        error_free(s->migration_blocker);
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Failed to realize vGICv3 as its migration"
+                              "is not implemented yet");
+        }
         return;
     }
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 585cc5b..58f27b1 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -837,6 +837,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
 {
     IVShmemState *s = IVSHMEM_COMMON(dev);
     Error *err = NULL;
+    int ret;
     uint8_t *pci_conf;
     uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
         PCI_BASE_ADDRESS_MEM_PREFETCH;
@@ -910,8 +911,13 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
     if (!ivshmem_is_master(s)) {
         error_setg(&s->migration_blocker,
                    "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
-        if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
-            error_free(s->migration_blocker);
+        ret = migrate_add_blocker(s->migration_blocker, errp);
+        if (ret) {
+            if (ret == -EACCES) {
+                error_append_hint(errp, "Cannot use the 'peer mode' feature "
+                                  "in device 'ivshmem' as it does not "
+                                  "support live migration");
+            }
             return;
         }
     }
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ff503d0..0c571f8 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -244,8 +244,12 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     error_setg(&s->migration_blocker,
                "vhost-scsi does not support migration");
     ret = migrate_add_blocker(s->migration_blocker, errp);
-    if (ret < 0) {
-        goto free_blocker;
+    if (ret) {
+        if (ret == -EACCES) {
+            error_append_hint(errp, "Cannot use vhost-scsi as it does not "
+                              "support live migration");
+        }
+        goto close_fd;
     }
 
     s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -272,8 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
  free_vqs:
     migrate_del_blocker(s->migration_blocker);
     g_free(s->dev.vqs);
- free_blocker:
-    error_free(s->migration_blocker);
  close_fd:
     close(vhostfd);
     return;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 416fada..1c4a4f9 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1159,9 +1159,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     if (hdev->migration_blocker != NULL) {
         Error *local_err;
         r = migrate_add_blocker(hdev->migration_blocker, &local_err);
-        if (r < 0) {
-            error_report_err(local_err);
-            error_free(hdev->migration_blocker);
+        if (r) {
+            if (r == -EACCES) {
+                error_append_hint(&local_err, "Cannot use vhost drivers as "
+                                  "it does not support live migration");
+            }
             goto fail_busyloop;
         }
     }
diff --git a/migration/migration.c b/migration/migration.c
index 713e012..ab34328 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1115,9 +1115,16 @@ int migrate_add_blocker(Error *reason, Error **errp)
 {
     if (migration_is_idle(NULL)) {
         migration_blockers = g_slist_prepend(migration_blockers, reason);
+        error_free(reason);
         return 0;
     }
 
+    if (only_migratable) {
+        error_setg(errp, "Failed to add migration blocker: --only-migratable "
+                   "was specified");
+        return -EACCES;
+    }
+
     error_setg(errp, "Cannot block a migration already in-progress");
     return -EBUSY;
 }
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6031a1c..05c8365 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -962,7 +962,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
                    "State blocked by non-migratable CPU device"
                    " (invtsc flag)");
         r = migrate_add_blocker(invtsc_mig_blocker, NULL);
-        if (r < 0) {
+        if (r) {
+            if (r == -EACCES) {
+                error_report("kvm: non-migratable CPU device but"
+                        " --only-migratable was specified");
+            }
             error_report("kvm: migrate_add_blocker failed");
             goto efail;
         }
@@ -1009,7 +1013,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
  fail:
     migrate_del_blocker(invtsc_mig_blocker);
  efail:
-    error_free(invtsc_mig_blocker);
     return r;
 }
 
-- 
2.6.2