Commit f1faf2d5 authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging 



Pull request

v2:
 * Fixed incorrect virtio_blk_data_plane_create() local_err refactoring in
   "hw/block: Use errp directly rather than local_err" that broke virtio-blk
   over virtio-mmio [Peter]

# gpg: Signature made Tue 19 Dec 2017 15:08:14 GMT
# gpg:                using RSA key 0x9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>"
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha/tags/block-pull-request: (23 commits)
  qemu-iotests: add 203 savevm with IOThreads test
  iothread: fix iothread_stop() race condition
  iotests: add VM.add_object()
  blockdev: add x-blockdev-set-iothread force boolean
  docs: mark nested AioContext locking as a legacy API
  block: avoid recursive AioContext acquire in bdrv_inactivate_all()
  virtio-blk: reject configs with logical block size > physical block size
  virtio-blk: make queue size configurable
  qemu-iotests: add 202 external snapshots IOThread test
  blockdev: add x-blockdev-set-iothread testing command
  iothread: add iothread_by_id() API
  block: drop unused BlockDirtyBitmapState->aio_context field
  block: don't keep AioContext acquired after internal_snapshot_prepare()
  block: don't keep AioContext acquired after blockdev_backup_prepare()
  block: don't keep AioContext acquired after drive_backup_prepare()
  block: don't keep AioContext acquired after external_snapshot_prepare()
  blockdev: hold AioContext for bdrv_unref() in external_snapshot_clean()
  qdev: drop unused #include "sysemu/iothread.h"
  dev-storage: Fix the unusual function name
  hw/block: Use errp directly rather than local_err
  ...

Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>

# Conflicts:
#	hw/core/qdev-properties-system.c
parents af352675 7a9dda0d

block.c

+11 −3
Original line number Diff line number Diff line
@@ -4320,9 +4320,15 @@ int bdrv_inactivate_all(void) 
    BdrvNextIterator it;

    int ret = 0;

    int pass;

    GSList *aio_ctxs = NULL, *ctx;



    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {

        aio_context_acquire(bdrv_get_aio_context(bs));

        AioContext *aio_context = bdrv_get_aio_context(bs);



        if (!g_slist_find(aio_ctxs, aio_context)) {

            aio_ctxs = g_slist_prepend(aio_ctxs, aio_context);

            aio_context_acquire(aio_context);

        }

    }



    /* We do two passes of inactivation. The first pass calls to drivers'
@@ -4340,9 +4346,11 @@ int bdrv_inactivate_all(void) 
    }



out:

    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {

        aio_context_release(bdrv_get_aio_context(bs));

    for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {

        AioContext *aio_context = ctx->data;

        aio_context_release(aio_context);

    }

    g_slist_free(aio_ctxs);



    return ret;

}

block/null.c

+1 −2
Original line number Diff line number Diff line
@@ -110,8 +110,7 @@ static coroutine_fn int null_co_common(BlockDriverState *bs) 
    BDRVNullState *s = bs->opaque;



    if (s->latency_ns) {

        co_aio_sleep_ns(bdrv_get_aio_context(bs), QEMU_CLOCK_REALTIME,

                        s->latency_ns);

        qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, s->latency_ns);

    }

    return 0;

}

block/sheepdog.c

+1 −2
Original line number Diff line number Diff line
@@ -776,8 +776,7 @@ static coroutine_fn void reconnect_to_sdog(void *opaque) 
        if (s->fd < 0) {

            DPRINTF("Wait for connection to be established\n");

            error_report_err(local_err);

            co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,

                            1000000000ULL);

            qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);

        }

    };

blockdev.c

+189 −70
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ 
#include "qapi/qmp/qerror.h"

#include "qapi/qobject-output-visitor.h"

#include "sysemu/sysemu.h"

#include "sysemu/iothread.h"

#include "block/block_int.h"

#include "qmp-commands.h"

#include "block/trace.h"
@@ -1454,7 +1455,6 @@ struct BlkActionState { 
typedef struct InternalSnapshotState {

    BlkActionState common;

    BlockDriverState *bs;

    AioContext *aio_context;

    QEMUSnapshotInfo sn;

    bool created;

} InternalSnapshotState;
@@ -1485,6 +1485,7 @@ static void internal_snapshot_prepare(BlkActionState *common, 
    qemu_timeval tv;

    BlockdevSnapshotInternal *internal;

    InternalSnapshotState *state;

    AioContext *aio_context;

    int ret1;



    g_assert(common->action->type ==
@@ -1506,32 +1507,33 @@ static void internal_snapshot_prepare(BlkActionState *common, 
        return;

    }



    /* AioContext is released in .clean() */

    state->aio_context = bdrv_get_aio_context(bs);

    aio_context_acquire(state->aio_context);

    aio_context = bdrv_get_aio_context(bs);

    aio_context_acquire(aio_context);



    state->bs = bs;



    /* Paired with .clean() */

    bdrv_drained_begin(bs);



    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) {

        return;

        goto out;

    }



    if (bdrv_is_read_only(bs)) {

        error_setg(errp, "Device '%s' is read only", device);

        return;

        goto out;

    }



    if (!bdrv_can_snapshot(bs)) {

        error_setg(errp, "Block format '%s' used by device '%s' "

                   "does not support internal snapshots",

                   bs->drv->format_name, device);

        return;

        goto out;

    }



    if (!strlen(name)) {

        error_setg(errp, "Name is empty");

        return;

        goto out;

    }



    /* check whether a snapshot with name exist */
