From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVewZ-0008Uo-3j for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:36:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVewU-0003wC-3v for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:36:55 -0400 Sender: Paolo Bonzini Message-ID: <550028CC.9090500@redhat.com> Date: Wed, 11 Mar 2015 12:36:44 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1424887718-10800-1-git-send-email-mreitz@redhat.com> In-Reply-To: <1424887718-10800-1-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/25] nbd: Several fixes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 25/02/2015 19:08, Max Reitz wrote: > This series contains a variety of fixes for qemu's NBD code, with the > most interesting (and probably most prone to be wrong) thing being a > timeout of ten seconds for NBD connections. As a first step, I am applying these patches: Max Reitz (12): util/uri: Add overflow check to rfc3986_parse_port qemu-nbd: Detect unused partitions by system == 0 nbd: Fix nbd_establish_connection()'s return value nbd: Fix response to invalid requests nbd: Pass return value from nbd_handle_list() nbd: Handle blk_getlength() failure qemu-nbd: fork() can fail nbd: Fix potential signed overflow issues nbd: Set block size to BDRV_SECTOR_SIZE nbd: Fix nbd_receive_options() nbd: Fix interpretation of the export flags nbd: Drop unexpected data for NBD_OPT_LIST coroutine-io: Return -errno in case of error I disagree on the timeouts, because they can cause reordering of operations (where you write A, get a timeout, write B, succeed, but then A succeeds too and the application is confused). The only correct application of timeouts is to tear down the connection, reconnect _and_ resubmit pending I/O operations. For all other patches, either I replied directly to the messages, or they dependent on previous ones that are not included. Thanks! Paolo > > > Max Reitz (25): > util/uri: Add overflow check to rfc3986_parse_port > qemu-nbd: Detect unused partitions by system == 0 > nbd: Fix nbd_establish_connection()'s return value > nbd: Fix response to invalid requests > nbd: Avoid generic -EINVAL > nbd: Pass return value from nbd_handle_list() > nbd: Add "failed to open export" error message > nbd: Handle blk_getlength() failure > qemu-nbd: fork() can fail > nbd: Fix potential signed overflow issues > qemu-nbd: Fix and improve input verification > nbd: Set block size to BDRV_SECTOR_SIZE > nbd: Enforce sector alignment > coroutine: Add co_yield_timeout() > coroutine-io: Return -errno in case of error > coroutine-io: Add I/O functions with timeout > nbd: Employ timeouts > nbd: Fix nbd_receive_options() > nbd: Fix interpretation of the export flags > block/nbd: Comment on discard/flush silently failing > nbd: Drop unexpected data for NBD_OPT_LIST > iotests: Add _timeout function > iotests: Add test for invalid qemu-nbd parameters > iotests: Add test for issuing discard over NBD > iotests: Add test for a non-existing NBD export > > block/nbd-client.c | 40 +++++++-- > block/nbd-client.h | 1 - > block/nbd.c | 2 +- > blockdev-nbd.c | 6 +- > include/block/coroutine.h | 6 ++ > include/block/nbd.h | 13 +-- > include/qemu-common.h | 45 ++++++++-- > nbd.c | 203 ++++++++++++++++++++++++++++++------------- > qemu-coroutine-io.c | 25 ++++-- > qemu-coroutine-sleep.c | 34 ++++++++ > qemu-nbd.c | 74 +++++++++++----- > tests/qemu-iotests/096.out | 2 +- > tests/qemu-iotests/125 | 69 +++++++++++++++ > tests/qemu-iotests/125.out | 21 +++++ > tests/qemu-iotests/126 | 105 ++++++++++++++++++++++ > tests/qemu-iotests/126.out | 28 ++++++ > tests/qemu-iotests/127 | 80 +++++++++++++++++ > tests/qemu-iotests/127.out | 14 +++ > tests/qemu-iotests/common.rc | 16 ++++ > tests/qemu-iotests/group | 3 + > util/uri.c | 24 ++--- > 21 files changed, 686 insertions(+), 125 deletions(-) > create mode 100755 tests/qemu-iotests/125 > create mode 100644 tests/qemu-iotests/125.out > create mode 100755 tests/qemu-iotests/126 > create mode 100644 tests/qemu-iotests/126.out > create mode 100755 tests/qemu-iotests/127 > create mode 100644 tests/qemu-iotests/127.out >