Commit 2e75021e authored by Peter Maydell's avatar Peter Maydell
Browse files

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-08-30' into staging 



nbd patches for 2017-08-30

- Kashyap Chamarthy: qemu-iotests: Extend non-shared storage migration test (194)
- Stefan Hajnaczi: 0/3 nbd-client: enter read_reply_co during init to avoid crash
- Vladimir Sementsov-Ogievskiy: portions of 0/17 nbd client refactoring and fixing

# gpg: Signature made Wed 30 Aug 2017 19:03:46 BST
# gpg:                using RSA key 0xA7A16B4A2527436A
# 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-2017-08-30:
  block/nbd-client: refactor request send/receive
  block/nbd-client: rename nbd_recv_coroutines_enter_all
  block/nbd-client: get rid of ssize_t
  nbd/client: fix nbd_send_request to return int
  nbd/client: refactor nbd_receive_reply
  nbd/client: refactor nbd_read_eof
  nbd/client: fix nbd_opt_go
  qemu-iotests: test NBD over UNIX domain sockets in 083
  qemu-iotests: improve nbd-fault-injector.py startup protocol
  nbd-client: avoid read_reply_co entry if send failed
  qemu-iotests: Extend non-shared storage migration test (194)

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

block/nbd-client.c

+37 −65
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ 
#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))

#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))



static void nbd_recv_coroutines_enter_all(NBDClientSession *s)

static void nbd_recv_coroutines_wake_all(NBDClientSession *s)

{

    int i;
@@ -112,7 +112,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque) 
    }



    s->quit = true;

    nbd_recv_coroutines_enter_all(s);

    nbd_recv_coroutines_wake_all(s);

    s->read_reply_co = NULL;

}
@@ -144,12 +144,12 @@ static int nbd_co_send_request(BlockDriverState *bs, 
    request->handle = INDEX_TO_HANDLE(s, i);



    if (s->quit) {

        qemu_co_mutex_unlock(&s->send_mutex);

        return -EIO;

        rc = -EIO;

        goto err;

    }

    if (!s->ioc) {

        qemu_co_mutex_unlock(&s->send_mutex);

        return -EPIPE;

        rc = -EPIPE;

        goto err;

    }



    if (qiov) {
@@ -166,8 +166,13 @@ static int nbd_co_send_request(BlockDriverState *bs, 
    } else {

        rc = nbd_send_request(s->ioc, request);

    }



err:

    if (rc < 0) {

        s->quit = true;

        s->requests[i].coroutine = NULL;

        s->in_flight--;

        qemu_co_queue_next(&s->free_sema);

    }

    qemu_co_mutex_unlock(&s->send_mutex);

    return rc;
@@ -201,13 +206,6 @@ static void nbd_co_receive_reply(NBDClientSession *s, 
        /* Tell the read handler to read another header.  */

        s->reply.handle = 0;

    }

}



static void nbd_coroutine_end(BlockDriverState *bs,

                              NBDRequest *request)

{

    NBDClientSession *s = nbd_get_client_session(bs);

    int i = HANDLE_TO_INDEX(s, request->handle);



    s->requests[i].coroutine = NULL;
@@ -222,29 +220,40 @@ static void nbd_coroutine_end(BlockDriverState *bs, 
    qemu_co_mutex_unlock(&s->send_mutex);

}



static int nbd_co_request(BlockDriverState *bs,

                          NBDRequest *request,

                          QEMUIOVector *qiov)

{

    NBDClientSession *client = nbd_get_client_session(bs);

    NBDReply reply;

    int ret;



    assert(!qiov || request->type == NBD_CMD_WRITE ||

           request->type == NBD_CMD_READ);

    ret = nbd_co_send_request(bs, request,

                              request->type == NBD_CMD_WRITE ? qiov : NULL);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, request, &reply,

                             request->type == NBD_CMD_READ ? qiov : NULL);

    }

    return -reply.error;

}



