Commit Graph

13 Commits

Author SHA1 Message Date
Vladimir Sementsov-Ogievskiy d8b4bad846 block/nbd-client: use traces instead of noisy error_report_err
Reduce extra noise of nbd-client, change 083 correspondingly.

In various commits (be41c100 in 2.10, f140e300 in 2.11, 78a33ab
in 2.12), we added spots where qemu as an NBD client would report
problems communicating with the server to stderr, because there
was no where else to send the error to.  However, this is racy,
particularly since the most common source of these errors is when
either the client or the server abruptly hangs up, leaving one
coroutine to report the error only if it wins (or loses) the
race in attempting the read from the server before another
thread completes its cleanup of a protocol error that caused the
disconnect in the first place.  The race is also apparent in the
fact that differences in the flush behavior of the server can
alter the frequency of encountering the race in the client (see
commit 6d39db96).

Rather than polluting stderr, it's better to just trace these
situations, for use by developers debugging a flaky connection,
particularly since the real error that either triggers the abrupt
disconnection in the first place, or that results from the EIO
when a request can't receive a reply, DOES make it back to the
user in the normal Error propagation channels.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20181102151152.288399-4-vsementsov@virtuozzo.com>
[eblake: drop depedence on error hint, enhance commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
2019-01-04 17:34:58 -06:00
Max Reitz 6d39db96d2 iotests: Make nbd-fault-injector flush
When closing a connection, make the nbd-fault-injector flush the socket.
Without this, the output is a bit unreliable with Python 3.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20181022135307.14398-2-mreitz@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2018-10-30 21:11:52 -03:00
Vladimir Sementsov-Ogievskiy f140e30003 nbd: Minimal structured read for client
Minimal implementation: for structured error only error_report error
message.

Note that test 83 is now more verbose, because the implementation
prints more warnings about unexpected communication errors; perhaps
future patches should tone things down by using trace messages
instead of traces, but the common case of successful communication
is no noisier than before.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171027104037.8319-13-eblake@redhat.com>
2017-10-30 21:48:41 +01:00
Eric Blake 030fa7f6f9 nbd: Use new qio_channel_*_all() functions
Rather than open-coding our own read/write-all functions, we
can make use of the recently-added qio code.  It slightly
changes the error message in one of the iotests.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170905191114.5959-4-eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
2017-09-06 10:11:54 -05:00
Vladimir Sementsov-Ogievskiy ab01df1fe2 nbd/client: refactor nbd_read_eof
Refactor nbd_read_eof to return 1 on success, 0 on eof, when no
data was read and <0 for other cases, because returned size of
read data is not actually used.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170804151440.320927-3-vsementsov@virtuozzo.com>
[eblake: tweak function comments, rebase to test 083 enhancements]
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-08-30 13:00:38 -05:00
Stefan Hajnoczi 02d2d860d2 qemu-iotests: test NBD over UNIX domain sockets in 083
083 only tests TCP.  Some failures might be specific to UNIX domain
sockets.

A few adjustments are necessary:

1. Generating a port number and waiting for server startup is
   TCP-specific.  Use the new nbd-fault-injector.py startup protocol to
   fetch the address.  This is a little more elegant because we don't
   need netstat anymore.

2. The NBD filter does not work for the UNIX domain sockets URIs we
   generate and must be extended.

3. Run all tests twice: once for TCP and once for UNIX domain sockets.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20170829122745.14309-4-stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2017-08-30 13:00:38 -05:00
Vladimir Sementsov-Ogievskiy be41c100c0 nbd/client.c: use errp instead of LOG
Move to modern errp scheme from just LOGging errors.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170526110913.89098-1-vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06 20:18:36 +02:00
Nir Soffer b7aa131519 qemu-io: Return non-zero exit code on failure
The result of openfile was not checked, leading to failure deep in the
actual command with confusing error message, and exiting with exit code 0.

Here is a simple example - trying to read with the wrong format:

    $ touch file
    $ qemu-io -f qcow2 -c 'read -P 1 0 1024' file; echo $?
    can't open device file: Image is not in qcow2 format
    no file open, try 'help open'
    0

With this patch, we fail earlier with exit code 1:

    $ ./qemu-io -f qcow2 -c 'read -P 1 0 1024' file; echo $?
    can't open device file: Image is not in qcow2 format
    1

Failing earlier, we don't log this error now:

    no file open, try 'help open'

But some tests expected it; the line was removed from the test output.

Signed-off-by: Nir Soffer <nirsof@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20170201003120.23378-2-nirsof@gmail.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-02-12 00:47:42 +01:00
Max Reitz dd170c0677 iotests: Make _filter_nbd drop log lines
The NBD log lines ("/your/source/dir/nbd/xyz.c:function():line: error")
should not be converted to empty lines but removed altogether.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-02-02 17:49:42 +01:00
Markus Armbruster b988468149 qemu-io qemu-nbd: Use error_report() etc. instead of fprintf()
Just three instances left.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-16-git-send-email-armbru@redhat.com>
2016-01-13 15:16:18 +01:00
Max Reitz 1ce52846d3 nbd: Improve error messages
This patch makes use of the Error object for nbd_receive_negotiate() so
that errors during negotiation look nicer.

Furthermore, this patch adds an additional error message if the received
magic was wrong, but would be correct for the other protocol version,
respectively: So if an export name was specified, but the NBD server
magic corresponds to an old handshake, this condition is explicitly
signaled to the user, and vice versa.

As these messages are now part of the "Could not open image" error
message, additional filtering has to be employed in iotest 083, which
this patch does as well.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-02-06 17:24:22 +01:00
Max Reitz 53f9e77f4e iotests: Fix 083
As of 8f9e835fd2, probing should be
disabled in the qemu-iotests (at least when using qemu-io). This broke
083's reference output (which consisted mostly of "Could not read image
for determining its format").

This patch fixes it.

Note that one case which failed before is now successful: Disconnect
after data. This is due to qemu having read twice before (once for
probing, once for the qemu-io read command), but only once now (the
qemu-io read command). Therefore, reading is successful (which is
correct).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-02-06 17:24:22 +01:00
Stefan Hajnoczi dc668ded10 qemu-iotests: add 083 NBD client disconnect tests
This new test case uses nbd-fault-injector.py to simulate broken TCP
connections at each stage in the NBD protocol.  This way we can exercise
block/nbd-client.c's socket error handling code paths.

In particular, this serves as a regression test to make sure
nbd-client.c doesn't cause an infinite loop by leaving its
nbd_receive_reply() fd handler registered after the connection has been
closed.  This bug was fixed in an earlier patch.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-03-14 16:29:02 +01:00