@@ -1539,12 +1541,12 @@ static void internal_snapshot_prepare(BlkActionState *common, 
                                            &local_err);

    if (local_err) {

        error_propagate(errp, local_err);

        return;

        goto out;

    } else if (ret) {

        error_setg(errp,

                   "Snapshot with name '%s' already exists on device '%s'",

                   name, device);

        return;

        goto out;

    }



    /* 3. take the snapshot */
@@ -1560,11 +1562,14 @@ static void internal_snapshot_prepare(BlkActionState *common, 
        error_setg_errno(errp, -ret1,

                         "Failed to create snapshot '%s' on device '%s'",

                         name, device);

        return;

        goto out;

    }



    /* 4. succeed, mark a snapshot is created */

    state->created = true;



out:

    aio_context_release(aio_context);

}



static void internal_snapshot_abort(BlkActionState *common)
@@ -1573,12 +1578,16 @@ static void internal_snapshot_abort(BlkActionState *common) 
                             DO_UPCAST(InternalSnapshotState, common, common);

    BlockDriverState *bs = state->bs;

    QEMUSnapshotInfo *sn = &state->sn;

    AioContext *aio_context;

    Error *local_error = NULL;



    if (!state->created) {

        return;

    }



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {

        error_reportf_err(local_error,

                          "Failed to delete snapshot with id '%s' and "
@@ -1586,19 +1595,26 @@ static void internal_snapshot_abort(BlkActionState *common) 
                          sn->id_str, sn->name,

                          bdrv_get_device_name(bs));

    }



    aio_context_release(aio_context);

}



static void internal_snapshot_clean(BlkActionState *common)

{

    InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState,

                                             common, common);

    AioContext *aio_context;



    if (state->aio_context) {

        if (state->bs) {

            bdrv_drained_end(state->bs);

        }

        aio_context_release(state->aio_context);

    if (!state->bs) {

        return;

    }



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    bdrv_drained_end(state->bs);



    aio_context_release(aio_context);

}



/* external snapshot private data */
@@ -1606,7 +1622,6 @@ typedef struct ExternalSnapshotState { 
    BlkActionState common;

    BlockDriverState *old_bs;

    BlockDriverState *new_bs;

    AioContext *aio_context;

    bool overlay_appended;

} ExternalSnapshotState;
@@ -1626,6 +1641,7 @@ static void external_snapshot_prepare(BlkActionState *common, 
    ExternalSnapshotState *state =

                             DO_UPCAST(ExternalSnapshotState, common, common);

    TransactionAction *action = common->action;

    AioContext *aio_context;



    /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar

     * purpose but a different set of parameters */
@@ -1662,31 +1678,32 @@ static void external_snapshot_prepare(BlkActionState *common, 
        return;

    }



    /* Acquire AioContext now so any threads operating on old_bs stop */

    state->aio_context = bdrv_get_aio_context(state->old_bs);

    aio_context_acquire(state->aio_context);

    aio_context = bdrv_get_aio_context(state->old_bs);

    aio_context_acquire(aio_context);



    /* Paired with .clean() */

    bdrv_drained_begin(state->old_bs);



    if (!bdrv_is_inserted(state->old_bs)) {

        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);

        return;

        goto out;

    }



    if (bdrv_op_is_blocked(state->old_bs,

                           BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {

        return;

        goto out;

    }



    if (!bdrv_is_read_only(state->old_bs)) {

        if (bdrv_flush(state->old_bs)) {

            error_setg(errp, QERR_IO_ERROR);

            return;

            goto out;

        }

    }



    if (!bdrv_is_first_non_filter(state->old_bs)) {

        error_setg(errp, QERR_FEATURE_DISABLED, "snapshot");

        return;

        goto out;

    }



    if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
@@ -1698,13 +1715,13 @@ static void external_snapshot_prepare(BlkActionState *common, 


        if (node_name && !snapshot_node_name) {

            error_setg(errp, "New snapshot node name missing");

            return;

            goto out;

        }



        if (snapshot_node_name &&

            bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {

            error_setg(errp, "New snapshot node name already in use");

            return;

            goto out;

        }



        flags = state->old_bs->open_flags;
@@ -1717,7 +1734,7 @@ static void external_snapshot_prepare(BlkActionState *common, 
            int64_t size = bdrv_getlength(state->old_bs);

            if (size < 0) {

                error_setg_errno(errp, -size, "bdrv_getlength failed");

                return;

                goto out;

            }

            bdrv_img_create(new_image_file, format,

                            state->old_bs->filename,
@@ -1725,7 +1742,7 @@ static void external_snapshot_prepare(BlkActionState *common, 
                            NULL, size, flags, false, &local_err);

            if (local_err) {

                error_propagate(errp, local_err);

                return;

                goto out;

            }

        }
@@ -1740,30 +1757,30 @@ static void external_snapshot_prepare(BlkActionState *common, 
                              errp);

    /* We will manually add the backing_hd field to the bs later */

    if (!state->new_bs) {

        return;

        goto out;

    }



    if (bdrv_has_blk(state->new_bs)) {

        error_setg(errp, "The snapshot is already in use");

        return;

        goto out;

    }



    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,

                           errp)) {

        return;

        goto out;

    }



    if (state->new_bs->backing != NULL) {

        error_setg(errp, "The snapshot already has a backing image");

        return;

        goto out;

    }



    if (!state->new_bs->drv->supports_backing) {

        error_setg(errp, "The snapshot does not support backing images");

        return;

        goto out;

    }



    bdrv_set_aio_context(state->new_bs, state->aio_context);

    bdrv_set_aio_context(state->new_bs, aio_context);



    /* This removes our old bs and adds the new bs. This is an operation that

     * can fail, so we need to do it in .prepare; undoing it for abort is
@@ -1772,15 +1789,22 @@ static void external_snapshot_prepare(BlkActionState *common, 
    bdrv_append(state->new_bs, state->old_bs, &local_err);

    if (local_err) {

        error_propagate(errp, local_err);

        return;

        goto out;

    }

    state->overlay_appended = true;



out:

    aio_context_release(aio_context);

}



static void external_snapshot_commit(BlkActionState *common)

{

    ExternalSnapshotState *state =

                             DO_UPCAST(ExternalSnapshotState, common, common);

    AioContext *aio_context;



    aio_context = bdrv_get_aio_context(state->old_bs);

    aio_context_acquire(aio_context);



    /* We don't need (or want) to use the transactional

     * bdrv_reopen_multiple() across all the entries at once, because we
@@ -1789,6 +1813,8 @@ static void external_snapshot_commit(BlkActionState *common) 
        bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR,

                    NULL);

    }



    aio_context_release(aio_context);

}



static void external_snapshot_abort(BlkActionState *common)
@@ -1797,11 +1823,18 @@ static void external_snapshot_abort(BlkActionState *common) 
                             DO_UPCAST(ExternalSnapshotState, common, common);

    if (state->new_bs) {

        if (state->overlay_appended) {

            AioContext *aio_context;



            aio_context = bdrv_get_aio_context(state->old_bs);

            aio_context_acquire(aio_context);



            bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()

                                          close state->old_bs; we need it */

            bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);

            bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);

            bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */



            aio_context_release(aio_context);

        }

    }

}
@@ -1810,17 +1843,24 @@ static void external_snapshot_clean(BlkActionState *common) 
{

    ExternalSnapshotState *state =

                             DO_UPCAST(ExternalSnapshotState, common, common);

    if (state->aio_context) {

    AioContext *aio_context;



    if (!state->old_bs) {

        return;

    }



    aio_context = bdrv_get_aio_context(state->old_bs);

    aio_context_acquire(aio_context);



    bdrv_drained_end(state->old_bs);

        aio_context_release(state->aio_context);

    bdrv_unref(state->new_bs);

    }



    aio_context_release(aio_context);

}



