From patchwork Tue Sep 29 20:20:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Snow X-Patchwork-Id: 524624 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 98279140788 for ; Thu, 1 Oct 2015 07:36:54 +1000 (AEST) Received: from localhost ([::1]:33680 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhP3U-0001Ca-Gs for incoming@patchwork.ozlabs.org; Wed, 30 Sep 2015 17:36:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60136) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zh1O6-0008Mk-VY for qemu-devel@nongnu.org; Tue, 29 Sep 2015 16:20:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zh1O5-0001ms-0y for qemu-devel@nongnu.org; Tue, 29 Sep 2015 16:20:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60960) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zh1Nx-0001kl-6L; Tue, 29 Sep 2015 16:20:25 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 27DCE91581; Tue, 29 Sep 2015 20:20:24 +0000 (UTC) Received: from scv.usersys.redhat.com (dhcp-17-163.bos.redhat.com [10.18.17.163]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8TKKMWC013078; Tue, 29 Sep 2015 16:20:23 -0400 From: John Snow To: qemu-devel@nongnu.org Date: Tue, 29 Sep 2015 16:20:21 -0400 Message-Id: <1443558021-6511-1-git-send-email-jsnow@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, qemu-block@nongnu.org, quintela@redhat.com, jcody@redhat.com, dgilbert@redhat.com, amit.shah@redhat.com, John Snow Subject: [Qemu-devel] [PATCH] migration: disallow migrate_add_blocker during migration X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- block/qcow.c | 5 ++++- block/vdi.c | 5 ++++- block/vhdx.c | 5 ++++- block/vmdk.c | 13 +++++++++---- block/vpc.c | 9 ++++++--- block/vvfat.c | 19 +++++++++++-------- hw/9pfs/virtio-9p.c | 15 +++++++++++---- hw/misc/ivshmem.c | 5 ++++- hw/scsi/vhost-scsi.c | 11 +++++++---- hw/virtio/vhost.c | 31 +++++++++++++++++++------------ include/migration/migration.h | 4 +++- migration/migration.c | 32 ++++++++++++++++++++++++++++---- stubs/migr-blocker.c | 3 ++- target-i386/kvm.c | 6 +++++- 14 files changed, 117 insertions(+), 46 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 6e35db1..1b82dec 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -236,7 +236,10 @@ 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) { + goto fail; + } qemu_co_mutex_init(&s->lock); return 0; diff --git a/block/vdi.c b/block/vdi.c index 062a654..95b2690 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -505,7 +505,10 @@ 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) { + goto fail_free_bmap; + } qemu_co_mutex_init(&s->write_lock); diff --git a/block/vhdx.c b/block/vhdx.c index d3bb1bd..5bebe34 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1005,7 +1005,10 @@ 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) { + goto fail; + } return 0; fail: diff --git a/block/vmdk.c b/block/vmdk.c index be0d640..09dcf6b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -943,15 +943,20 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, if (ret) { goto fail; } - s->cid = vmdk_read_cid(bs, 0); - s->parent_cid = vmdk_read_cid(bs, 1); - qemu_co_mutex_init(&s->lock); /* Disable migration when VMDK images are used */ 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) { + goto fail; + } + + s->cid = vmdk_read_cid(bs, 0); + s->parent_cid = vmdk_read_cid(bs, 1); + qemu_co_mutex_init(&s->lock); + g_free(buf); return 0; diff --git a/block/vpc.c b/block/vpc.c index 2b3b518..4c60942 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -325,13 +325,16 @@ 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) { + goto fail; + } + + qemu_co_mutex_init(&s->lock); return 0; diff --git a/block/vvfat.c b/block/vvfat.c index 7ddc962..100f42a 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1191,22 +1191,25 @@ 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) { + 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..4572e18 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -978,20 +978,27 @@ 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) { + 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..5c6d7a2 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -248,6 +248,13 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s->dev.vq_index = 0; s->dev.backend_features = 0; + error_setg(&s->migration_blocker, + "vhost-scsi does not support migration"); + ret = migrate_add_blocker(s->migration_blocker, errp); + if (ret < 0) { + return; + } + ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL); if (ret < 0) { @@ -261,10 +268,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s->lun = 0; /* 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); } static void vhost_scsi_unrealize(DeviceState *dev, Error **errp) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index c0ed5b2..0f27a2d 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -926,13 +926,24 @@ 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) { + goto fail_keep_r; + } + } + + 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 +960,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; @@ -971,6 +977,7 @@ fail_vq: } fail: r = -errno; +fail_keep_r: hdev->vhost_ops->vhost_backend_cleanup(hdev); return r; } diff --git a/include/migration/migration.h b/include/migration/migration.h index 8334621..de7ebb3 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,9 @@ 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. */ -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..a457cfc 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 -EINPROGRESS; + } + 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..d535029 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"); + return r; + } /* for savevm */ vmstate_x86_cpu.unmigratable = 1; }