int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,

                         uint64_t bytes, QEMUIOVector *qiov, int flags)

{

    NBDClientSession *client = nbd_get_client_session(bs);

    NBDRequest request = {

        .type = NBD_CMD_READ,

        .from = offset,

        .len = bytes,

    };

    NBDReply reply;

    ssize_t ret;



    assert(bytes <= NBD_MAX_BUFFER_SIZE);

    assert(!flags);



    ret = nbd_co_send_request(bs, &request, NULL);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, &request, &reply, qiov);

    }

    nbd_coroutine_end(bs, &request);

    return -reply.error;

    return nbd_co_request(bs, &request, qiov);

}



int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
@@ -256,8 +265,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset, 
        .from = offset,

        .len = bytes,

    };

    NBDReply reply;

    ssize_t ret;



    if (flags & BDRV_REQ_FUA) {

        assert(client->info.flags & NBD_FLAG_SEND_FUA);
@@ -266,27 +273,18 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset, 


    assert(bytes <= NBD_MAX_BUFFER_SIZE);



    ret = nbd_co_send_request(bs, &request, qiov);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, &request, &reply, NULL);

    }

    nbd_coroutine_end(bs, &request);

    return -reply.error;

    return nbd_co_request(bs, &request, qiov);

}



int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,

                                int bytes, BdrvRequestFlags flags)

{

    ssize_t ret;

    NBDClientSession *client = nbd_get_client_session(bs);

    NBDRequest request = {

        .type = NBD_CMD_WRITE_ZEROES,

        .from = offset,

        .len = bytes,

    };

    NBDReply reply;



    if (!(client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {

        return -ENOTSUP;
@@ -300,22 +298,13 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, 
        request.flags |= NBD_CMD_FLAG_NO_HOLE;

    }



    ret = nbd_co_send_request(bs, &request, NULL);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, &request, &reply, NULL);

    }

    nbd_coroutine_end(bs, &request);

    return -reply.error;

    return nbd_co_request(bs, &request, NULL);

}



int nbd_client_co_flush(BlockDriverState *bs)

{

    NBDClientSession *client = nbd_get_client_session(bs);

    NBDRequest request = { .type = NBD_CMD_FLUSH };

    NBDReply reply;

    ssize_t ret;



    if (!(client->info.flags & NBD_FLAG_SEND_FLUSH)) {

        return 0;
@@ -324,14 +313,7 @@ int nbd_client_co_flush(BlockDriverState *bs) 
    request.from = 0;

    request.len = 0;



    ret = nbd_co_send_request(bs, &request, NULL);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, &request, &reply, NULL);

    }

    nbd_coroutine_end(bs, &request);

    return -reply.error;

    return nbd_co_request(bs, &request, NULL);

}



int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
@@ -342,22 +324,12 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) 
        .from = offset,

        .len = bytes,

    };

    NBDReply reply;

    ssize_t ret;



    if (!(client->info.flags & NBD_FLAG_SEND_TRIM)) {

        return 0;

    }



    ret = nbd_co_send_request(bs, &request, NULL);

    if (ret < 0) {

        reply.error = -ret;

    } else {

        nbd_co_receive_reply(client, &request, &reply, NULL);

    }

    nbd_coroutine_end(bs, &request);

    return -reply.error;



    return nbd_co_request(bs, &request, NULL);

}



void nbd_client_detach_aio_context(BlockDriverState *bs)

include/block/nbd.h

+2 −2
Original line number Diff line number Diff line
@@ -163,8 +163,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, 
                          Error **errp);

int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,

             Error **errp);

ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request);

ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp);

int nbd_send_request(QIOChannel *ioc, NBDRequest *request);

int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp);

int nbd_client(int fd);

int nbd_disconnect(int fd);

nbd/client.c

