* [Qemu-devel] [PATCH 01/15] qemu-nbd: remove offset argument to nbd_trip
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 02/15] qemu-nbd: remove data_size " Paolo Bonzini
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
The argument is write-only.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 8 +++-----
nbd.h | 2 +-
qemu-nbd.c | 3 +--
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/nbd.c b/nbd.c
index 6a96878..a96c109 100644
--- a/nbd.c
+++ b/nbd.c
@@ -585,8 +585,9 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
return 0;
}
-int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
- off_t *offset, uint32_t nbdflags, uint8_t *data, int data_size)
+int nbd_trip(BlockDriverState *bs, int csock, off_t size,
+ uint64_t dev_offset, uint32_t nbdflags,
+ uint8_t *data, int data_size)
{
struct nbd_request request;
struct nbd_reply reply;
@@ -637,7 +638,6 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
reply.error = -ret;
request.len = 0;
}
- *offset += request.len;
TRACE("Read %u byte(s)", request.len);
@@ -686,8 +686,6 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
request.len = 0;
}
- *offset += request.len;
-
if (request.type & NBD_CMD_FLAG_FUA) {
ret = bdrv_flush(bs);
if (ret < 0) {
diff --git a/nbd.h b/nbd.h
index 61553f4..ebdb2db 100644
--- a/nbd.h
+++ b/nbd.h
@@ -72,7 +72,7 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize);
int nbd_send_request(int csock, struct nbd_request *request);
int nbd_receive_reply(int csock, struct nbd_reply *reply);
int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
- off_t *offset, uint32_t nbdflags, uint8_t *data, int data_size);
+ uint32_t nbdflags, uint8_t *data, int data_size);
int nbd_client(int fd);
int nbd_disconnect(int fd);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d8d3e15..e2fe7cf 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -184,7 +184,6 @@ int main(int argc, char **argv)
{
BlockDriverState *bs;
off_t dev_offset = 0;
- off_t offset = 0;
uint32_t nbdflags = 0;
bool disconnect = false;
const char *bindto = "0.0.0.0";
@@ -462,7 +461,7 @@ int main(int argc, char **argv)
for (i = 1; i < nb_fds && ret; i++) {
if (FD_ISSET(sharing_fds[i], &fds)) {
if (nbd_trip(bs, sharing_fds[i], fd_size, dev_offset,
- &offset, nbdflags, data, NBD_BUFFER_SIZE) != 0) {
+ nbdflags, data, NBD_BUFFER_SIZE) != 0) {
close(sharing_fds[i]);
nb_fds--;
sharing_fds[i] = sharing_fds[nb_fds];
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 02/15] qemu-nbd: remove data_size argument to nbd_trip
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 01/15] qemu-nbd: remove offset argument to nbd_trip Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 03/15] move corking functions to osdep.c Paolo Bonzini
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
The size of the buffer is in practice part of the protocol.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 6 +++---
nbd.h | 4 +++-
qemu-nbd.c | 4 +---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/nbd.c b/nbd.c
index a96c109..59c47ee 100644
--- a/nbd.c
+++ b/nbd.c
@@ -587,7 +587,7 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
int nbd_trip(BlockDriverState *bs, int csock, off_t size,
uint64_t dev_offset, uint32_t nbdflags,
- uint8_t *data, int data_size)
+ uint8_t *data)
{
struct nbd_request request;
struct nbd_reply reply;
@@ -598,9 +598,9 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
if (nbd_receive_request(csock, &request) == -1)
return -1;
- if (request.len + NBD_REPLY_SIZE > data_size) {
+ if (request.len + NBD_REPLY_SIZE > NBD_BUFFER_SIZE) {
LOG("len (%u) is larger than max len (%u)",
- request.len + NBD_REPLY_SIZE, data_size);
+ request.len + NBD_REPLY_SIZE, NBD_BUFFER_SIZE);
errno = EINVAL;
return -1;
}
diff --git a/nbd.h b/nbd.h
index ebdb2db..dbc4c0d 100644
--- a/nbd.h
+++ b/nbd.h
@@ -57,6 +57,8 @@ enum {
#define NBD_DEFAULT_PORT 10809
+#define NBD_BUFFER_SIZE (1024*1024)
+
size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
int tcp_socket_outgoing(const char *address, uint16_t port);
int tcp_socket_incoming(const char *address, uint16_t port);
@@ -72,7 +74,7 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize);
int nbd_send_request(int csock, struct nbd_request *request);
int nbd_receive_reply(int csock, struct nbd_reply *reply);
int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
- uint32_t nbdflags, uint8_t *data, int data_size);
+ uint32_t nbdflags, uint8_t *data);
int nbd_client(int fd);
int nbd_disconnect(int fd);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e2fe7cf..befbbfd 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -34,8 +34,6 @@
#define SOCKET_PATH "/var/lock/qemu-nbd-%s"
-#define NBD_BUFFER_SIZE (1024*1024)
-
static int verbose;
static void usage(const char *name)
@@ -461,7 +459,7 @@ int main(int argc, char **argv)
for (i = 1; i < nb_fds && ret; i++) {
if (FD_ISSET(sharing_fds[i], &fds)) {
if (nbd_trip(bs, sharing_fds[i], fd_size, dev_offset,
- nbdflags, data, NBD_BUFFER_SIZE) != 0) {
+ nbdflags, data) != 0) {
close(sharing_fds[i]);
nb_fds--;
sharing_fds[i] = sharing_fds[nb_fds];
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 03/15] move corking functions to osdep.c
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 01/15] qemu-nbd: remove offset argument to nbd_trip Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 02/15] qemu-nbd: remove data_size " Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 04/15] qemu-nbd: simplify nbd_trip Paolo Bonzini
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/sheepdog.c | 20 ++------------------
osdep.c | 9 +++++++++
qemu_socket.h | 1 +
3 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d168681..ddf7fa2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -702,22 +702,6 @@ static int aio_flush_request(void *opaque)
return !QLIST_EMPTY(&s->outstanding_aio_head);
}
-#if !defined(SOL_TCP) || !defined(TCP_CORK)
-
-static int set_cork(int fd, int v)
-{
- return 0;
-}
-
-#else
-
-static int set_cork(int fd, int v)
-{
- return setsockopt(fd, SOL_TCP, TCP_CORK, &v, sizeof(v));
-}
-
-#endif
-
static int set_nodelay(int fd)
{
int ret, opt;
@@ -923,7 +907,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
s->co_send = qemu_coroutine_self();
qemu_aio_set_fd_handler(s->fd, co_read_response, co_write_request,
aio_flush_request, NULL, s);
- set_cork(s->fd, 1);
+ socket_set_cork(s->fd, 1);
/* send a header */
ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
@@ -940,7 +924,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
}
}
- set_cork(s->fd, 0);
+ socket_set_cork(s->fd, 0);
qemu_aio_set_fd_handler(s->fd, co_read_response, NULL,
aio_flush_request, NULL, s);
qemu_co_mutex_unlock(&s->lock);
diff --git a/osdep.c b/osdep.c
index 70bad27..3e6bada 100644
--- a/osdep.c
+++ b/osdep.c
@@ -48,6 +48,15 @@ extern int madvise(caddr_t, size_t, int);
#include "trace.h"
#include "qemu_socket.h"
+int socket_set_cork(int fd, int v)
+{
+#if defined(SOL_TCP) && defined(TCP_CORK)
+ return setsockopt(fd, SOL_TCP, TCP_CORK, &v, sizeof(v));
+#else
+ return 0;
+#endif
+}
+
int qemu_madvise(void *addr, size_t len, int advice)
{
if (advice == QEMU_MADV_INVALID) {
diff --git a/qemu_socket.h b/qemu_socket.h
index 9e32fac..fe4cf6c 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -35,6 +35,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
/* misc helpers */
int qemu_socket(int domain, int type, int protocol);
int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
+int socket_set_cork(int fd, int v);
void socket_set_block(int fd);
void socket_set_nonblock(int fd);
int send_all(int fd, const void *buf, int len1);
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 04/15] qemu-nbd: simplify nbd_trip
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (2 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 03/15] move corking functions to osdep.c Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 05/15] qemu-nbd: introduce nbd_do_send_reply Paolo Bonzini
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Use TCP_CORK to remove a violation of encapsulation, that would later
require nbd_trip to know too much about an NBD reply.
We could also switch to sendmsg (qemu_co_sendv) later, it is even
easier once coroutines are in.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 25 ++++++++-----------------
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/nbd.c b/nbd.c
index 59c47ee..252fa7c 100644
--- a/nbd.c
+++ b/nbd.c
@@ -598,9 +598,9 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
if (nbd_receive_request(csock, &request) == -1)
return -1;
- if (request.len + NBD_REPLY_SIZE > NBD_BUFFER_SIZE) {
+ if (request.len > NBD_BUFFER_SIZE) {
LOG("len (%u) is larger than max len (%u)",
- request.len + NBD_REPLY_SIZE, NBD_BUFFER_SIZE);
+ request.len, NBD_BUFFER_SIZE);
errno = EINVAL;
return -1;
}
@@ -631,8 +631,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
TRACE("Request type is READ");
ret = bdrv_read(bs, (request.from + dev_offset) / 512,
- data + NBD_REPLY_SIZE,
- request.len / 512);
+ data, request.len / 512);
if (ret < 0) {
LOG("reading from file failed");
reply.error = -ret;
@@ -640,26 +639,18 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
}
TRACE("Read %u byte(s)", request.len);
-
- /* Reply
- [ 0 .. 3] magic (NBD_REPLY_MAGIC)
- [ 4 .. 7] error (0 == no error)
- [ 7 .. 15] handle
- */
-
- cpu_to_be32w((uint32_t*)data, NBD_REPLY_MAGIC);
- cpu_to_be32w((uint32_t*)(data + 4), reply.error);
- cpu_to_be64w((uint64_t*)(data + 8), reply.handle);
+ socket_set_cork(csock, 1);
+ if (nbd_send_reply(csock, &reply) == -1)
+ return -1;
TRACE("Sending data to client");
- if (write_sync(csock, data,
- request.len + NBD_REPLY_SIZE) !=
- request.len + NBD_REPLY_SIZE) {
+ if (write_sync(csock, data, request.len) != request.len) {
LOG("writing to socket failed");
errno = EINVAL;
return -1;
}
+ socket_set_cork(csock, 0);
break;
case NBD_CMD_WRITE:
TRACE("Request type is WRITE");
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 05/15] qemu-nbd: introduce nbd_do_send_reply
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (3 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 04/15] qemu-nbd: simplify nbd_trip Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 06/15] qemu-nbd: more robust handling of invalid requests Paolo Bonzini
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Group the sending of a reply and the associated data into a new function.
Without corking, the caller would be forced to leave 12 free bytes at the
beginning of the data pointer. Not too ugly, but still ugly. :)
Using nbd_do_send_reply everywhere will help when the routine will set up
the write handler that re-enters the send coroutine.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 46 ++++++++++++++++++++++++++++++++--------------
1 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/nbd.c b/nbd.c
index 252fa7c..5fd6daf 100644
--- a/nbd.c
+++ b/nbd.c
@@ -585,6 +585,34 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
return 0;
}
+static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
+ uint8_t *data, int len)
+{
+ int rc, ret;
+
+ if (!len) {
+ rc = nbd_send_reply(csock, reply);
+ if (rc == -1) {
+ rc = -errno;
+ }
+ } else {
+ socket_set_cork(csock, 1);
+ rc = nbd_send_reply(csock, reply);
+ if (rc != -1) {
+ ret = write_sync(csock, data, len);
+ if (ret != len) {
+ errno = EIO;
+ rc = -1;
+ }
+ }
+ if (rc == -1) {
+ rc = -errno;
+ }
+ socket_set_cork(csock, 0);
+ }
+ return rc;
+}
+
int nbd_trip(BlockDriverState *bs, int csock, off_t size,
uint64_t dev_offset, uint32_t nbdflags,
uint8_t *data)
@@ -639,18 +667,8 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
}
TRACE("Read %u byte(s)", request.len);
- socket_set_cork(csock, 1);
- if (nbd_send_reply(csock, &reply) == -1)
+ if (nbd_do_send_reply(csock, &reply, data, request.len) < 0)
return -1;
-
- TRACE("Sending data to client");
-
- if (write_sync(csock, data, request.len) != request.len) {
- LOG("writing to socket failed");
- errno = EINVAL;
- return -1;
- }
- socket_set_cork(csock, 0);
break;
case NBD_CMD_WRITE:
TRACE("Request type is WRITE");
@@ -686,7 +704,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
}
}
- if (nbd_send_reply(csock, &reply) == -1)
+ if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
return -1;
break;
case NBD_CMD_DISC:
@@ -702,7 +720,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
reply.error = -ret;
}
- if (nbd_send_reply(csock, &reply) == -1)
+ if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
return -1;
break;
case NBD_CMD_TRIM:
@@ -713,7 +731,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
LOG("discard failed");
reply.error = -ret;
}
- if (nbd_send_reply(csock, &reply) == -1)
+ if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
return -1;
break;
default:
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 06/15] qemu-nbd: more robust handling of invalid requests
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (4 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 05/15] qemu-nbd: introduce nbd_do_send_reply Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 07/15] qemu-nbd: introduce nbd_do_receive_request Paolo Bonzini
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Fail invalid requests with EINVAL instead of dropping them into
the void.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 57 ++++++++++++++++++++++++++++++---------------------------
1 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/nbd.c b/nbd.c
index 5fd6daf..ec84071 100644
--- a/nbd.c
+++ b/nbd.c
@@ -626,18 +626,19 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
if (nbd_receive_request(csock, &request) == -1)
return -1;
+ reply.handle = request.handle;
+ reply.error = 0;
+
if (request.len > NBD_BUFFER_SIZE) {
LOG("len (%u) is larger than max len (%u)",
request.len, NBD_BUFFER_SIZE);
- errno = EINVAL;
- return -1;
+ goto invalid_request;
}
if ((request.from + request.len) < request.from) {
LOG("integer overflow detected! "
"you're probably being attacked");
- errno = EINVAL;
- return -1;
+ goto invalid_request;
}
if ((request.from + request.len) > size) {
@@ -645,15 +646,11 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
", Offset: %" PRIu64 "\n",
request.from, request.len, (uint64_t)size, dev_offset);
LOG("requested operation past EOF--bad client?");
- errno = EINVAL;
- return -1;
+ goto invalid_request;
}
TRACE("Decoding type");
- reply.handle = request.handle;
- reply.error = 0;
-
switch (request.type & NBD_CMD_MASK_COMMAND) {
case NBD_CMD_READ:
TRACE("Request type is READ");
@@ -663,7 +660,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
if (ret < 0) {
LOG("reading from file failed");
reply.error = -ret;
- request.len = 0;
+ goto error_reply;
}
TRACE("Read %u byte(s)", request.len);
@@ -683,24 +680,26 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
if (nbdflags & NBD_FLAG_READ_ONLY) {
TRACE("Server is read-only, return error");
- reply.error = 1;
- } else {
- TRACE("Writing to device");
+ reply.error = EROFS;
+ goto error_reply;
+ }
+
+ TRACE("Writing to device");
+
+ ret = bdrv_write(bs, (request.from + dev_offset) / 512,
+ data, request.len / 512);
+ if (ret < 0) {
+ LOG("writing to file failed");
+ reply.error = -ret;
+ goto error_reply;
+ }
- ret = bdrv_write(bs, (request.from + dev_offset) / 512,
- data, request.len / 512);
+ if (request.type & NBD_CMD_FLAG_FUA) {
+ ret = bdrv_flush(bs);
if (ret < 0) {
- LOG("writing to file failed");
+ LOG("flush failed");
reply.error = -ret;
- request.len = 0;
- }
-
- if (request.type & NBD_CMD_FLAG_FUA) {
- ret = bdrv_flush(bs);
- if (ret < 0) {
- LOG("flush failed");
- reply.error = -ret;
- }
+ goto error_reply;
}
}
@@ -736,8 +735,12 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
break;
default:
LOG("invalid request type (%u) received", request.type);
- errno = EINVAL;
- return -1;
+ invalid_request:
+ reply.error = -EINVAL;
+ error_reply:
+ if (nbd_do_send_reply(csock, &reply, NULL, 0) == -1)
+ return -1;
+ break;
}
TRACE("Request/Reply complete");
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 07/15] qemu-nbd: introduce nbd_do_receive_request
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (5 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 06/15] qemu-nbd: more robust handling of invalid requests Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 08/15] qemu-nbd: introduce NBDExport Paolo Bonzini
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Group the receiving of a response and the associated data into a new function.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 68 ++++++++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 47 insertions(+), 21 deletions(-)
diff --git a/nbd.c b/nbd.c
index ec84071..898206f 100644
--- a/nbd.c
+++ b/nbd.c
@@ -613,6 +613,47 @@ static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
return rc;
}
+static int nbd_do_receive_request(int csock, struct nbd_request *request,
+ uint8_t *data)
+{
+ int rc;
+
+ if (nbd_receive_request(csock, request) == -1) {
+ rc = -EIO;
+ goto out;
+ }
+
+ if (request->len > NBD_BUFFER_SIZE) {
+ LOG("len (%u) is larger than max len (%u)",
+ request->len, NBD_BUFFER_SIZE);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if ((request->from + request->len) < request->from) {
+ LOG("integer overflow detected! "
+ "you're probably being attacked");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ TRACE("Decoding type");
+
+ if ((request->type & NBD_CMD_MASK_COMMAND) == NBD_CMD_WRITE) {
+ TRACE("Reading %u byte(s)", request->len);
+
+ if (read_sync(csock, data, request->len) != request->len) {
+ LOG("reading from socket failed");
+ rc = -EIO;
+ goto out;
+ }
+ }
+ rc = 0;
+
+out:
+ return rc;
+}
+
int nbd_trip(BlockDriverState *bs, int csock, off_t size,
uint64_t dev_offset, uint32_t nbdflags,
uint8_t *data)
@@ -623,22 +664,17 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
TRACE("Reading request.");
- if (nbd_receive_request(csock, &request) == -1)
+ ret = nbd_do_receive_request(csock, &request, data);
+ if (ret == -EIO) {
return -1;
+ }
reply.handle = request.handle;
reply.error = 0;
- if (request.len > NBD_BUFFER_SIZE) {
- LOG("len (%u) is larger than max len (%u)",
- request.len, NBD_BUFFER_SIZE);
- goto invalid_request;
- }
-
- if ((request.from + request.len) < request.from) {
- LOG("integer overflow detected! "
- "you're probably being attacked");
- goto invalid_request;
+ if (ret < 0) {
+ reply.error = -ret;
+ goto error_reply;
}
if ((request.from + request.len) > size) {
@@ -649,8 +685,6 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
goto invalid_request;
}
- TRACE("Decoding type");
-
switch (request.type & NBD_CMD_MASK_COMMAND) {
case NBD_CMD_READ:
TRACE("Request type is READ");
@@ -670,14 +704,6 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
case NBD_CMD_WRITE:
TRACE("Request type is WRITE");
- TRACE("Reading %u byte(s)", request.len);
-
- if (read_sync(csock, data, request.len) != request.len) {
- LOG("reading from socket failed");
- errno = EINVAL;
- return -1;
- }
-
if (nbdflags & NBD_FLAG_READ_ONLY) {
TRACE("Server is read-only, return error");
reply.error = EROFS;
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 08/15] qemu-nbd: introduce NBDExport
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (6 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 07/15] qemu-nbd: introduce nbd_do_receive_request Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 09/15] qemu-nbd: introduce NBDRequest Paolo Bonzini
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Wrap the common parameters of nbd_trip in a single opaque struct.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 57 ++++++++++++++++++++++++++++++++++++++++++---------------
nbd.h | 9 +++++++--
qemu-nbd.c | 18 ++++++------------
3 files changed, 55 insertions(+), 29 deletions(-)
diff --git a/nbd.c b/nbd.c
index 898206f..00764da 100644
--- a/nbd.c
+++ b/nbd.c
@@ -18,6 +18,7 @@
#include "nbd.h"
#include "block.h"
+#include "block_int.h"
#include <errno.h>
#include <string.h>
@@ -585,6 +586,33 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
return 0;
}
+struct NBDExport {
+ BlockDriverState *bs;
+ off_t dev_offset;
+ off_t size;
+ uint8_t *data;
+ uint32_t nbdflags;
+};
+
+NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
+ off_t size, uint32_t nbdflags)
+{
+ NBDExport *exp = g_malloc0(sizeof(NBDExport));
+ exp->bs = bs;
+ exp->dev_offset = dev_offset;
+ exp->nbdflags = nbdflags;
+ exp->size = size == -1 ? exp->bs->total_sectors * 512 : size;
+ exp->data = qemu_blockalign(exp->bs, NBD_BUFFER_SIZE);
+ return exp;
+}
+
+void nbd_export_close(NBDExport *exp)
+{
+ qemu_vfree(exp->data);
+ bdrv_close(exp->bs);
+ g_free(exp);
+}
+
static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
uint8_t *data, int len)
{
@@ -654,9 +682,7 @@ out:
return rc;
}
-int nbd_trip(BlockDriverState *bs, int csock, off_t size,
- uint64_t dev_offset, uint32_t nbdflags,
- uint8_t *data)
+int nbd_trip(NBDExport *exp, int csock)
{
struct nbd_request request;
struct nbd_reply reply;
@@ -664,7 +690,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
TRACE("Reading request.");
- ret = nbd_do_receive_request(csock, &request, data);
+ ret = nbd_do_receive_request(csock, &request, exp->data);
if (ret == -EIO) {
return -1;
}
@@ -677,10 +703,11 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
goto error_reply;
}
- if ((request.from + request.len) > size) {
+ if ((request.from + request.len) > exp->size) {
LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64
", Offset: %" PRIu64 "\n",
- request.from, request.len, (uint64_t)size, dev_offset);
+ request.from, request.len,
+ (uint64_t)exp->size, exp->dev_offset);
LOG("requested operation past EOF--bad client?");
goto invalid_request;
}
@@ -689,8 +716,8 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
case NBD_CMD_READ:
TRACE("Request type is READ");
- ret = bdrv_read(bs, (request.from + dev_offset) / 512,
- data, request.len / 512);
+ ret = bdrv_read(exp->bs, (request.from + exp->dev_offset) / 512,
+ exp->data, request.len / 512);
if (ret < 0) {
LOG("reading from file failed");
reply.error = -ret;
@@ -698,13 +725,13 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
}
TRACE("Read %u byte(s)", request.len);
- if (nbd_do_send_reply(csock, &reply, data, request.len) < 0)
+ if (nbd_do_send_reply(csock, &reply, exp->data, request.len) < 0)
return -1;
break;
case NBD_CMD_WRITE:
TRACE("Request type is WRITE");
- if (nbdflags & NBD_FLAG_READ_ONLY) {
+ if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
TRACE("Server is read-only, return error");
reply.error = EROFS;
goto error_reply;
@@ -712,8 +739,8 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
TRACE("Writing to device");
- ret = bdrv_write(bs, (request.from + dev_offset) / 512,
- data, request.len / 512);
+ ret = bdrv_write(exp->bs, (request.from + exp->dev_offset) / 512,
+ exp->data, request.len / 512);
if (ret < 0) {
LOG("writing to file failed");
reply.error = -ret;
@@ -721,7 +748,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
}
if (request.type & NBD_CMD_FLAG_FUA) {
- ret = bdrv_flush(bs);
+ ret = bdrv_flush(exp->bs);
if (ret < 0) {
LOG("flush failed");
reply.error = -ret;
@@ -739,7 +766,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
case NBD_CMD_FLUSH:
TRACE("Request type is FLUSH");
- ret = bdrv_flush(bs);
+ ret = bdrv_flush(exp->bs);
if (ret < 0) {
LOG("flush failed");
reply.error = -ret;
@@ -750,7 +777,7 @@ int nbd_trip(BlockDriverState *bs, int csock, off_t size,
break;
case NBD_CMD_TRIM:
TRACE("Request type is TRIM");
- ret = bdrv_discard(bs, (request.from + dev_offset) / 512,
+ ret = bdrv_discard(exp->bs, (request.from + exp->dev_offset) / 512,
request.len / 512);
if (ret < 0) {
LOG("discard failed");
diff --git a/nbd.h b/nbd.h
index dbc4c0d..d368156 100644
--- a/nbd.h
+++ b/nbd.h
@@ -73,9 +73,14 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize);
int nbd_send_request(int csock, struct nbd_request *request);
int nbd_receive_reply(int csock, struct nbd_reply *reply);
-int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
- uint32_t nbdflags, uint8_t *data);
int nbd_client(int fd);
int nbd_disconnect(int fd);
+typedef struct NBDExport NBDExport;
+
+NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
+ off_t size, uint32_t nbdflags);
+void nbd_export_close(NBDExport *exp);
+int nbd_trip(NBDExport *exp, int csock);
+
#endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index befbbfd..9fa3979 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -34,6 +34,7 @@
#define SOCKET_PATH "/var/lock/qemu-nbd-%s"
+static NBDExport *exp;
static int verbose;
static void usage(const char *name)
@@ -188,7 +189,7 @@ int main(int argc, char **argv)
int port = NBD_DEFAULT_PORT;
struct sockaddr_in addr;
socklen_t addr_len = sizeof(addr);
- off_t fd_size;
+ off_t fd_size = -1;
char *device = NULL;
char *socket = NULL;
char sockpath[128];
@@ -219,7 +220,6 @@ int main(int argc, char **argv)
int partition = -1;
int ret;
int shared = 1;
- uint8_t *data;
fd_set fds;
int *sharing_fds;
int fd;
@@ -338,12 +338,12 @@ int main(int argc, char **argv)
err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
}
- fd_size = bs->total_sectors * 512;
-
if (partition != -1 &&
find_partition(bs, partition, &dev_offset, &fd_size))
err(EXIT_FAILURE, "Could not find partition %d", partition);
+ exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags);
+
if (device) {
pid_t pid;
int sock;
@@ -440,10 +440,6 @@ int main(int argc, char **argv)
max_fd = sharing_fds[0];
nb_fds++;
- data = qemu_blockalign(bs, NBD_BUFFER_SIZE);
- if (data == NULL)
- errx(EXIT_FAILURE, "Cannot allocate data buffer");
-
do {
FD_ZERO(&fds);
@@ -458,8 +454,7 @@ int main(int argc, char **argv)
ret--;
for (i = 1; i < nb_fds && ret; i++) {
if (FD_ISSET(sharing_fds[i], &fds)) {
- if (nbd_trip(bs, sharing_fds[i], fd_size, dev_offset,
- nbdflags, data) != 0) {
+ if (nbd_trip(exp, sharing_fds[i]) != 0) {
close(sharing_fds[i]);
nb_fds--;
sharing_fds[i] = sharing_fds[nb_fds];
@@ -483,10 +478,9 @@ int main(int argc, char **argv)
}
}
} while (persistent || nb_fds > 1);
- qemu_vfree(data);
close(sharing_fds[0]);
- bdrv_close(bs);
+ nbd_export_close(exp);
g_free(sharing_fds);
if (socket)
unlink(socket);
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 09/15] qemu-nbd: introduce NBDRequest
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (7 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 08/15] qemu-nbd: introduce NBDExport Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 10/15] link the main loop and its dependencies into the tools Paolo Bonzini
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Move the buffer from NBDExport to a new structure, so that it will be
possible to have multiple in-flight requests for the same export
(and for the same client too---we get that for free).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/nbd.c b/nbd.c
index 00764da..ae7f5ab 100644
--- a/nbd.c
+++ b/nbd.c
@@ -36,6 +36,7 @@
#endif
#include "qemu_socket.h"
+#include "qemu-queue.h"
//#define DEBUG_NBD
@@ -586,29 +587,60 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
return 0;
}
+typedef struct NBDRequest NBDRequest;
+
+struct NBDRequest {
+ QSIMPLEQ_ENTRY(NBDRequest) entry;
+ uint8_t *data;
+};
+
struct NBDExport {
BlockDriverState *bs;
off_t dev_offset;
off_t size;
- uint8_t *data;
uint32_t nbdflags;
+ QSIMPLEQ_HEAD(, NBDRequest) requests;
};
+static NBDRequest *nbd_request_get(NBDExport *exp)
+{
+ NBDRequest *req;
+ if (QSIMPLEQ_EMPTY(&exp->requests)) {
+ req = g_malloc0(sizeof(NBDRequest));
+ req->data = qemu_blockalign(exp->bs, NBD_BUFFER_SIZE);
+ } else {
+ req = QSIMPLEQ_FIRST(&exp->requests);
+ QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
+ }
+ return req;
+}
+
+static void nbd_request_put(NBDExport *exp, NBDRequest *req)
+{
+ QSIMPLEQ_INSERT_HEAD(&exp->requests, req, entry);
+}
+
NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
off_t size, uint32_t nbdflags)
{
NBDExport *exp = g_malloc0(sizeof(NBDExport));
+ QSIMPLEQ_INIT(&exp->requests);
exp->bs = bs;
exp->dev_offset = dev_offset;
exp->nbdflags = nbdflags;
exp->size = size == -1 ? exp->bs->total_sectors * 512 : size;
- exp->data = qemu_blockalign(exp->bs, NBD_BUFFER_SIZE);
return exp;
}
void nbd_export_close(NBDExport *exp)
{
- qemu_vfree(exp->data);
+ while (!QSIMPLEQ_EMPTY(&exp->requests)) {
+ NBDRequest *first = QSIMPLEQ_FIRST(&exp->requests);
+ QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
+ qemu_vfree(first->data);
+ g_free(first);
+ }
+
bdrv_close(exp->bs);
g_free(exp);
}
@@ -684,15 +716,17 @@ out:
int nbd_trip(NBDExport *exp, int csock)
{
+ NBDRequest *req = nbd_request_get(exp);
struct nbd_request request;
struct nbd_reply reply;
+ int rc = -1;
int ret;
TRACE("Reading request.");
- ret = nbd_do_receive_request(csock, &request, exp->data);
+ ret = nbd_do_receive_request(csock, &request, req->data);
if (ret == -EIO) {
- return -1;
+ goto out;
}
reply.handle = request.handle;
@@ -717,7 +751,7 @@ int nbd_trip(NBDExport *exp, int csock)
TRACE("Request type is READ");
ret = bdrv_read(exp->bs, (request.from + exp->dev_offset) / 512,
- exp->data, request.len / 512);
+ req->data, request.len / 512);
if (ret < 0) {
LOG("reading from file failed");
reply.error = -ret;
@@ -725,8 +759,8 @@ int nbd_trip(NBDExport *exp, int csock)
}
TRACE("Read %u byte(s)", request.len);
- if (nbd_do_send_reply(csock, &reply, exp->data, request.len) < 0)
- return -1;
+ if (nbd_do_send_reply(csock, &reply, req->data, request.len) < 0)
+ goto out;
break;
case NBD_CMD_WRITE:
TRACE("Request type is WRITE");
@@ -740,7 +774,7 @@ int nbd_trip(NBDExport *exp, int csock)
TRACE("Writing to device");
ret = bdrv_write(exp->bs, (request.from + exp->dev_offset) / 512,
- exp->data, request.len / 512);
+ req->data, request.len / 512);
if (ret < 0) {
LOG("writing to file failed");
reply.error = -ret;
@@ -757,7 +791,7 @@ int nbd_trip(NBDExport *exp, int csock)
}
if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
- return -1;
+ goto out;
break;
case NBD_CMD_DISC:
TRACE("Request type is DISCONNECT");
@@ -773,7 +807,7 @@ int nbd_trip(NBDExport *exp, int csock)
}
if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
- return -1;
+ goto out;
break;
case NBD_CMD_TRIM:
TRACE("Request type is TRIM");
@@ -784,7 +818,7 @@ int nbd_trip(NBDExport *exp, int csock)
reply.error = -ret;
}
if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
- return -1;
+ goto out;
break;
default:
LOG("invalid request type (%u) received", request.type);
@@ -792,11 +826,14 @@ int nbd_trip(NBDExport *exp, int csock)
reply.error = -EINVAL;
error_reply:
if (nbd_do_send_reply(csock, &reply, NULL, 0) == -1)
- return -1;
+ goto out;
break;
}
TRACE("Request/Reply complete");
- return 0;
+ rc = 0;
+out:
+ nbd_request_put(exp, req);
+ return rc;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 10/15] link the main loop and its dependencies into the tools
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (8 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 09/15] qemu-nbd: introduce NBDRequest Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 11/15] qemu-nbd: use common main loop Paolo Bonzini
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Using the main loop code from QEMU enables tools to operate fully
asynchronously. Advantages include better Windows portability (for some
definition of portability) over glib's.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile | 4 +++-
os-posix.c | 42 ------------------------------------------
os-win32.c | 5 -----
oslib-posix.c | 42 ++++++++++++++++++++++++++++++++++++++++++
oslib-win32.c | 5 +++++
qemu-tool.c | 42 +++++++++++++++++++++++-------------------
6 files changed, 73 insertions(+), 67 deletions(-)
diff --git a/Makefile b/Makefile
index 5723108..4adf082 100644
--- a/Makefile
+++ b/Makefile
@@ -146,7 +146,9 @@ qemu-img.o: qemu-img-cmds.h
qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-ga.o: $(GENERATED_HEADERS)
tools-obj-y = qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) \
- $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
+ $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o \
+ qemu-timer.o main-loop.o notify.o iohandler.o
+tools-obj-$(CONFIG_POSIX) += compatfd.o
qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)
diff --git a/os-posix.c b/os-posix.c
index dbf3b24..c0188b3 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,11 +42,6 @@
#ifdef CONFIG_LINUX
#include <sys/prctl.h>
-#include <sys/syscall.h>
-#endif
-
-#ifdef CONFIG_EVENTFD
-#include <sys/eventfd.h>
#endif
static struct passwd *user_pwd;
@@ -333,34 +328,6 @@ void os_set_line_buffering(void)
setvbuf(stdout, NULL, _IOLBF, 0);
}
-/*
- * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
- */
-int qemu_eventfd(int fds[2])
-{
-#ifdef CONFIG_EVENTFD
- int ret;
-
- ret = eventfd(0, 0);
- if (ret >= 0) {
- fds[0] = ret;
- qemu_set_cloexec(ret);
- if ((fds[1] = dup(ret)) == -1) {
- close(ret);
- return -1;
- }
- qemu_set_cloexec(fds[1]);
- return 0;
- }
-
- if (errno != ENOSYS) {
- return -1;
- }
-#endif
-
- return qemu_pipe(fds);
-}
-
int qemu_create_pidfile(const char *filename)
{
char buffer[128];
@@ -381,12 +348,3 @@ int qemu_create_pidfile(const char *filename)
return 0;
}
-
-int qemu_get_thread_id(void)
-{
-#if defined (__linux__)
- return syscall(SYS_gettid);
-#else
- return getpid();
-#endif
-}
diff --git a/os-win32.c b/os-win32.c
index 7909401..30a96ef 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -143,8 +143,3 @@ int qemu_create_pidfile(const char *filename)
}
return 0;
}
-
-int qemu_get_thread_id(void)
-{
- return GetCurrentThreadId();
-}
diff --git a/oslib-posix.c b/oslib-posix.c
index dbc8ee8..29c8647 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -47,7 +47,21 @@ extern int daemon(int, int);
#include "trace.h"
#include "qemu_socket.h"
+#ifdef CONFIG_LINUX
+#include <sys/syscall.h>
+#endif
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#endif
+int qemu_get_thread_id(void)
+{
+#if defined(__linux__)
+ return syscall(SYS_gettid);
+#else
+ return getpid();
+#endif
+}
int qemu_daemon(int nochdir, int noclose)
{
@@ -146,6 +160,34 @@ int qemu_pipe(int pipefd[2])
return ret;
}
+/*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+#ifdef CONFIG_EVENTFD
+ int ret;
+
+ ret = eventfd(0, 0);
+ if (ret >= 0) {
+ fds[0] = ret;
+ fds[1] = dup(ret);
+ if (fds[1] == -1) {
+ close(ret);
+ return -1;
+ }
+ qemu_set_cloexec(ret);
+ qemu_set_cloexec(fds[1]);
+ return 0;
+ }
+ if (errno != ENOSYS) {
+ return -1;
+ }
+#endif
+
+ return qemu_pipe(fds);
+}
+
int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
int flags)
{
diff --git a/oslib-win32.c b/oslib-win32.c
index 5e3de7d..ce3021e 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -118,3 +118,8 @@ int qemu_gettimeofday(qemu_timeval *tp)
Do not set errno on error. */
return 0;
}
+
+int qemu_get_thread_id(void)
+{
+ return GetCurrentThreadId();
+}
diff --git a/qemu-tool.c b/qemu-tool.c
index e9f7fe1..aafa76a 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -15,12 +15,12 @@
#include "monitor.h"
#include "qemu-timer.h"
#include "qemu-log.h"
+#include "main-loop.h"
+#include "qemu_socket.h"
+#include "slirp/libslirp.h"
#include <sys/time.h>
-QEMUClock *rt_clock;
-QEMUClock *vm_clock;
-
FILE *logfile;
struct QEMUBH
@@ -56,39 +56,43 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
{
}
-int qemu_set_fd_handler2(int fd,
- IOCanReadHandler *fd_read_poll,
- IOHandler *fd_read,
- IOHandler *fd_write,
- void *opaque)
+int64 cpu_get_clock(void)
{
- return 0;
+ abort();
}
-void qemu_notify_event(void)
+int64 cpu_get_icount(void)
{
+ abort();
}
-QEMUTimer *qemu_new_timer(QEMUClock *clock, int scale,
- QEMUTimerCB *cb, void *opaque)
+void qemu_mutex_lock_iothread(void)
{
- return g_malloc(1);
}
-void qemu_free_timer(QEMUTimer *ts)
+void qemu_mutex_unlock_iothread(void)
{
- g_free(ts);
}
-void qemu_del_timer(QEMUTimer *ts)
+int use_icount;
+
+void qemu_clock_warp(QEMUClock *clock)
{
}
-void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
+static void __attribute__((constructor)) init_main_loop(void)
{
+ init_clocks();
+ init_timer_alarm();
+ qemu_clock_enable(vm_clock, false);
}
-int64_t qemu_get_clock_ns(QEMUClock *clock)
+void slirp_select_fill(int *pnfds, fd_set *readfds,
+ fd_set *writefds, fd_set *xfds)
+{
+}
+
+void slirp_select_poll(fd_set *readfds, fd_set *writefds,
+ fd_set *xfds, int select_error)
{
- return 0;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 11/15] qemu-nbd: use common main loop
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (9 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 10/15] link the main loop and its dependencies into the tools Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 12/15] qemu-nbd: move client handling to nbd.c Paolo Bonzini
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Using a single main loop for sockets will help yielding from the socket
coroutine back to the main loop, and later reentering it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-nbd.c | 101 ++++++++++++++++++++++++++++--------------------------------
1 files changed, 47 insertions(+), 54 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9fa3979..7896e9b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -36,6 +36,8 @@
static NBDExport *exp;
static int verbose;
+static int shared = 1;
+static int nb_fds;
static void usage(const char *name)
{
@@ -179,6 +181,35 @@ static void show_parts(const char *device)
}
}
+static int nbd_can_accept(void *opaque)
+{
+ return nb_fds < shared;
+}
+
+static void nbd_read(void *opaque)
+{
+ int fd = (uintptr_t) opaque;
+
+ if (nbd_trip(&exp, fd) != 0) {
+ qemu_set_fd_handler2(fd, NULL, NULL, NULL, NULL);
+ close(fd);
+ nb_fds--;
+ }
+}
+
+static void nbd_accept(void *opaque)
+{
+ int server_fd = (uintptr_t) opaque;
+ struct sockaddr_in addr;
+ socklen_t addr_len = sizeof(addr);
+
+ int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+ if (fd != -1 && nbd_negotiate(fd, exp.size, exp.nbdflags) != -1) {
+ qemu_set_fd_handler2(fd, NULL, nbd_read, NULL, (void *) (intptr_t) fd);
+ nb_fds++;
+ }
+}
+
int main(int argc, char **argv)
{
BlockDriverState *bs;
@@ -187,8 +218,6 @@ int main(int argc, char **argv)
bool disconnect = false;
const char *bindto = "0.0.0.0";
int port = NBD_DEFAULT_PORT;
- struct sockaddr_in addr;
- socklen_t addr_len = sizeof(addr);
off_t fd_size = -1;
char *device = NULL;
char *socket = NULL;
@@ -219,13 +248,8 @@ int main(int argc, char **argv)
int flags = BDRV_O_RDWR;
int partition = -1;
int ret;
- int shared = 1;
fd_set fds;
- int *sharing_fds;
int fd;
- int i;
- int nb_fds = 0;
- int max_fd;
int persistent = 0;
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
@@ -427,61 +451,30 @@ int main(int argc, char **argv)
/* children */
}
- sharing_fds = g_malloc((shared + 1) * sizeof(int));
-
if (socket) {
- sharing_fds[0] = unix_socket_incoming(socket);
+ fd = unix_socket_incoming(socket);
} else {
- sharing_fds[0] = tcp_socket_incoming(bindto, port);
+ fd = tcp_socket_incoming(bindto, port);
}
- if (sharing_fds[0] == -1)
+ if (fd == -1) {
return 1;
- max_fd = sharing_fds[0];
- nb_fds++;
-
- do {
-
- FD_ZERO(&fds);
- for (i = 0; i < nb_fds; i++)
- FD_SET(sharing_fds[i], &fds);
-
- ret = select(max_fd + 1, &fds, NULL, NULL, NULL);
- if (ret == -1)
- break;
+ }
- if (FD_ISSET(sharing_fds[0], &fds))
- ret--;
- for (i = 1; i < nb_fds && ret; i++) {
- if (FD_ISSET(sharing_fds[i], &fds)) {
- if (nbd_trip(exp, sharing_fds[i]) != 0) {
- close(sharing_fds[i]);
- nb_fds--;
- sharing_fds[i] = sharing_fds[nb_fds];
- i--;
- }
- ret--;
- }
- }
- /* new connection ? */
- if (FD_ISSET(sharing_fds[0], &fds)) {
- if (nb_fds < shared + 1) {
- sharing_fds[nb_fds] = accept(sharing_fds[0],
- (struct sockaddr *)&addr,
- &addr_len);
- if (sharing_fds[nb_fds] != -1 &&
- nbd_negotiate(sharing_fds[nb_fds], fd_size, nbdflags) != -1) {
- if (sharing_fds[nb_fds] > max_fd)
- max_fd = sharing_fds[nb_fds];
- nb_fds++;
- }
- }
- }
- } while (persistent || nb_fds > 1);
+ qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
+ (void *)(uintptr_t)fd);
+
+ /* Wait for the first incoming connection. */
+ FD_ZERO(&fds);
+ FD_SET(fd, &fds);
+ ret = select(fd + 1, &fds, NULL, NULL, NULL);
+ if (ret != -1) {
+ do {
+ main_loop_wait(false);
+ } while (persistent || nb_fds > 0);
+ }
- close(sharing_fds[0]);
nbd_export_close(exp);
- g_free(sharing_fds);
if (socket)
unlink(socket);
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 12/15] qemu-nbd: move client handling to nbd.c
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (10 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 11/15] qemu-nbd: use common main loop Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 13/15] qemu-nbd: add client pointer to NBDRequest Paolo Bonzini
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
This patch sets up the fd handler in nbd.c instead of qemu-nbd.c. It
introduces NBDClient, which wraps the arguments to nbd_trip in a single
structure, so that we can add a notifier to it. This way, qemu-nbd can
know about disconnections.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
nbd.h | 4 ++-
qemu-nbd.c | 13 ++---------
3 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/nbd.c b/nbd.c
index ae7f5ab..16c4c27 100644
--- a/nbd.c
+++ b/nbd.c
@@ -602,6 +602,37 @@ struct NBDExport {
QSIMPLEQ_HEAD(, NBDRequest) requests;
};
+struct NBDClient {
+ int refcount;
+ void (*close)(NBDClient *client);
+
+ NBDExport *exp;
+ int sock;
+};
+
+static void nbd_client_get(NBDClient *client)
+{
+ client->refcount++;
+}
+
+static void nbd_client_put(NBDClient *client)
+{
+ if (--client->refcount == 0) {
+ g_free(client);
+ }
+}
+
+static void nbd_client_close(NBDClient *client)
+{
+ qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
+ close(client->sock);
+ client->sock = -1;
+ if (client->close) {
+ client->close(client);
+ }
+ nbd_client_put(client);
+}
+
static NBDRequest *nbd_request_get(NBDExport *exp)
{
NBDRequest *req;
@@ -714,9 +745,11 @@ out:
return rc;
}
-int nbd_trip(NBDExport *exp, int csock)
+static int nbd_trip(NBDClient *client)
{
+ NBDExport *exp = client->exp;
NBDRequest *req = nbd_request_get(exp);
+ int csock = client->sock;
struct nbd_request request;
struct nbd_reply reply;
int rc = -1;
@@ -837,3 +870,31 @@ out:
nbd_request_put(exp, req);
return rc;
}
+
+static void nbd_read(void *opaque)
+{
+ NBDClient *client = opaque;
+
+ nbd_client_get(client);
+ if (nbd_trip(client) != 0) {
+ nbd_client_close(client);
+ }
+
+ nbd_client_put(client);
+}
+
+NBDClient *nbd_client_new(NBDExport *exp, int csock,
+ void (*close)(NBDClient *))
+{
+ NBDClient *client;
+ if (nbd_negotiate(csock, exp->size, exp->nbdflags) == -1) {
+ return NULL;
+ }
+ client = g_malloc0(sizeof(NBDClient));
+ client->refcount = 1;
+ client->exp = exp;
+ client->sock = csock;
+ client->close = close;
+ qemu_set_fd_handler2(csock, NULL, nbd_read, NULL, client);
+ return client;
+}
diff --git a/nbd.h b/nbd.h
index d368156..383c693 100644
--- a/nbd.h
+++ b/nbd.h
@@ -77,10 +77,12 @@ int nbd_client(int fd);
int nbd_disconnect(int fd);
typedef struct NBDExport NBDExport;
+typedef struct NBDClient NBDClient;
NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
off_t size, uint32_t nbdflags);
void nbd_export_close(NBDExport *exp);
-int nbd_trip(NBDExport *exp, int csock);
+NBDClient *nbd_client_new(NBDExport *exp, int csock,
+ void (*close)(NBDClient *));
#endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7896e9b..2b29011 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -186,15 +186,9 @@ static int nbd_can_accept(void *opaque)
return nb_fds < shared;
}
-static void nbd_read(void *opaque)
+static void nbd_client_closed(NBDClient *client)
{
- int fd = (uintptr_t) opaque;
-
- if (nbd_trip(&exp, fd) != 0) {
- qemu_set_fd_handler2(fd, NULL, NULL, NULL, NULL);
- close(fd);
- nb_fds--;
- }
+ nb_fds--;
}
static void nbd_accept(void *opaque)
@@ -204,8 +198,7 @@ static void nbd_accept(void *opaque)
socklen_t addr_len = sizeof(addr);
int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
- if (fd != -1 && nbd_negotiate(fd, exp.size, exp.nbdflags) != -1) {
- qemu_set_fd_handler2(fd, NULL, nbd_read, NULL, (void *) (intptr_t) fd);
+ if (fd != -1 && nbd_client_new(exp, fd, nbd_client_closed)) {
nb_fds++;
}
}
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 13/15] qemu-nbd: add client pointer to NBDRequest
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (11 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 12/15] qemu-nbd: move client handling to nbd.c Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 14/15] qemu-nbd: asynchronous operation Paolo Bonzini
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
By attaching a client to an NBDRequest, we can avoid passing around the
socket descriptor and data buffer.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 48 +++++++++++++++++++++++++++---------------------
1 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/nbd.c b/nbd.c
index 16c4c27..e016175 100644
--- a/nbd.c
+++ b/nbd.c
@@ -591,6 +591,7 @@ typedef struct NBDRequest NBDRequest;
struct NBDRequest {
QSIMPLEQ_ENTRY(NBDRequest) entry;
+ NBDClient *client;
uint8_t *data;
};
@@ -633,9 +634,11 @@ static void nbd_client_close(NBDClient *client)
nbd_client_put(client);
}
-static NBDRequest *nbd_request_get(NBDExport *exp)
+static NBDRequest *nbd_request_get(NBDClient *client)
{
NBDRequest *req;
+ NBDExport *exp = client->exp;
+
if (QSIMPLEQ_EMPTY(&exp->requests)) {
req = g_malloc0(sizeof(NBDRequest));
req->data = qemu_blockalign(exp->bs, NBD_BUFFER_SIZE);
@@ -643,12 +646,16 @@ static NBDRequest *nbd_request_get(NBDExport *exp)
req = QSIMPLEQ_FIRST(&exp->requests);
QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
}
+ nbd_client_get(client);
+ req->client = client;
return req;
}
-static void nbd_request_put(NBDExport *exp, NBDRequest *req)
+static void nbd_request_put(NBDRequest *req)
{
- QSIMPLEQ_INSERT_HEAD(&exp->requests, req, entry);
+ NBDClient *client = req->client;
+ QSIMPLEQ_INSERT_HEAD(&client->exp->requests, req, entry);
+ nbd_client_put(client);
}
NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
@@ -676,9 +683,11 @@ void nbd_export_close(NBDExport *exp)
g_free(exp);
}
-static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
- uint8_t *data, int len)
+static int nbd_do_send_reply(NBDRequest *req, struct nbd_reply *reply,
+ int len)
{
+ NBDClient *client = req->client;
+ int csock = client->sock;
int rc, ret;
if (!len) {
@@ -690,7 +699,7 @@ static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
socket_set_cork(csock, 1);
rc = nbd_send_reply(csock, reply);
if (rc != -1) {
- ret = write_sync(csock, data, len);
+ ret = write_sync(csock, req->data, len);
if (ret != len) {
errno = EIO;
rc = -1;
@@ -704,9 +713,10 @@ static int nbd_do_send_reply(int csock, struct nbd_reply *reply,
return rc;
}
-static int nbd_do_receive_request(int csock, struct nbd_request *request,
- uint8_t *data)
+static int nbd_do_receive_request(NBDRequest *req, struct nbd_request *request)
{
+ NBDClient *client = req->client;
+ int csock = client->sock;
int rc;
if (nbd_receive_request(csock, request) == -1) {
@@ -733,7 +743,7 @@ static int nbd_do_receive_request(int csock, struct nbd_request *request,
if ((request->type & NBD_CMD_MASK_COMMAND) == NBD_CMD_WRITE) {
TRACE("Reading %u byte(s)", request->len);
- if (read_sync(csock, data, request->len) != request->len) {
+ if (read_sync(csock, req->data, request->len) != request->len) {
LOG("reading from socket failed");
rc = -EIO;
goto out;
@@ -747,9 +757,8 @@ out:
static int nbd_trip(NBDClient *client)
{
+ NBDRequest *req = nbd_request_get(client);
NBDExport *exp = client->exp;
- NBDRequest *req = nbd_request_get(exp);
- int csock = client->sock;
struct nbd_request request;
struct nbd_reply reply;
int rc = -1;
@@ -757,7 +766,7 @@ static int nbd_trip(NBDClient *client)
TRACE("Reading request.");
- ret = nbd_do_receive_request(csock, &request, req->data);
+ ret = nbd_do_receive_request(req, &request);
if (ret == -EIO) {
goto out;
}
@@ -792,7 +801,7 @@ static int nbd_trip(NBDClient *client)
}
TRACE("Read %u byte(s)", request.len);
- if (nbd_do_send_reply(csock, &reply, req->data, request.len) < 0)
+ if (nbd_do_send_reply(req, &reply, request.len) < 0)
goto out;
break;
case NBD_CMD_WRITE:
@@ -823,7 +832,7 @@ static int nbd_trip(NBDClient *client)
}
}
- if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
+ if (nbd_do_send_reply(req, &reply, 0) < 0)
goto out;
break;
case NBD_CMD_DISC:
@@ -839,7 +848,7 @@ static int nbd_trip(NBDClient *client)
reply.error = -ret;
}
- if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
+ if (nbd_do_send_reply(req, &reply, 0) < 0)
goto out;
break;
case NBD_CMD_TRIM:
@@ -850,7 +859,7 @@ static int nbd_trip(NBDClient *client)
LOG("discard failed");
reply.error = -ret;
}
- if (nbd_do_send_reply(csock, &reply, NULL, 0) < 0)
+ if (nbd_do_send_reply(req, &reply, 0) < 0)
goto out;
break;
default:
@@ -858,7 +867,7 @@ static int nbd_trip(NBDClient *client)
invalid_request:
reply.error = -EINVAL;
error_reply:
- if (nbd_do_send_reply(csock, &reply, NULL, 0) == -1)
+ if (nbd_do_send_reply(req, &reply, 0) == -1)
goto out;
break;
}
@@ -867,7 +876,7 @@ static int nbd_trip(NBDClient *client)
rc = 0;
out:
- nbd_request_put(exp, req);
+ nbd_request_put(req);
return rc;
}
@@ -875,12 +884,9 @@ static void nbd_read(void *opaque)
{
NBDClient *client = opaque;
- nbd_client_get(client);
if (nbd_trip(client) != 0) {
nbd_client_close(client);
}
-
- nbd_client_put(client);
}
NBDClient *nbd_client_new(NBDExport *exp, int csock,
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 14/15] qemu-nbd: asynchronous operation
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (12 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 13/15] qemu-nbd: add client pointer to NBDRequest Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-10 9:37 ` [Qemu-devel] [PATCH 15/15] qemu-nbd: throttle requests Paolo Bonzini
2011-10-11 8:19 ` [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Using coroutines enable asynchronous operation on both the network and
the block side. Network can be owned by two coroutines at the same time,
one writing and one reading. On the send side, mutual exclusion is
guaranteed by a CoMutex. On the receive side, mutual exclusion is
guaranteed because new coroutines immediately start receiving data,
and no new coroutines are created as long as the previous one is receiving.
Between receive and send, qemu-nbd can have an arbitrary number of
in-flight block transfers. Throttling is implemented by the next
patch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/nbd.c b/nbd.c
index e016175..01d3a85 100644
--- a/nbd.c
+++ b/nbd.c
@@ -20,6 +20,8 @@
#include "block.h"
#include "block_int.h"
+#include "qemu-coroutine.h"
+
#include <errno.h>
#include <string.h>
#ifndef _WIN32
@@ -609,6 +611,11 @@ struct NBDClient {
NBDExport *exp;
int sock;
+
+ Coroutine *recv_coroutine;
+
+ CoMutex send_lock;
+ Coroutine *send_coroutine;
};
static void nbd_client_get(NBDClient *client)
@@ -683,13 +690,20 @@ void nbd_export_close(NBDExport *exp)
g_free(exp);
}
-static int nbd_do_send_reply(NBDRequest *req, struct nbd_reply *reply,
+static void nbd_read(void *opaque);
+static void nbd_restart_write(void *opaque);
+
+static int nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
int len)
{
NBDClient *client = req->client;
int csock = client->sock;
int rc, ret;
+ qemu_co_mutex_lock(&client->send_lock);
+ qemu_set_fd_handler2(csock, NULL, nbd_read, nbd_restart_write, client);
+ client->send_coroutine = qemu_coroutine_self();
+
if (!len) {
rc = nbd_send_reply(csock, reply);
if (rc == -1) {
@@ -699,7 +713,7 @@ static int nbd_do_send_reply(NBDRequest *req, struct nbd_reply *reply,
socket_set_cork(csock, 1);
rc = nbd_send_reply(csock, reply);
if (rc != -1) {
- ret = write_sync(csock, req->data, len);
+ ret = qemu_co_send(csock, req->data, len);
if (ret != len) {
errno = EIO;
rc = -1;
@@ -710,15 +724,20 @@ static int nbd_do_send_reply(NBDRequest *req, struct nbd_reply *reply,
}
socket_set_cork(csock, 0);
}
+
+ client->send_coroutine = NULL;
+ qemu_set_fd_handler2(csock, NULL, nbd_read, NULL, client);
+ qemu_co_mutex_unlock(&client->send_lock);
return rc;
}
-static int nbd_do_receive_request(NBDRequest *req, struct nbd_request *request)
+static int nbd_co_receive_request(NBDRequest *req, struct nbd_request *request)
{
NBDClient *client = req->client;
int csock = client->sock;
int rc;
+ client->recv_coroutine = qemu_coroutine_self();
if (nbd_receive_request(csock, request) == -1) {
rc = -EIO;
goto out;
@@ -743,7 +762,7 @@ static int nbd_do_receive_request(NBDRequest *req, struct nbd_request *request)
if ((request->type & NBD_CMD_MASK_COMMAND) == NBD_CMD_WRITE) {
TRACE("Reading %u byte(s)", request->len);
- if (read_sync(csock, req->data, request->len) != request->len) {
+ if (qemu_co_recv(csock, req->data, request->len) != request->len) {
LOG("reading from socket failed");
rc = -EIO;
goto out;
@@ -752,21 +771,22 @@ static int nbd_do_receive_request(NBDRequest *req, struct nbd_request *request)
rc = 0;
out:
+ client->recv_coroutine = NULL;
return rc;
}
-static int nbd_trip(NBDClient *client)
+static void nbd_trip(void *opaque)
{
+ NBDClient *client = opaque;
NBDRequest *req = nbd_request_get(client);
NBDExport *exp = client->exp;
struct nbd_request request;
struct nbd_reply reply;
- int rc = -1;
int ret;
TRACE("Reading request.");
- ret = nbd_do_receive_request(req, &request);
+ ret = nbd_co_receive_request(req, &request);
if (ret == -EIO) {
goto out;
}
@@ -801,7 +821,7 @@ static int nbd_trip(NBDClient *client)
}
TRACE("Read %u byte(s)", request.len);
- if (nbd_do_send_reply(req, &reply, request.len) < 0)
+ if (nbd_co_send_reply(req, &reply, request.len) < 0)
goto out;
break;
case NBD_CMD_WRITE:
@@ -832,13 +852,13 @@ static int nbd_trip(NBDClient *client)
}
}
- if (nbd_do_send_reply(req, &reply, 0) < 0)
+ if (nbd_co_send_reply(req, &reply, 0) < 0)
goto out;
break;
case NBD_CMD_DISC:
TRACE("Request type is DISCONNECT");
errno = 0;
- return 1;
+ goto out;
case NBD_CMD_FLUSH:
TRACE("Request type is FLUSH");
@@ -848,7 +868,7 @@ static int nbd_trip(NBDClient *client)
reply.error = -ret;
}
- if (nbd_do_send_reply(req, &reply, 0) < 0)
+ if (nbd_co_send_reply(req, &reply, 0) < 0)
goto out;
break;
case NBD_CMD_TRIM:
@@ -859,7 +879,7 @@ static int nbd_trip(NBDClient *client)
LOG("discard failed");
reply.error = -ret;
}
- if (nbd_do_send_reply(req, &reply, 0) < 0)
+ if (nbd_co_send_reply(req, &reply, 0) < 0)
goto out;
break;
default:
@@ -867,28 +887,39 @@ static int nbd_trip(NBDClient *client)
invalid_request:
reply.error = -EINVAL;
error_reply:
- if (nbd_do_send_reply(req, &reply, 0) == -1)
+ if (nbd_co_send_reply(req, &reply, 0) == -1)
goto out;
break;
}
TRACE("Request/Reply complete");
- rc = 0;
+ nbd_request_put(req);
+ return;
+
out:
nbd_request_put(req);
- return rc;
+ nbd_client_close(client);
}
static void nbd_read(void *opaque)
{
NBDClient *client = opaque;
- if (nbd_trip(client) != 0) {
- nbd_client_close(client);
+ if (client->recv_coroutine) {
+ qemu_coroutine_enter(client->recv_coroutine, NULL);
+ } else {
+ qemu_coroutine_enter(qemu_coroutine_create(nbd_trip), client);
}
}
+static void nbd_restart_write(void *opaque)
+{
+ NBDClient *client = opaque;
+
+ qemu_coroutine_enter(client->send_coroutine, NULL);
+}
+
NBDClient *nbd_client_new(NBDExport *exp, int csock,
void (*close)(NBDClient *))
{
@@ -901,6 +932,7 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
client->exp = exp;
client->sock = csock;
client->close = close;
+ qemu_co_mutex_init(&client->send_lock);
qemu_set_fd_handler2(csock, NULL, nbd_read, NULL, client);
return client;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 15/15] qemu-nbd: throttle requests
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (13 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 14/15] qemu-nbd: asynchronous operation Paolo Bonzini
@ 2011-10-10 9:37 ` Paolo Bonzini
2011-10-11 8:19 ` [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-10 9:37 UTC (permalink / raw)
To: qemu-devel
Limiting the number of in-flight requests is implemented very simply
with a can_read callback. It does not require a semaphore, unlike the
client side in block/nbd.c, because we can throttle directly the creation
of coroutines. The client side can have a coroutine created at any time
when an I/O request is made.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd.c | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/nbd.c b/nbd.c
index 01d3a85..1869212 100644
--- a/nbd.c
+++ b/nbd.c
@@ -589,6 +589,8 @@ static int nbd_send_reply(int csock, struct nbd_reply *reply)
return 0;
}
+#define MAX_NBD_REQUESTS 16
+
typedef struct NBDRequest NBDRequest;
struct NBDRequest {
@@ -616,6 +618,8 @@ struct NBDClient {
CoMutex send_lock;
Coroutine *send_coroutine;
+
+ int nb_requests;
};
static void nbd_client_get(NBDClient *client)
@@ -646,6 +650,9 @@ static NBDRequest *nbd_request_get(NBDClient *client)
NBDRequest *req;
NBDExport *exp = client->exp;
+ assert(client->nb_requests < MAX_NBD_REQUESTS);
+ client->nb_requests++;
+
if (QSIMPLEQ_EMPTY(&exp->requests)) {
req = g_malloc0(sizeof(NBDRequest));
req->data = qemu_blockalign(exp->bs, NBD_BUFFER_SIZE);
@@ -662,6 +669,7 @@ static void nbd_request_put(NBDRequest *req)
{
NBDClient *client = req->client;
QSIMPLEQ_INSERT_HEAD(&client->exp->requests, req, entry);
+ client->nb_requests--;
nbd_client_put(client);
}
@@ -690,6 +698,7 @@ void nbd_export_close(NBDExport *exp)
g_free(exp);
}
+static int nbd_can_read(void *opaque);
static void nbd_read(void *opaque);
static void nbd_restart_write(void *opaque);
@@ -701,7 +710,8 @@ static int nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
int rc, ret;
qemu_co_mutex_lock(&client->send_lock);
- qemu_set_fd_handler2(csock, NULL, nbd_read, nbd_restart_write, client);
+ qemu_set_fd_handler2(csock, nbd_can_read, nbd_read,
+ nbd_restart_write, client);
client->send_coroutine = qemu_coroutine_self();
if (!len) {
@@ -726,7 +736,7 @@ static int nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
}
client->send_coroutine = NULL;
- qemu_set_fd_handler2(csock, NULL, nbd_read, NULL, client);
+ qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
qemu_co_mutex_unlock(&client->send_lock);
return rc;
}
@@ -902,6 +912,13 @@ out:
nbd_client_close(client);
}
+static int nbd_can_read(void *opaque)
+{
+ NBDClient *client = opaque;
+
+ return client->recv_coroutine || client->nb_requests < MAX_NBD_REQUESTS;
+}
+
static void nbd_read(void *opaque)
{
NBDClient *client = opaque;
@@ -933,6 +950,6 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
client->sock = csock;
client->close = close;
qemu_co_mutex_init(&client->send_lock);
- qemu_set_fd_handler2(csock, NULL, nbd_read, NULL, client);
+ qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
return client;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 00/15] NBD server improvements
2011-10-10 9:37 [Qemu-devel] [PATCH 00/15] NBD server improvements Paolo Bonzini
` (14 preceding siblings ...)
2011-10-10 9:37 ` [Qemu-devel] [PATCH 15/15] qemu-nbd: throttle requests Paolo Bonzini
@ 2011-10-11 8:19 ` Paolo Bonzini
15 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2011-10-11 8:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On 10/10/2011 11:37 AM, Paolo Bonzini wrote:
> This series adds asynchronous operation support for the NBD server.
> The first 9 patches are a general refactoring that can be applied now.
> The others require the "main loop in tools" series.
They also need rebasing after the QMP changes. Kevin, is it okay for
you to apply only the first part?
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread