If a migration is already in progress and somebody attempts to add a migration blocker, this should rightly fail.
Add an errp parameter and a retcode return value to migrate_add_blocker. This is part one of two for a solution to prohibit e.g. block jobs from running concurrently with migration. Signed-off-by: John Snow <js...@redhat.com> --- block/qcow.c | 6 +++++- block/vdi.c | 6 +++++- block/vhdx.c | 6 +++++- block/vmdk.c | 7 ++++++- block/vpc.c | 10 +++++++--- block/vvfat.c | 20 ++++++++++++-------- hw/9pfs/virtio-9p.c | 16 ++++++++++++---- hw/misc/ivshmem.c | 5 ++++- hw/scsi/vhost-scsi.c | 25 +++++++++++++++++++------ hw/virtio/vhost.c | 35 +++++++++++++++++++++++------------ include/migration/migration.h | 6 +++++- migration/migration.c | 32 ++++++++++++++++++++++++++++---- stubs/migr-blocker.c | 3 ++- target-i386/kvm.c | 16 +++++++++++++--- 14 files changed, 146 insertions(+), 47 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 6e35db1..fbb498f 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -236,7 +236,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The qcow format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail; + } qemu_co_mutex_init(&s->lock); return 0; diff --git a/block/vdi.c b/block/vdi.c index 062a654..1635ab1 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -505,7 +505,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The vdi format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail_free_bmap; + } qemu_co_mutex_init(&s->write_lock); diff --git a/block/vhdx.c b/block/vhdx.c index d3bb1bd..097c707 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1005,7 +1005,11 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The vhdx format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail; + } return 0; fail: diff --git a/block/vmdk.c b/block/vmdk.c index be0d640..36ff6f4 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -951,7 +951,12 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The vmdk format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail; + } + g_free(buf); return 0; diff --git a/block/vpc.c b/block/vpc.c index 2b3b518..f3dd677 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -325,13 +325,17 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, #endif } - qemu_co_mutex_init(&s->lock); - /* Disable migration when VHD images are used */ error_setg(&s->migration_blocker, "The vpc format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail; + } + + qemu_co_mutex_init(&s->lock); return 0; diff --git a/block/vvfat.c b/block/vvfat.c index 7ddc962..bdc1297 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1191,22 +1191,26 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count; - if (s->first_sectors_number == 0x40) { - init_mbr(s, cyls, heads, secs); - } - - // assert(is_consistent(s)); - qemu_co_mutex_init(&s->lock); - /* Disable migration when vvfat is used rw */ if (s->qcow) { error_setg(&s->migration_blocker, "The vvfat (rw) format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + error_free(s->migration_blocker); + goto fail; + } } + if (s->first_sectors_number == 0x40) { + init_mbr(s, cyls, heads, secs); + } + + // assert(is_consistent(s)); + qemu_co_mutex_init(&s->lock); + ret = 0; fail: qemu_opts_del(opts); diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index f972731..67dc626 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -978,20 +978,28 @@ static void v9fs_attach(void *opaque) clunk_fid(s, fid); goto out; } - err += offset; - trace_v9fs_attach_return(pdu->tag, pdu->id, - qid.type, qid.version, qid.path); + /* * disable migration if we haven't done already. * attach could get called multiple times for the same export. */ if (!s->migration_blocker) { + int ret; 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); - migrate_add_blocker(s->migration_blocker); + ret = migrate_add_blocker(s->migration_blocker, NULL); + if (ret < 0) { + err = ret; + clunk_fid(s, fid); + goto out; + } } + + err += offset; + trace_v9fs_attach_return(pdu->tag, pdu->id, + qid.type, qid.version, qid.path); out: put_fid(pdu, fidp); out_nofid: diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index cc76989..859e844 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -740,7 +740,10 @@ static int pci_ivshmem_init(PCIDevice *dev) if (s->role_val == IVSHMEM_PEER) { error_setg(&s->migration_blocker, "Migration is disabled when using feature 'peer mode' in device 'ivshmem'"); - migrate_add_blocker(s->migration_blocker); + if (migrate_add_blocker(s->migration_blocker, NULL) < 0) { + error_report("Unable to prohibit migration during ivshmem init"); + exit(1); + } } pci_conf = dev->config; diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index fb7983d..46ab197 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -239,8 +239,14 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); - close(vhostfd); - return; + goto close_fd; + } + + 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; } s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; @@ -253,7 +259,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) if (ret < 0) { error_setg(errp, "vhost-scsi: vhost initialization failed: %s", strerror(-ret)); - return; + goto free_vqs; } /* At present, channel and lun both are 0 for bootable vhost-scsi disk */ @@ -262,9 +268,16 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) /* Note: we can also get the minimum tpgt from kernel */ s->target = vs->conf.boot_tpgt; - error_setg(&s->migration_blocker, - "vhost-scsi does not support migration"); - migrate_add_blocker(s->migration_blocker); + return; + + 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; } static void vhost_scsi_unrealize(DeviceState *dev, Error **errp) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index c0ed5b2..0de7a01 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -926,13 +926,25 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, goto fail; } - for (i = 0; i < hdev->nvqs; ++i) { - r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i); - if (r < 0) { - goto fail_vq; - } - } hdev->features = features; + hdev->migration_blocker = NULL; + + if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) { + error_setg(&hdev->migration_blocker, + "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature."); + r = migrate_add_blocker(hdev->migration_blocker, NULL); + if (r < 0) { + errno = -r; + goto fail_mig; + } + } + + for (i = 0; i < hdev->nvqs; ++i) { + r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i); + if (r < 0) { + goto fail_vq; + } + } hdev->memory_listener = (MemoryListener) { .begin = vhost_begin, @@ -949,12 +961,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, .eventfd_del = vhost_eventfd_del, .priority = 10 }; - hdev->migration_blocker = NULL; - if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) { - error_setg(&hdev->migration_blocker, - "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature."); - migrate_add_blocker(hdev->migration_blocker); - } + hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions)); hdev->n_mem_sections = 0; hdev->mem_sections = NULL; @@ -965,10 +972,14 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev->memory_changed = false; memory_listener_register(&hdev->memory_listener, &address_space_memory); return 0; + fail_vq: while (--i >= 0) { vhost_virtqueue_cleanup(hdev->vqs + i); } + migrate_del_blocker(hdev->migration_blocker); +fail_mig: + error_free(hdev->migration_blocker); fail: r = -errno; hdev->vhost_ops->vhost_backend_cleanup(hdev); diff --git a/include/migration/migration.h b/include/migration/migration.h index 8334621..fc18956 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -117,6 +117,7 @@ int migrate_fd_close(MigrationState *s); void add_migration_state_change_notifier(Notifier *notify); void remove_migration_state_change_notifier(Notifier *notify); bool migration_in_setup(MigrationState *); +bool migration_has_started(MigrationState *s); bool migration_has_finished(MigrationState *); bool migration_has_failed(MigrationState *); MigrationState *migrate_get_current(void); @@ -150,8 +151,11 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size); * @migrate_add_blocker - prevent migration from proceeding * * @reason - an error to be returned whenever migration is attempted + * @errp - [out] The reason (if any) we cannot block migration right now. + * + * @returns - 0 on success, -EBUSY on failure, with errp set. */ -void migrate_add_blocker(Error *reason); +int migrate_add_blocker(Error *reason, Error **errp); /** * @migrate_del_blocker - remove a blocking error from migration diff --git a/migration/migration.c b/migration/migration.c index e48dd13..897ae40 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -632,6 +632,26 @@ bool migration_has_failed(MigrationState *s) s->state == MIGRATION_STATUS_FAILED); } +bool migration_has_started(MigrationState *s) +{ + if (!s) { + s = migrate_get_current(); + } + + switch (s->state) { + case MIGRATION_STATUS_NONE: + case MIGRATION_STATUS_CANCELLED: + case MIGRATION_STATUS_COMPLETED: + case MIGRATION_STATUS_FAILED: + return false; + case MIGRATION_STATUS_SETUP: + case MIGRATION_STATUS_CANCELLING: + case MIGRATION_STATUS_ACTIVE: + default: + return true; + } +} + static MigrationState *migrate_init(const MigrationParams *params) { MigrationState *s = migrate_get_current(); @@ -667,9 +687,15 @@ static MigrationState *migrate_init(const MigrationParams *params) static GSList *migration_blockers; -void migrate_add_blocker(Error *reason) +int migrate_add_blocker(Error *reason, Error **errp) { + if (migration_has_started(NULL)) { + error_setg(errp, "Cannot block a migration already in-progress"); + return -EBUSY; + } + migration_blockers = g_slist_prepend(migration_blockers, reason); + return 0; } void migrate_del_blocker(Error *reason) @@ -712,9 +738,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, params.blk = has_blk && blk; params.shared = has_inc && inc; - if (s->state == MIGRATION_STATUS_ACTIVE || - s->state == MIGRATION_STATUS_SETUP || - s->state == MIGRATION_STATUS_CANCELLING) { + if (migration_has_started(s)) { error_setg(errp, QERR_MIGRATION_ACTIVE); return; } diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c index 300df6e..06812bd 100644 --- a/stubs/migr-blocker.c +++ b/stubs/migr-blocker.c @@ -1,8 +1,9 @@ #include "qemu-common.h" #include "migration/migration.h" -void migrate_add_blocker(Error *reason) +int migrate_add_blocker(Error *reason, Error **errp) { + return 0; } void migrate_del_blocker(Error *reason) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 7b0ba17..f45baa5 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -731,7 +731,11 @@ int kvm_arch_init_vcpu(CPUState *cs) error_setg(&invtsc_mig_blocker, "State blocked by non-migratable CPU device" " (invtsc flag)"); - migrate_add_blocker(invtsc_mig_blocker); + r = migrate_add_blocker(invtsc_mig_blocker, NULL); + if (r < 0) { + fprintf(stderr, "migrate_add_blocker failed\n"); + goto fail_mig; + } /* for savevm */ vmstate_x86_cpu.unmigratable = 1; } @@ -739,7 +743,7 @@ int kvm_arch_init_vcpu(CPUState *cs) cpuid_data.cpuid.padding = 0; r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data); if (r) { - return r; + goto fail_ioctl; } r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL); @@ -747,7 +751,7 @@ int kvm_arch_init_vcpu(CPUState *cs) r = kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz); if (r < 0) { fprintf(stderr, "KVM_SET_TSC_KHZ failed\n"); - return r; + goto fail_ioctl; } } @@ -760,6 +764,12 @@ int kvm_arch_init_vcpu(CPUState *cs) } return 0; + + fail_ioctl: + migrate_del_blocker(invtsc_mig_blocker); + fail_mig: + error_free(invtsc_mig_blocker); + return r; } void kvm_arch_reset_vcpu(X86CPU *cpu) -- 2.4.3