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

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2019-09-24-v2' into staging 



nbd patches for 2019-09-24

- Improved error message for plaintext client of encrypted server
- Fix various assertions when -object iothread is in use
- Silence a Coverity error for use-after-free on error path

# gpg: Signature made Wed 25 Sep 2019 14:35:52 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2019-09-24-v2:
  util/qemu-sockets: fix keep_alive handling in inet_connect_saddr
  tests: Use iothreads during iotest 223
  nbd: Grab aio context lock in more places
  nbd/server: attach client channel to the export's AioContext
  nbd/client: Add hint when TLS is missing

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

blockdev-nbd.c

+12 −2
Original line number Diff line number Diff line
@@ -151,6 +151,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
    BlockBackend *on_eject_blk;

    NBDExport *exp;

    int64_t len;

    AioContext *aio_context;



    if (!nbd_server) {

        error_setg(errp, "NBD server not running");
@@ -173,11 +174,13 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
        return;

    }



    aio_context = bdrv_get_aio_context(bs);

    aio_context_acquire(aio_context);

    len = bdrv_getlength(bs);

    if (len < 0) {

        error_setg_errno(errp, -len,

                         "Failed to determine the NBD export's length");

        return;

        goto out;

    }



    if (!has_writable) {
@@ -190,13 +193,16 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, 
    exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,

                         NULL, false, on_eject_blk, errp);

    if (!exp) {

        return;

        goto out;

    }



    /* The list of named exports has a strong reference to this export now and

     * our only way of accessing it is through nbd_export_find(), so we can drop

     * the strong reference that is @exp. */

    nbd_export_put(exp);



 out:

    aio_context_release(aio_context);

}



void qmp_nbd_server_remove(const char *name,
@@ -204,6 +210,7 @@ void qmp_nbd_server_remove(const char *name, 
                           Error **errp)

{

    NBDExport *exp;

    AioContext *aio_context;



    if (!nbd_server) {

        error_setg(errp, "NBD server not running");
@@ -220,7 +227,10 @@ void qmp_nbd_server_remove(const char *name, 
        mode = NBD_SERVER_REMOVE_MODE_SAFE;

    }



    aio_context = nbd_export_aio_context(exp);

    aio_context_acquire(aio_context);

    nbd_export_remove(exp, mode, errp);

    aio_context_release(aio_context);

}



void qmp_nbd_server_stop(Error **errp)

include/block/nbd.h

+1 −0
Original line number Diff line number Diff line
@@ -340,6 +340,7 @@ void nbd_export_put(NBDExport *exp); 


BlockBackend *nbd_export_get_blockdev(NBDExport *exp);



AioContext *nbd_export_aio_context(NBDExport *exp);

NBDExport *nbd_export_find(const char *name);

void nbd_export_close_all(void);

nbd/client.c

+1 −0
Original line number Diff line number Diff line
@@ -204,6 +204,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, NBDOptionReply *reply, 
    case NBD_REP_ERR_TLS_REQD:

        error_setg(errp, "TLS negotiation required before option %" PRIu32

                   " (%s)", reply->option, nbd_opt_lookup(reply->option));

        error_append_hint(errp, "Did you forget a valid tls-creds?\n");

        break;



    case NBD_REP_ERR_UNKNOWN:

nbd/server.c

+23 −4
Original line number Diff line number Diff line
@@ -1297,6 +1297,11 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp) 
        return ret;

    }



    /* Attach the channel to the same AioContext as the export */

    if (client->exp && client->exp->ctx) {

        qio_channel_attach_aio_context(client->ioc, client->exp->ctx);

    }



    assert(!client->optlen);

    trace_nbd_negotiate_success();
@@ -1456,7 +1461,12 @@ static void blk_aio_detach(void *opaque) 
static void nbd_eject_notifier(Notifier *n, void *data)

{

    NBDExport *exp = container_of(n, NBDExport, eject_notifier);

    AioContext *aio_context;



    aio_context = exp->ctx;

    aio_context_acquire(aio_context);

    nbd_export_close(exp);

    aio_context_release(aio_context);

}



NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
@@ -1475,12 +1485,11 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset, 
     * NBD exports are used for non-shared storage migration.  Make sure

     * that BDRV_O_INACTIVE is cleared and the image is ready for write

     * access since the export could be available before migration handover.

     * ctx was acquired in the caller.

     */

    assert(name);

    ctx = bdrv_get_aio_context(bs);

    aio_context_acquire(ctx);

    bdrv_invalidate_cache(bs, NULL);

    aio_context_release(ctx);



    /* Don't allow resize while the NBD server is running, otherwise we don't

     * care what happens with the node. */
@@ -1488,7 +1497,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset, 
    if (!readonly) {

        perm |= BLK_PERM_WRITE;

    }

    blk = blk_new(bdrv_get_aio_context(bs), perm,

    blk = blk_new(ctx, perm,

                  BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |

                  BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD);

    ret = blk_insert_bs(blk, bs, errp);
@@ -1555,7 +1564,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset, 
    }



    exp->close = close;

    exp->ctx = blk_get_aio_context(blk);

    exp->ctx = ctx;

    blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);



    if (on_eject_blk) {
@@ -1588,6 +1597,12 @@ NBDExport *nbd_export_find(const char *name) 
    return NULL;

}



AioContext *

nbd_export_aio_context(NBDExport *exp)

{

    return exp->ctx;

}



void nbd_export_close(NBDExport *exp)

{

    NBDClient *client, *next;
@@ -1682,9 +1697,13 @@ BlockBackend *nbd_export_get_blockdev(NBDExport *exp) 
void nbd_export_close_all(void)

{

    NBDExport *exp, *next;

    AioContext *aio_context;



    QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {

        aio_context = exp->ctx;

        aio_context_acquire(aio_context);

        nbd_export_close(exp);

        aio_context_release(aio_context);

    }

}

tests/qemu-iotests/223

+4 −2
Original line number Diff line number Diff line
@@ -2,7 +2,7 @@ 
#

# Test reading dirty bitmap over NBD

#

# Copyright (C) 2018 Red Hat, Inc.

# Copyright (C) 2018-2019 Red Hat, Inc.

#

# This program is free software; you can redistribute it and/or modify

# it under the terms of the GNU General Public License as published by
@@ -109,7 +109,7 @@ echo 
echo "=== End dirty bitmaps, and start serving image over NBD ==="

echo



_launch_qemu 2> >(_filter_nbd)

_launch_qemu -object iothread,id=io0 2> >(_filter_nbd)



# Intentionally provoke some errors as well, to check error handling

silent=
@@ -117,6 +117,8 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return" 
_send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",

  "arguments":{"driver":"qcow2", "node-name":"n",

    "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"

_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-blockdev-set-iothread",

  "arguments":{"node-name":"n", "iothread":"io0"}}' "return"

_send_qemu_cmd $QEMU_HANDLE '{"execute":"block-dirty-bitmap-disable",

  "arguments":{"node":"n", "name":"b"}}' "return"

_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",