+10 −11
Original line number Diff line number Diff line
@@ -399,12 +399,10 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, 
               phase, but make sure it sent flags */

            if (len) {

                error_setg(errp, "server sent invalid NBD_REP_ACK");

                nbd_send_opt_abort(ioc);

                return -1;

            }

            if (!info->flags) {

                error_setg(errp, "broken server omitted NBD_INFO_EXPORT");

                nbd_send_opt_abort(ioc);

                return -1;

            }

            trace_nbd_opt_go_success();
@@ -898,7 +896,7 @@ int nbd_disconnect(int fd) 
}

#endif



ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)

int nbd_send_request(QIOChannel *ioc, NBDRequest *request)

{

    uint8_t buf[NBD_REQUEST_SIZE];
@@ -916,22 +914,22 @@ ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request) 
    return nbd_write(ioc, buf, sizeof(buf), NULL);

}



ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)

/* nbd_receive_reply

 * Returns 1 on success

 *         0 on eof, when no data was read (errp is not set)

 *         negative errno on failure (errp is set)

 */

int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)

{

    uint8_t buf[NBD_REPLY_SIZE];

    uint32_t magic;

    ssize_t ret;

    int ret;



    ret = nbd_read_eof(ioc, buf, sizeof(buf), errp);

    if (ret <= 0) {

        return ret;

    }



    if (ret != sizeof(buf)) {

        error_setg(errp, "read failed");

        return -EINVAL;

    }



    /* Reply

       [ 0 ..  3]    magic   (NBD_REPLY_MAGIC)

       [ 4 ..  7]    error   (0 == no error)
@@ -955,6 +953,7 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp) 
        error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);

        return -EINVAL;

    }

    return sizeof(buf);



    return 1;

}

nbd/nbd-internal.h

+24 −9
Original line number Diff line number Diff line
@@ -77,21 +77,36 @@ 
#define NBD_ESHUTDOWN  108



/* nbd_read_eof

 * Tries to read @size bytes from @ioc. Returns number of bytes actually read.

 * May return a value >= 0 and < size only on EOF, i.e. when iteratively called

 * qio_channel_readv() returns 0. So, there is no need to call nbd_read_eof

 * iteratively.

 * Tries to read @size bytes from @ioc.

 * Returns 1 on success

 *         0 on eof, when no data was read (errp is not set)

 *         negative errno on failure (errp is set)

 */

static inline ssize_t nbd_read_eof(QIOChannel *ioc, void *buffer, size_t size,

static inline int nbd_read_eof(QIOChannel *ioc, void *buffer, size_t size,

                               Error **errp)

{

    struct iovec iov = { .iov_base = buffer, .iov_len = size };

    ssize_t ret;



    /* Sockets are kept in blocking mode in the negotiation phase.  After

     * that, a non-readable socket simply means that another thread stole

     * our request/reply.  Synchronization is done with recv_coroutine, so

     * that this is coroutine-safe.

     */

    return nbd_rwv(ioc, &iov, 1, size, true, errp);



    assert(size);



    ret = nbd_rwv(ioc, &iov, 1, size, true, errp);

    if (ret <= 0) {

        return ret;

    }



    if (ret != size) {

        error_setg(errp, "End of file");

        return -EINVAL;

    }



    return 1;

}



/* nbd_read
@@ -100,9 +115,9 @@ static inline ssize_t nbd_read_eof(QIOChannel *ioc, void *buffer, size_t size, 
static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size,

                           Error **errp)

{

    ssize_t ret = nbd_read_eof(ioc, buffer, size, errp);

    int ret = nbd_read_eof(ioc, buffer, size, errp);



    if (ret >= 0 && ret != size) {

    if (ret == 0) {

        ret = -EINVAL;

        error_setg(errp, "End of file");

    }

tests/qemu-iotests/083

+84 −52
Original line number Diff line number Diff line
@@ -27,6 +27,14 @@ echo "QA output created by $seq" 
here=`pwd`

status=1	# failure is the default!



_cleanup()

{

	rm -f nbd.sock

	rm -f nbd-fault-injector.out

	rm -f nbd-fault-injector.conf

}

trap "_cleanup; exit \$status" 0 1 2 3 15



# get standard environment, filters and checks

. ./common.rc

. ./common.filter
@@ -35,73 +43,96 @@ _supported_fmt generic 
_supported_proto nbd

_supported_os Linux



# Pick a TCP port based on our pid.  This way multiple instances of this test

# can run in parallel without conflicting.

choose_tcp_port() {

	echo $((($$ % 31744) + 1024)) # 1024 <= port < 32768

}

check_disconnect() {

	local event export_name=foo extra_args nbd_addr nbd_url proto when



wait_for_tcp_port() {

	while ! (netstat --tcp --listening --numeric | \

		 grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") >/dev/null 2>&1; do

		sleep 0.1

	while true; do

		case $1 in

		--classic-negotiation)

			shift

			extra_args=--classic-negotiation

			export_name=

			;;

		--tcp)

			shift

			proto=tcp

			;;

		--unix)

			shift

			proto=unix

			;;

		*)

			break

			;;

		esac

	done

}



check_disconnect() {

	event=$1

	when=$2

	negotiation=$3

	echo "=== Check disconnect $when $event ==="

	echo



	port=$(choose_tcp_port)



	cat > "$TEST_DIR/nbd-fault-injector.conf" <<EOF

[inject-error]

event=$event

when=$when

EOF



	if [ "$negotiation" = "--classic-negotiation" ]; then

		extra_args=--classic-negotiation

		nbd_url="nbd:127.0.0.1:$port"

	if [ "$proto" = "tcp" ]; then

		nbd_addr="127.0.0.1:0"

	else

		nbd_addr="$TEST_DIR/nbd.sock"

	fi



	rm -f "$TEST_DIR/nbd.sock"



	$PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 &



	# Wait for server to be ready

	while ! grep -q 'Listening on ' "$TEST_DIR/nbd-fault-injector.out"; do

		sleep 0.1

	done



	# Extract the final address (port number has now been assigned in tcp case)

	nbd_addr=$(sed 's/Listening on \(.*\)$/\1/' "$TEST_DIR/nbd-fault-injector.out")



	if [ "$proto" = "tcp" ]; then

		nbd_url="nbd+tcp://$nbd_addr/$export_name"

	else

		nbd_url="nbd:127.0.0.1:$port:exportname=foo"

		nbd_url="nbd+unix:///$export_name?socket=$nbd_addr"

	fi



	$PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" >/dev/null 2>&1 &

	wait_for_tcp_port "127\\.0\\.0\\.1:$port"

	$QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd



	echo

}



for proto in tcp unix; do

	for event in neg1 "export" neg2 request reply data; do

		for when in before after; do

		check_disconnect "$event" "$when"

			check_disconnect "--$proto" "$event" "$when"

		done



		# Also inject short replies from the NBD server

		case "$event" in

		neg1)

			for when in 8 16; do

			check_disconnect "$event" "$when"

				check_disconnect "--$proto" "$event" "$when"

			done

			;;

		"export")

			for when in 4 12 16; do

			check_disconnect "$event" "$when"

				check_disconnect "--$proto" "$event" "$when"

			done

			;;

		neg2)

			for when in 8 10; do

			check_disconnect "$event" "$when"

				check_disconnect "--$proto" "$event" "$when"

			done

			;;

		reply)

			for when in 4 8; do

			check_disconnect "$event" "$when"

				check_disconnect "--$proto" "$event" "$when"

			done

			;;

		esac
@@ -109,7 +140,8 @@ done 


	# Also check classic negotiation without export information

	for when in before 8 16 24 28 after; do

	check_disconnect "neg-classic" "$when" --classic-negotiation

		check_disconnect "--$proto" --classic-negotiation "neg-classic" "$when"

	done

done



# success, all done