Commit 475fe457 authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-03-13-v2' into staging



nbd patches for 2018-03-13

- Eric Blake: iotests: Fix stuck NBD process on 33
- Vladimir Sementsov-Ogievskiy: 0/5 nbd server fixing and refactoring before BLOCK_STATUS
- Eric Blake: nbd/server: Honor FUA request on NBD_CMD_TRIM
- Stefan Hajnoczi: 0/2 block: fix nbd-server-stop crash after blockdev-snapshot-sync
- Vladimir Sementsov-Ogievskiy: nbd block status base:allocation

# gpg: Signature made Tue 13 Mar 2018 20:48:37 GMT
# gpg:                using RSA key A7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2018-03-13-v2:
  iotests: new test 209 for NBD BLOCK_STATUS
  iotests: add file_path helper
  iotests.py: tiny refactor: move system imports up
  nbd: BLOCK_STATUS for standard get_block_status function: client part
  block/nbd-client: save first fatal error in nbd_iter_error
  nbd: BLOCK_STATUS for standard get_block_status function: server part
  nbd/server: add nbd_read_opt_name helper
  nbd/server: add nbd_opt_invalid helper
  iotests: add 208 nbd-server + blockdev-snapshot-sync test case
  block: let blk_add/remove_aio_context_notifier() tolerate BDS changes
  nbd/server: Honor FUA request on NBD_CMD_TRIM
  nbd/server: refactor nbd_trip: split out nbd_handle_request
  nbd/server: refactor nbd_trip: cmd_read and generic reply
  nbd/server: fix: check client->closing before sending reply
  nbd/server: fix sparse read
  nbd/server: move nbd_co_send_structured_error up
  iotests: Fix stuck NBD process on 33

Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parents 3788c7b6 65374c1a
Loading
Loading
Loading
Loading
+63 −0
Original line number Diff line number Diff line
@@ -31,6 +31,13 @@

static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb);

typedef struct BlockBackendAioNotifier {
    void (*attached_aio_context)(AioContext *new_context, void *opaque);
    void (*detach_aio_context)(void *opaque);
    void *opaque;
    QLIST_ENTRY(BlockBackendAioNotifier) list;
} BlockBackendAioNotifier;