typedef struct DriveBackupState {

    BlkActionState common;

    BlockDriverState *bs;

    AioContext *aio_context;

    BlockJob *job;

} DriveBackupState;
@@ -1832,6 +1872,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp) 
    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

    BlockDriverState *bs;

    DriveBackup *backup;

    AioContext *aio_context;

    Error *local_err = NULL;



    assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
@@ -1842,24 +1883,36 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp) 
        return;

    }



    /* AioContext is released in .clean() */

    state->aio_context = bdrv_get_aio_context(bs);

    aio_context_acquire(state->aio_context);

    aio_context = bdrv_get_aio_context(bs);

    aio_context_acquire(aio_context);



    /* Paired with .clean() */

    bdrv_drained_begin(bs);



    state->bs = bs;



    state->job = do_drive_backup(backup, common->block_job_txn, &local_err);

    if (local_err) {

        error_propagate(errp, local_err);

        return;

        goto out;

    }



out:

    aio_context_release(aio_context);

}



static void drive_backup_commit(BlkActionState *common)

{

    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

    AioContext *aio_context;



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    assert(state->job);

    block_job_start(state->job);



    aio_context_release(aio_context);

}



static void drive_backup_abort(BlkActionState *common)
@@ -1867,25 +1920,38 @@ static void drive_backup_abort(BlkActionState *common) 
    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);



    if (state->job) {

        AioContext *aio_context;



        aio_context = bdrv_get_aio_context(state->bs);

        aio_context_acquire(aio_context);



        block_job_cancel_sync(state->job);



        aio_context_release(aio_context);

    }

}



static void drive_backup_clean(BlkActionState *common)

{

    DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

    AioContext *aio_context;



    if (state->aio_context) {

        bdrv_drained_end(state->bs);

        aio_context_release(state->aio_context);

    if (!state->bs) {

        return;

    }



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    bdrv_drained_end(state->bs);



    aio_context_release(aio_context);

}



typedef struct BlockdevBackupState {

    BlkActionState common;

    BlockDriverState *bs;

    BlockJob *job;

    AioContext *aio_context;

} BlockdevBackupState;



static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
@@ -1896,6 +1962,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp) 
    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

    BlockdevBackup *backup;

    BlockDriverState *bs, *target;

    AioContext *aio_context;

    Error *local_err = NULL;



    assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
@@ -1911,29 +1978,39 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp) 
        return;

    }



    /* AioContext is released in .clean() */

    state->aio_context = bdrv_get_aio_context(bs);

    if (state->aio_context != bdrv_get_aio_context(target)) {

        state->aio_context = NULL;

    aio_context = bdrv_get_aio_context(bs);

    if (aio_context != bdrv_get_aio_context(target)) {

        error_setg(errp, "Backup between two IO threads is not implemented");

        return;

    }

    aio_context_acquire(state->aio_context);

    aio_context_acquire(aio_context);

    state->bs = bs;



    /* Paired with .clean() */

    bdrv_drained_begin(state->bs);



    state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);

    if (local_err) {

        error_propagate(errp, local_err);

        return;

        goto out;

    }



out:

    aio_context_release(aio_context);

}



static void blockdev_backup_commit(BlkActionState *common)

{

    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

    AioContext *aio_context;



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    assert(state->job);

    block_job_start(state->job);



    aio_context_release(aio_context);

}



