Commit Graph

6 Commits

Author SHA1 Message Date
Paolo Bonzini
dacca04c8d nbd: do not hang nbd_wr_syncv if outside a coroutine and no available data
Until commit 1c778ef7 ("nbd: convert to using I/O channels for actual
socket I/O", 2016-02-16), nbd_wr_sync returned -EAGAIN this scenario.
nbd_reply_ready required these semantics because it has two conflicting
requirements:

1) if a reply can be received on the socket, nbd_reply_ready needs
to read the header outside coroutine context to identify _which_
coroutine to enter to process the rest of the reply

2) on the other hand, nbd_reply_ready can find a false positive if
another thread (e.g. a VCPU thread running aio_poll) sneaks in and
calls nbd_reply_ready too.  In this case nbd_reply_ready does nothing
and expects nbd_wr_syncv to return -EAGAIN.

Currently, the solution to the first requirement is to wait in the very
rare case of a read() that doesn't retrieve the reply header in its
entirety; this is what nbd_wr_syncv does by calling qio_channel_wait().
However, the unconditional call to qio_channel_wait() breaks the second
requirement.  To fix this, the patch makes nbd_wr_syncv return -EAGAIN
if done is zero, similar to the code before commit 1c778ef7.

This is okay because NBD client-side negotiation is the only other case
that calls nbd_wr_syncv outside a coroutine, and it places the socket
in blocking mode.  On the other hand, it is a bit unpleasant to put
this in nbd_wr_syncv(), because the function is used by both client
and server.

The full fix would be to add a counter to NbdClientSession for how
many bytes have been filled in s->reply.  Then a reply can be filled
by multiple separate invocations of nbd_reply_ready and the
qio_channel_wait() call can be removed completely.  Something to
consider for 2.7...

Reported-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08 00:07:44 +02:00
Markus Armbruster
da34e65cb4 include/qemu/osdep.h: Don't include qapi/error.h
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef.  Since then, we've moved to include qemu/osdep.h
everywhere.  Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h.  That's in excess of
100KiB of crap most .c files don't actually need.

Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h.  Include qapi/error.h in .c files that need it and don't
get it now.  Include qapi-types.h in qom/object.h for uint16List.

Update scripts/clean-includes accordingly.  Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h.  Update the list of includes in the qemu/osdep.h
comment quoted above similarly.

This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third.  Unfortunately, the number depending on
qapi-types.h shrinks only a little.  More work is needed for that one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-22 22:20:15 +01:00
Daniel P. Berrange
f95910fe6b nbd: implement TLS support in the protocol negotiation
This extends the NBD protocol handling code so that it is capable
of negotiating TLS support during the connection setup. This involves
requesting the STARTTLS protocol option before any other NBD options.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1455129674-17255-14-git-send-email-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16 17:16:28 +01:00
Daniel P. Berrange
1c778ef729 nbd: convert to using I/O channels for actual socket I/O
Now that all callers are converted to use I/O channels for
initial connection setup, it is possible to switch the core
NBD protocol handling core over to use QIOChannel APIs for
actual sockets I/O.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1455129674-17255-7-git-send-email-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16 17:13:57 +01:00
Peter Maydell
d38ea87ac5 all: Clean up includes
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1454089805-5470-16-git-send-email-peter.maydell@linaro.org
2016-02-04 17:41:30 +00:00
Fam Zheng
798bfe0006 nbd: Split nbd.c
We have NBD server code and client code, all mixed in a file. Now split
them into separate files under nbd/, and update MAINTAINERS.

filter_nbd for iotest 083 is updated to keep the log filtered out.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1452760863-25350-3-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-15 18:58:02 +01:00