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: Peter Maydell <peter.maydell@linaro.org>

6 files changed, 245 insertions, 82 deletions

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index bff9360..0306f11 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -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,81 +43,105 @@ _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
-}
-
-wait_for_tcp_port() {
-	while ! (netstat --tcp --listening --numeric | \
-		 grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") >/dev/null 2>&1; do
-		sleep 0.1
+check_disconnect() {
+	local event export_name=foo extra_args nbd_addr nbd_url proto when
+
+	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_url="nbd:127.0.0.1:$port:exportname=foo"
+		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+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 event in neg1 "export" neg2 request reply data; do
-	for when in before after; do
-		check_disconnect "$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"
-		done
-		;;
-	"export")
-		for when in 4 12 16; do
-			check_disconnect "$event" "$when"
+for proto in tcp unix; do
+	for event in neg1 "export" neg2 request reply data; do
+		for when in before after; do
+			check_disconnect "--$proto" "$event" "$when"
 		done
-		;;
-	neg2)
-		for when in 8 10; do
-			check_disconnect "$event" "$when"
-		done
-		;;
-	reply)
-		for when in 4 8; do
-			check_disconnect "$event" "$when"
-		done
-		;;
-	esac
-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
+		# Also inject short replies from the NBD server
+		case "$event" in
+		neg1)
+			for when in 8 16; do
+				check_disconnect "--$proto" "$event" "$when"
+			done
+			;;
+		"export")
+			for when in 4 12 16; do
+				check_disconnect "--$proto" "$event" "$when"
+			done
+			;;
+		neg2)
+			for when in 8 10; do
+				check_disconnect "--$proto" "$event" "$when"
+			done
+			;;
+		reply)
+			for when in 4 8; do
+				check_disconnect "--$proto" "$event" "$when"
+			done
+			;;
+		esac
+	done
+
+	# Also check classic negotiation without export information
+	for when in before 8 16 24 28 after; do
+		check_disconnect "--$proto" --classic-negotiation "neg-classic" "$when"
+	done
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index a24c6bf..fb71b6f 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -1,43 +1,43 @@
 QA output created by 083
 === Check disconnect before neg1 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect after neg1 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 8 neg1 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 16 neg1 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect before export ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect after export ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 4 export ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 12 export ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 16 export ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect before neg2 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect after neg2 ===
 
@@ -45,11 +45,11 @@ read failed: Input/output error
 
 === Check disconnect 8 neg2 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect 10 neg2 ===
 
-can't open device nbd:127.0.0.1:PORT:exportname=foo
+can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect before request ===
 
@@ -69,12 +69,12 @@ read failed: Input/output error
 
 === Check disconnect 4 reply ===
 
-read failed
+End of file
 read failed: Input/output error
 
 === Check disconnect 8 reply ===
 
-read failed
+End of file
 read failed: Input/output error
 
 === Check disconnect before data ===
@@ -88,23 +88,134 @@ read 512/512 bytes at offset 0
 
 === Check disconnect before neg-classic ===
 
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
 
 === Check disconnect 8 neg-classic ===
 
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
 
 === Check disconnect 16 neg-classic ===
 
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
 
 === Check disconnect 24 neg-classic ===
 
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
 
 === Check disconnect 28 neg-classic ===
 
-can't open device nbd:127.0.0.1:PORT
+can't open device nbd+tcp://127.0.0.1:PORT/
+
+=== Check disconnect after neg-classic ===
+
+read failed: Input/output error
+
+=== Check disconnect before neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 8 neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 neg1 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 4 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 12 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 export ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect after neg2 ===
+
+read failed: Input/output error
+
+=== Check disconnect 8 neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 10 neg2 ===
+
+can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect before request ===
+
+read failed: Input/output error
+
+=== Check disconnect after request ===
+
+read failed: Input/output error
+
+=== Check disconnect before reply ===
+
+read failed: Input/output error
+
+=== Check disconnect after reply ===
+
+read failed: Input/output error
+
+=== Check disconnect 4 reply ===
+
+End of file
+read failed: Input/output error
+
+=== Check disconnect 8 reply ===
+
+End of file
+read failed: Input/output error
+
+=== Check disconnect before data ===
+
+read failed: Input/output error
+
+=== Check disconnect after data ===
+
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Check disconnect before neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 8 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 16 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 24 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+
+=== Check disconnect 28 neg-classic ===
+
+can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
 
 === Check disconnect after neg-classic ===
 
diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index 8028111..a3e3bad 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -46,16 +46,17 @@ iotests.log('Launching NBD server on destination...')
 iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
 iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
 
-iotests.log('Starting drive-mirror on source...')
+iotests.log('Starting `drive-mirror` on source...')
 iotests.log(source_vm.qmp(
               'drive-mirror',
               device='drive0',
               target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
               sync='full',
               format='raw', # always raw, the server handles the format
-              mode='existing'))
+              mode='existing',
+              job_id='mirror-job0'))
 
-iotests.log('Waiting for drive-mirror to complete...')
+iotests.log('Waiting for `drive-mirror` to complete...')
 iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
             filters=[iotests.filter_qmp_event])
 
@@ -67,7 +68,17 @@ dest_vm.qmp('migrate-set-capabilities',
 iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))
 
 while True:
-    event = source_vm.event_wait('MIGRATION')
-    iotests.log(event, filters=[iotests.filter_qmp_event])
-    if event['data']['status'] in ('completed', 'failed'):
+    event1 = source_vm.event_wait('MIGRATION')
+    iotests.log(event1, filters=[iotests.filter_qmp_event])
+    if event1['data']['status'] in ('completed', 'failed'):
+        iotests.log('Gracefully ending the `drive-mirror` job on source...')
+        iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
+        break
+
+while True:
+    event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
+    iotests.log(event2, filters=[iotests.filter_qmp_event])
+    if event2['event'] == 'BLOCK_JOB_COMPLETED':
+        iotests.log('Stopping the NBD server on destination...')
+        iotests.log(dest_vm.qmp('nbd-server-stop'))
         break
diff --git a/tests/qemu-iotests/194.out b/tests/qemu-iotests/194.out
index ae501fe..50ac50d 100644
--- a/tests/qemu-iotests/194.out
+++ b/tests/qemu-iotests/194.out
@@ -2,12 +2,17 @@ Launching VMs...
 Launching NBD server on destination...
 {u'return': {}}
 {u'return': {}}
-Starting drive-mirror on source...
+Starting `drive-mirror` on source...
 {u'return': {}}
-Waiting for drive-mirror to complete...
-{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'drive0', u'type': u'mirror', u'speed': 0, u'len': 1073741824, u'offset': 1073741824}, u'event': u'BLOCK_JOB_READY'}
+Waiting for `drive-mirror` to complete...
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror-job0', u'type': u'mirror', u'speed': 0, u'len': 1073741824, u'offset': 1073741824}, u'event': u'BLOCK_JOB_READY'}
 Starting migration...
 {u'return': {}}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'setup'}, u'event': u'MIGRATION'}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'active'}, u'event': u'MIGRATION'}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'completed'}, u'event': u'MIGRATION'}
+Gracefully ending the `drive-mirror` job on source...
+{u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror-job0', u'type': u'mirror', u'speed': 0, u'len': 1073741824, u'offset': 1073741824}, u'event': u'BLOCK_JOB_COMPLETED'}
+Stopping the NBD server on destination...
+{u'return': {}}
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 7a58e57..9d5442e 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -170,9 +170,9 @@ _filter_nbd()
     #
     # Filter out the TCP port number since this changes between runs.
     sed -e '/nbd\/.*\.c:/d' \
-        -e 's#nbd:\(//\)\?127\.0\.0\.1:[0-9]*#nbd:\1127.0.0.1:PORT#g' \
+        -e 's#127\.0\.0\.1:[0-9]*#127.0.0.1:PORT#g' \
         -e "s#?socket=$TEST_DIR#?socket=TEST_DIR#g" \
-        -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#'
+        -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
 }
 
 # make sure this script returns success
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index 6c07191..1c10dcb 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -235,11 +235,15 @@ def open_socket(path):
         sock = socket.socket()
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         sock.bind((host, int(port)))
+
+        # If given port was 0 the final port number is now available
+        path = '%s:%d' % sock.getsockname()
     else:
         sock = socket.socket(socket.AF_UNIX)
         sock.bind(path)
     sock.listen(0)
     print 'Listening on %s' % path
+    sys.stdout.flush() # another process may be waiting, show message now
     return sock
 
 def usage(args):