static void blockdev_backup_abort(BlkActionState *common)
@@ -1941,25 +2018,38 @@ static void blockdev_backup_abort(BlkActionState *common) 
    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);



    if (state->job) {

        AioContext *aio_context;



        aio_context = bdrv_get_aio_context(state->bs);

        aio_context_acquire(aio_context);



        block_job_cancel_sync(state->job);



        aio_context_release(aio_context);

    }

}



static void blockdev_backup_clean(BlkActionState *common)

{

    BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

    AioContext *aio_context;



    if (state->aio_context) {

        bdrv_drained_end(state->bs);

        aio_context_release(state->aio_context);

    if (!state->bs) {

        return;

    }



    aio_context = bdrv_get_aio_context(state->bs);

    aio_context_acquire(aio_context);



    bdrv_drained_end(state->bs);



    aio_context_release(aio_context);

}



typedef struct BlockDirtyBitmapState {

    BlkActionState common;

    BdrvDirtyBitmap *bitmap;

    BlockDriverState *bs;

    AioContext *aio_context;

    HBitmap *backup;

    bool prepared;

} BlockDirtyBitmapState;
@@ -2038,7 +2128,6 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common, 
    }



    bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);

    /* AioContext is released in .clean() */

}



static void block_dirty_bitmap_clear_abort(BlkActionState *common)
@@ -2059,16 +2148,6 @@ static void block_dirty_bitmap_clear_commit(BlkActionState *common) 
    hbitmap_free(state->backup);

}



static void block_dirty_bitmap_clear_clean(BlkActionState *common)

{

    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,

                                             common, common);



    if (state->aio_context) {

        aio_context_release(state->aio_context);

    }

}



static void abort_prepare(BlkActionState *common, Error **errp)

{

    error_setg(errp, "Transaction aborted using Abort action");
@@ -2129,7 +2208,6 @@ static const BlkActionOps actions[] = { 
        .prepare = block_dirty_bitmap_clear_prepare,

        .commit = block_dirty_bitmap_clear_commit,

        .abort = block_dirty_bitmap_clear_abort,

        .clean = block_dirty_bitmap_clear_clean,

    }

};
@@ -4052,6 +4130,47 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) 
    return head;

}



void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,

                                 bool has_force, bool force, Error **errp)

{

    AioContext *old_context;

    AioContext *new_context;

    BlockDriverState *bs;



    bs = bdrv_find_node(node_name);

    if (!bs) {

        error_setg(errp, "Cannot find node %s", node_name);

        return;

    }



    /* Protects against accidents. */

    if (!(has_force && force) && bdrv_has_blk(bs)) {

        error_setg(errp, "Node %s is associated with a BlockBackend and could "

                         "be in use (use force=true to override this check)",

                         node_name);

        return;

    }



    if (iothread->type == QTYPE_QSTRING) {

        IOThread *obj = iothread_by_id(iothread->u.s);

        if (!obj) {

            error_setg(errp, "Cannot find iothread %s", iothread->u.s);

            return;

        }



        new_context = iothread_get_aio_context(obj);

    } else {

        new_context = qemu_get_aio_context();

    }



    old_context = bdrv_get_aio_context(bs);

    aio_context_acquire(old_context);



    bdrv_set_aio_context(bs, new_context);



    aio_context_release(old_context);

}



QemuOptsList qemu_common_drive_opts = {

    .name = "drive",

    .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),

docs/devel/multiple-iothreads.txt

+4 −3
Original line number Diff line number Diff line 
Copyright (c) 2014 Red Hat Inc.

Copyright (c) 2014-2017 Red Hat Inc.



This work is licensed under the terms of the GNU GPL, version 2 or later.  See

the COPYING file in the top-level directory.
@@ -92,8 +92,9 @@ aio_context_acquire()/aio_context_release() for mutual exclusion. Once the 
context is acquired no other thread can access it or run event loop iterations

in this AioContext.



aio_context_acquire()/aio_context_release() calls may be nested.  This

means you can call them if you're not sure whether #2 applies.

Legacy code sometimes nests aio_context_acquire()/aio_context_release() calls.

Do not use nesting anymore, it is incompatible with the BDRV_POLL_WHILE() macro

used in the block layer and can lead to hangs.



There is currently no lock ordering rule if a thread needs to acquire multiple

AioContexts simultaneously.  Therefore, it is only safe for code holding the