struct BlockBackend {
    char *name;
    int refcnt;
@@ -69,6 +76,7 @@ struct BlockBackend {
    bool allow_write_beyond_eof;

    NotifierList remove_bs_notifiers, insert_bs_notifiers;
    QLIST_HEAD(, BlockBackendAioNotifier) aio_notifiers;

    int quiesce_counter;
    VMChangeStateEntry *vmsh;
@@ -247,6 +255,36 @@ static int blk_root_inactivate(BdrvChild *child)
    return 0;
}

static void blk_root_attach(BdrvChild *child)
{
    BlockBackend *blk = child->opaque;
    BlockBackendAioNotifier *notifier;

    trace_blk_root_attach(child, blk, child->bs);

    QLIST_FOREACH(notifier, &blk->aio_notifiers, list) {
        bdrv_add_aio_context_notifier(child->bs,
                notifier->attached_aio_context,
                notifier->detach_aio_context,
                notifier->opaque);
    }
}

static void blk_root_detach(BdrvChild *child)
{
    BlockBackend *blk = child->opaque;
    BlockBackendAioNotifier *notifier;

    trace_blk_root_detach(child, blk, child->bs);

    QLIST_FOREACH(notifier, &blk->aio_notifiers, list) {
        bdrv_remove_aio_context_notifier(child->bs,
                notifier->attached_aio_context,
                notifier->detach_aio_context,
                notifier->opaque);
    }
}

static const BdrvChildRole child_root = {
    .inherit_options    = blk_root_inherit_options,

@@ -260,6 +298,9 @@ static const BdrvChildRole child_root = {

    .activate           = blk_root_activate,
    .inactivate         = blk_root_inactivate,

    .attach             = blk_root_attach,
    .detach             = blk_root_detach,
};

/*
@@ -287,6 +328,7 @@ BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm)

    notifier_list_init(&blk->remove_bs_notifiers);
    notifier_list_init(&blk->insert_bs_notifiers);
    QLIST_INIT(&blk->aio_notifiers);

    QTAILQ_INSERT_TAIL(&block_backends, blk, link);
    return blk;
@@ -364,6 +406,7 @@ static void blk_delete(BlockBackend *blk)
    }
    assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
    assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
    assert(QLIST_EMPTY(&blk->aio_notifiers));
    QTAILQ_REMOVE(&block_backends, blk, link);
    drive_info_del(blk->legacy_dinfo);
    block_acct_cleanup(&blk->stats);
@@ -1857,8 +1900,15 @@ void blk_add_aio_context_notifier(BlockBackend *blk,
        void (*attached_aio_context)(AioContext *new_context, void *opaque),
        void (*detach_aio_context)(void *opaque), void *opaque)
{
    BlockBackendAioNotifier *notifier;
    BlockDriverState *bs = blk_bs(blk);

    notifier = g_new(BlockBackendAioNotifier, 1);
    notifier->attached_aio_context = attached_aio_context;
    notifier->detach_aio_context = detach_aio_context;
    notifier->opaque = opaque;
    QLIST_INSERT_HEAD(&blk->aio_notifiers, notifier, list);

    if (bs) {
        bdrv_add_aio_context_notifier(bs, attached_aio_context,
                                      detach_aio_context, opaque);
@@ -1871,12 +1921,25 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
                                     void (*detach_aio_context)(void *),
                                     void *opaque)
{
    BlockBackendAioNotifier *notifier;
    BlockDriverState *bs = blk_bs(blk);

    if (bs) {
        bdrv_remove_aio_context_notifier(bs, attached_aio_context,
                                         detach_aio_context, opaque);
    }

    QLIST_FOREACH(notifier, &blk->aio_notifiers, list) {
        if (notifier->attached_aio_context == attached_aio_context &&
            notifier->detach_aio_context == detach_aio_context &&
            notifier->opaque == opaque) {
            QLIST_REMOVE(notifier, list);
            g_free(notifier);
            return;
        }
    }

    abort();
}

void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify)
+153 −1
Original line number Diff line number Diff line
@@ -228,6 +228,48 @@ static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk,
    return 0;
}

/* nbd_parse_blockstatus_payload
 * support only one extent in reply and only for
 * base:allocation context
 */
static int nbd_parse_blockstatus_payload(NBDClientSession *client,
                                         NBDStructuredReplyChunk *chunk,
                                         uint8_t *payload, uint64_t orig_length,
                                         NBDExtent *extent, Error **errp)
{
    uint32_t context_id;

    if (chunk->length != sizeof(context_id) + sizeof(extent)) {
        error_setg(errp, "Protocol error: invalid payload for "
                         "NBD_REPLY_TYPE_BLOCK_STATUS");
        return -EINVAL;
    }

    context_id = payload_advance32(&payload);
    if (client->info.meta_base_allocation_id != context_id) {
        error_setg(errp, "Protocol error: unexpected context id %d for "
                         "NBD_REPLY_TYPE_BLOCK_STATUS, when negotiated context "
                         "id is %d", context_id,
                         client->info.meta_base_allocation_id);
        return -EINVAL;
    }

    extent->length = payload_advance32(&payload);
    extent->flags = payload_advance32(&payload);

    if (extent->length == 0 ||
        (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,
                                                    client->info.min_block)) ||
        extent->length > orig_length)
    {
        error_setg(errp, "Protocol error: server sent status chunk with "
                   "invalid length");
        return -EINVAL;
    }

    return 0;
}

/* nbd_parse_error_payload
 * on success @errp contains message describing nbd error reply
 */
@@ -481,6 +523,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(

typedef struct NBDReplyChunkIter {
    int ret;
    bool fatal;
    Error *err;
    bool done, only_structured;
} NBDReplyChunkIter;
@@ -490,11 +533,12 @@ static void nbd_iter_error(NBDReplyChunkIter *iter, bool fatal,
{
    assert(ret < 0);

    if (fatal || iter->ret == 0) {
    if ((fatal && !iter->fatal) || iter->ret == 0) {
        if (iter->ret != 0) {
            error_free(iter->err);
            iter->err = NULL;
        }
        iter->fatal = fatal;
        iter->ret = ret;
        error_propagate(&iter->err, *local_err);
    } else {
@@ -640,6 +684,68 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle,
    return iter.ret;
}

static int nbd_co_receive_blockstatus_reply(NBDClientSession *s,
                                            uint64_t handle, uint64_t length,
                                            NBDExtent *extent, Error **errp)
{
    NBDReplyChunkIter iter;
    NBDReply reply;
    void *payload = NULL;
    Error *local_err = NULL;
    bool received = false;

    assert(!extent->length);
    NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply,
                            NULL, &reply, &payload)
    {
        int ret;
        NBDStructuredReplyChunk *chunk = &reply.structured;

        assert(nbd_reply_is_structured(&reply));

        switch (chunk->type) {
        case NBD_REPLY_TYPE_BLOCK_STATUS:
            if (received) {
                s->quit = true;
                error_setg(&local_err, "Several BLOCK_STATUS chunks in reply");
                nbd_iter_error(&iter, true, -EINVAL, &local_err);
            }
            received = true;

            ret = nbd_parse_blockstatus_payload(s, &reply.structured,
                                                payload, length, extent,
                                                &local_err);
            if (ret < 0) {
                s->quit = true;
                nbd_iter_error(&iter, true, ret, &local_err);
            }
            break;
        default:
            if (!nbd_reply_type_is_error(chunk->type)) {
                s->quit = true;
                error_setg(&local_err,
                           "Unexpected reply type: %d (%s) "
                           "for CMD_BLOCK_STATUS",
                           chunk->type, nbd_reply_type_lookup(chunk->type));
                nbd_iter_error(&iter, true, -EINVAL, &local_err);
            }
        }

        g_free(payload);
        payload = NULL;
    }

    if (!extent->length && !iter.err) {
        error_setg(&iter.err,
                   "Server did not reply with any status extents");
        if (!iter.ret) {
            iter.ret = -EIO;
        }
    }
    error_propagate(errp, iter.err);
    return iter.ret;
}

static int nbd_co_request(BlockDriverState *bs, NBDRequest *request,
                          QEMUIOVector *write_qiov)
{
@@ -782,6 +888,51 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
    return nbd_co_request(bs, &request, NULL);
}

int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs,
                                            bool want_zero,
                                            int64_t offset, int64_t bytes,
                                            int64_t *pnum, int64_t *map,
                                            BlockDriverState **file)
{
    int64_t ret;
    NBDExtent extent = { 0 };
    NBDClientSession *client = nbd_get_client_session(bs);
    Error *local_err = NULL;

    NBDRequest request = {
        .type = NBD_CMD_BLOCK_STATUS,
        .from = offset,
        .len = MIN(MIN_NON_ZERO(QEMU_ALIGN_DOWN(INT_MAX,
                                                bs->bl.request_alignment),
                                client->info.max_block), bytes),
        .flags = NBD_CMD_FLAG_REQ_ONE,
    };

    if (!client->info.base_allocation) {
        *pnum = bytes;
        return BDRV_BLOCK_DATA;
    }

    ret = nbd_co_send_request(bs, &request, NULL);
    if (ret < 0) {
        return ret;
    }

    ret = nbd_co_receive_blockstatus_reply(client, request.handle, bytes,
                                           &extent, &local_err);
    if (local_err) {
        error_report_err(local_err);
    }
    if (ret < 0) {
        return ret;
    }

    assert(extent.length);
    *pnum = extent.length;
    return (extent.flags & NBD_STATE_HOLE ? 0 : BDRV_BLOCK_DATA) |
           (extent.flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0);
}

void nbd_client_detach_aio_context(BlockDriverState *bs)
{
    NBDClientSession *client = nbd_get_client_session(bs);
@@ -826,6 +977,7 @@ int nbd_client_init(BlockDriverState *bs,

    client->info.request_sizes = true;
    client->info.structured_reply = true;
    client->info.base_allocation = true;
    ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,
                                tlscreds, hostname,
                                &client->ioc, &client->info, errp);
+6 −0
Original line number Diff line number Diff line
@@ -61,4 +61,10 @@ void nbd_client_detach_aio_context(BlockDriverState *bs);
void nbd_client_attach_aio_context(BlockDriverState *bs,
                                   AioContext *new_context);

int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs,
                                            bool want_zero,
                                            int64_t offset, int64_t bytes,
                                            int64_t *pnum, int64_t *map,
                                            BlockDriverState **file);

#endif /* NBD_CLIENT_H */
+3 −0
Original line number Diff line number Diff line
@@ -585,6 +585,7 @@ static BlockDriver bdrv_nbd = {
    .bdrv_detach_aio_context    = nbd_detach_aio_context,
    .bdrv_attach_aio_context    = nbd_attach_aio_context,
    .bdrv_refresh_filename      = nbd_refresh_filename,
    .bdrv_co_block_status       = nbd_client_co_block_status,
};

static BlockDriver bdrv_nbd_tcp = {
@@ -604,6 +605,7 @@ static BlockDriver bdrv_nbd_tcp = {
    .bdrv_detach_aio_context    = nbd_detach_aio_context,
    .bdrv_attach_aio_context    = nbd_attach_aio_context,
    .bdrv_refresh_filename      = nbd_refresh_filename,
    .bdrv_co_block_status       = nbd_client_co_block_status,
};

static BlockDriver bdrv_nbd_unix = {
@@ -623,6 +625,7 @@ static BlockDriver bdrv_nbd_unix = {
    .bdrv_detach_aio_context    = nbd_detach_aio_context,
    .bdrv_attach_aio_context    = nbd_attach_aio_context,
    .bdrv_refresh_filename      = nbd_refresh_filename,
    .bdrv_co_block_status       = nbd_client_co_block_status,
};

static void bdrv_nbd_init(void)
+2 −0
Original line number Diff line number Diff line
@@ -7,6 +7,8 @@ bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
# block/block-backend.c
blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
blk_root_attach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"

# block/io.c
bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
Loading