* [Qemu-devel] [PATCH 00/12] RFC: add Spice block device
@ 2013-06-20 17:45 Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include Marc-André Lureau
` (11 more replies)
0 siblings, 12 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:45 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Hi,
The following patch series implement a Spice block device, which
allows the client to redirect a block device using the NBD protocol,
which greatly simplifies the Spice code by reusing an existing
protocol, and allows sharing existing qemu NBD implementation.
This block device driver is a bit special, since it is successfully
initialized with size 0, and once the client is connected (or want to
change block device) it re-opens itself. For this to work, we allow a
block driver to be open with an existing opaque data.
The backend only support read-only device atm (although it shouldn't
be hard to add write support if necessary). Migration hasn't been
tested yet.
Usage with a CDROM drive:
-device ide-cd,drive=cd -drive if=none,id=cd,readonly,file=spicebd:
The associated server and client bits are:
http://lists.freedesktop.org/archives/spice-devel/2013-June/013608.html
http://lists.freedesktop.org/archives/spice-devel/2013-June/013609.html
http://lists.freedesktop.org/archives/spice-devel/2013-June/013610.html
Marc-André Lureau (12):
include: add missing config-host.h include
char: add qemu_chr_fe_event()
nbd: don't change socket block during negotiate
Split nbd block client code
nbd: pass export name as init argument
nbd: make session_close() idempotent
block: save the associated child in BlockDriverState
block: extract make_snapshot() from bdrv_open()
block: add "snapshot.size" option to avoid extra bdrv_open()
block: learn to open a driver with a given opaque
block: allow to call bdrv_open() with an opaque
block: add spice block device backend
block.c | 191 ++++++++++-------
block/Makefile.objs | 3 +-
block/nbd-client.c | 391 ++++++++++++++++++++++++++++++++++
block/nbd-client.h | 51 +++++
block/nbd.c | 394 ++++------------------------------
block/spice.c | 523 ++++++++++++++++++++++++++++++++++++++++++++++
include/block/block_int.h | 1 +
include/sysemu/char.h | 10 +
include/ui/qemu-spice.h | 2 +
nbd.c | 1 -
qemu-char.c | 7 +
spice-qemu-char.c | 10 +
12 files changed, 1151 insertions(+), 433 deletions(-)
create mode 100644 block/nbd-client.c
create mode 100644 block/nbd-client.h
create mode 100644 block/spice.c
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() Marc-André Lureau
` (10 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/ui/qemu-spice.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index eba6d77..a92b2cf 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -18,6 +18,8 @@
#ifndef QEMU_SPICE_H
#define QEMU_SPICE_H
+#include "config-host.h"
+
#ifdef CONFIG_SPICE
#include <spice.h>
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event()
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-21 7:35 ` Gerd Hoffmann
2013-06-20 17:46 ` [Qemu-devel] [PATCH 03/12] nbd: don't change socket block during negotiate Marc-André Lureau
` (9 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/sysemu/char.h | 10 ++++++++++
qemu-char.c | 7 +++++++
spice-qemu-char.c | 10 ++++++++++
3 files changed, 27 insertions(+)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 066c216..eee70fe 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -69,6 +69,7 @@ struct CharDriverState {
void (*chr_accept_input)(struct CharDriverState *chr);
void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
+ void (*chr_fe_event)(struct CharDriverState *chr, int event);
void *opaque;
char *label;
char *filename;
@@ -136,6 +137,15 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo);
void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open);
/**
+ * @qemu_chr_fe_event:
+ *
+ * Send an event from the back end to the front end.
+ *
+ * @event the event to send
+ */
+void qemu_chr_fe_event(CharDriverState *s, int event);
+
+/**
* @qemu_chr_fe_printf:
*
* Write to a character backend using a printf style interface.
diff --git a/qemu-char.c b/qemu-char.c
index 2c3cfe6..14e268e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3317,6 +3317,13 @@ void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open)
}
}
+void qemu_chr_fe_event(struct CharDriverState *chr, int event)
+{
+ if (chr->chr_fe_event) {
+ chr->chr_fe_event(chr, event);
+ }
+}
+
int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
GIOFunc func, void *user_data)
{
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 6d147a7..0d77f77 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -223,6 +223,15 @@ static void spice_chr_set_fe_open(struct CharDriverState *chr, int fe_open)
}
}
+static void spice_chr_fe_event(struct CharDriverState *chr, int event)
+{
+#if SPICE_SERVER_VERSION >= 0x000c02
+ SpiceCharDriver *s = chr->opaque;
+
+ spice_server_port_event(&s->sin, event);
+#endif
+}
+
static void print_allowed_subtypes(void)
{
const char** psubtype;
@@ -256,6 +265,7 @@ static CharDriverState *chr_open(const char *subtype)
chr->chr_close = spice_chr_close;
chr->chr_set_fe_open = spice_chr_set_fe_open;
chr->explicit_be_open = true;
+ chr->chr_fe_event = spice_chr_fe_event;
QLIST_INSERT_HEAD(&spice_chars, s, next);
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 03/12] nbd: don't change socket block during negotiate
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 04/12] Split nbd block client code Marc-André Lureau
` (8 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
The caller might handle non-blocking using coroutine. Leave the choice
to the caller to use a blocking or non-blocking noegotiate.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
nbd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/nbd.c b/nbd.c
index 2606403..2f8c946 100644
--- a/nbd.c
+++ b/nbd.c
@@ -442,7 +442,6 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
TRACE("Receiving negotiation.");
- qemu_set_block(csock);
rc = -EINVAL;
if (read_sync(csock, buf, 8) != 8) {
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 04/12] Split nbd block client code
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (2 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 03/12] nbd: don't change socket block during negotiate Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 05/12] nbd: pass export name as init argument Marc-André Lureau
` (7 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block/Makefile.objs | 2 +-
block/nbd-client.c | 386 +++++++++++++++++++++++++++++++++++++++++++++++++++
block/nbd-client.h | 52 +++++++
block/nbd.c | 387 ++++------------------------------------------------
4 files changed, 469 insertions(+), 358 deletions(-)
create mode 100644 block/nbd-client.c
create mode 100644 block/nbd-client.h
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 2981654..5890b5c 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -10,7 +10,7 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
ifeq ($(CONFIG_POSIX),y)
-block-obj-y += nbd.o sheepdog.o
+block-obj-y += nbd.o nbd-client.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
diff --git a/block/nbd-client.c b/block/nbd-client.c
new file mode 100644
index 0000000..6d5f39c
--- /dev/null
+++ b/block/nbd-client.c
@@ -0,0 +1,386 @@
+/*
+ * QEMU Block driver for NBD
+ *
+ * Copyright (C) 2008 Bull S.A.S.
+ * Author: Laurent Vivier <Laurent.Vivier@bull.net>
+ *
+ * Some parts:
+ * Copyright (C) 2007 Anthony Liguori <anthony@codemonkey.ws>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "nbd-client.h"
+#include "qemu/sockets.h"
+
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
+#define INDEX_TO_HANDLE(bs, index) ((index) ^ ((uint64_t)(intptr_t)bs))
+
+static void nbd_reply_ready(void *opaque)
+{
+ NbdClientSession *s = opaque;
+ uint64_t i;
+ int ret;
+
+ if (s->reply.handle == 0) {
+ /* No reply already in flight. Fetch a header. It is possible
+ * that another thread has done the same thing in parallel, so
+ * the socket is not readable anymore.
+ */
+ ret = nbd_receive_reply(s->sock, &s->reply);
+ if (ret == -EAGAIN) {
+ return;
+ }
+ if (ret < 0) {
+ s->reply.handle = 0;
+ goto fail;
+ }
+ }
+
+ /* There's no need for a mutex on the receive side, because the
+ * handler acts as a synchronization point and ensures that only
+ * one coroutine is called until the reply finishes. */
+ i = HANDLE_TO_INDEX(s, s->reply.handle);
+ if (i >= MAX_NBD_REQUESTS) {
+ goto fail;
+ }
+
+ if (s->recv_coroutine[i]) {
+ qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+ return;
+ }
+
+fail:
+ for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+ if (s->recv_coroutine[i]) {
+ qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+ }
+ }
+}
+
+static void nbd_restart_write(void *opaque)
+{
+ NbdClientSession *s = opaque;
+
+ qemu_coroutine_enter(s->send_coroutine, NULL);
+}
+
+static int nbd_have_request(void *opaque)
+{
+ NbdClientSession *s = opaque;
+
+ return s->in_flight > 0;
+}
+
+static int nbd_co_send_request(NbdClientSession *s,
+ struct nbd_request *request,
+ QEMUIOVector *qiov, int offset)
+{
+ int rc, ret;
+
+ qemu_co_mutex_lock(&s->send_mutex);
+ s->send_coroutine = qemu_coroutine_self();
+ qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
+ nbd_have_request, s);
+ if (qiov) {
+ if (!s->is_unix) {
+ socket_set_cork(s->sock, 1);
+ }
+ rc = nbd_send_request(s->sock, request);
+ if (rc >= 0) {
+ ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
+ offset, request->len);
+ if (ret != request->len) {
+ rc = -EIO;
+ }
+ }
+ if (!s->is_unix) {
+ socket_set_cork(s->sock, 0);
+ }
+ } else {
+ rc = nbd_send_request(s->sock, request);
+ }
+ qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
+ nbd_have_request, s);
+ s->send_coroutine = NULL;
+ qemu_co_mutex_unlock(&s->send_mutex);
+ return rc;
+}
+
+static void nbd_co_receive_reply(NbdClientSession *s,
+ struct nbd_request *request, struct nbd_reply *reply,
+ QEMUIOVector *qiov, int offset)
+{
+ int ret;
+
+ /* Wait until we're woken up by the read handler. TODO: perhaps
+ * peek at the next reply and avoid yielding if it's ours? */
+ qemu_coroutine_yield();
+ *reply = s->reply;
+ if (reply->handle != request->handle) {
+ reply->error = EIO;
+ } else {
+ if (qiov && reply->error == 0) {
+ ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov,
+ offset, request->len);
+ if (ret != request->len) {
+ reply->error = EIO;
+ }
+ }
+
+ /* Tell the read handler to read another header. */
+ s->reply.handle = 0;
+ }
+}
+
+static void nbd_coroutine_end(NbdClientSession *s,
+ struct nbd_request *request)
+{
+ int i = HANDLE_TO_INDEX(s, request->handle);
+ s->recv_coroutine[i] = NULL;
+ if (s->in_flight-- == MAX_NBD_REQUESTS) {
+ qemu_co_mutex_unlock(&s->free_sema);
+ }
+}
+
+static void nbd_coroutine_start(NbdClientSession *s,
+ struct nbd_request *request)
+{
+ int i;
+
+ /* Poor man semaphore. The free_sema is locked when no other request
+ * can be accepted, and unlocked after receiving one reply. */
+ if (s->in_flight >= MAX_NBD_REQUESTS - 1) {
+ qemu_co_mutex_lock(&s->free_sema);
+ assert(s->in_flight < MAX_NBD_REQUESTS);
+ }
+ s->in_flight++;
+
+ for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+ if (s->recv_coroutine[i] == NULL) {
+ s->recv_coroutine[i] = qemu_coroutine_self();
+ break;
+ }
+ }
+
+ assert(i < MAX_NBD_REQUESTS);
+ request->handle = INDEX_TO_HANDLE(s, i);
+}
+
+static int nbd_co_readv_1(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov,
+ int offset)
+{
+ struct nbd_request request;
+ struct nbd_reply reply;
+ ssize_t ret;
+
+ request.type = NBD_CMD_READ;
+ request.from = sector_num * 512;
+ request.len = nb_sectors * 512;
+
+ nbd_coroutine_start(client, &request);
+ ret = nbd_co_send_request(client, &request, NULL, 0);
+ if (ret < 0) {
+ reply.error = -ret;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, qiov, offset);
+ }
+ nbd_coroutine_end(client, &request);
+
+ return -reply.error;
+
+}
+
+static int nbd_co_writev_1(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov,
+ int offset)
+{
+ struct nbd_request request;
+ struct nbd_reply reply;
+ ssize_t ret;
+
+ request.type = NBD_CMD_WRITE;
+ if (!bdrv_enable_write_cache(client->bs) &&
+ (client->nbdflags & NBD_FLAG_SEND_FUA)) {
+ request.type |= NBD_CMD_FLAG_FUA;
+ }
+
+ request.from = sector_num * 512;
+ request.len = nb_sectors * 512;
+
+ nbd_coroutine_start(client, &request);
+ ret = nbd_co_send_request(client, &request, qiov, offset);
+ if (ret < 0) {
+ reply.error = -ret;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+ }
+ nbd_coroutine_end(client, &request);
+
+ return -reply.error;
+}
+
+/* qemu-nbd has a limit of slightly less than 1M per request. Try to
+ * remain aligned to 4K. */
+#define NBD_MAX_SECTORS 2040
+
+int nbd_client_session_co_readv(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
+{
+ int offset = 0;
+ int ret;
+
+ while (nb_sectors > NBD_MAX_SECTORS) {
+ ret = nbd_co_readv_1(client, sector_num, NBD_MAX_SECTORS, qiov, offset);
+ if (ret < 0) {
+ return ret;
+ }
+ offset += NBD_MAX_SECTORS * 512;
+ sector_num += NBD_MAX_SECTORS;
+ nb_sectors -= NBD_MAX_SECTORS;
+ }
+
+ return nbd_co_readv_1(client, sector_num, nb_sectors, qiov, offset);
+}
+
+int nbd_client_session_co_writev(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
+{
+ int offset = 0;
+ int ret;
+
+ while (nb_sectors > NBD_MAX_SECTORS) {
+ ret = nbd_co_writev_1(client, sector_num,
+ NBD_MAX_SECTORS, qiov, offset);
+ if (ret < 0) {
+ return ret;
+ }
+ offset += NBD_MAX_SECTORS * 512;
+ sector_num += NBD_MAX_SECTORS;
+ nb_sectors -= NBD_MAX_SECTORS;
+ }
+
+ return nbd_co_writev_1(client, sector_num, nb_sectors, qiov, offset);
+}
+
+int nbd_client_session_co_flush(NbdClientSession *client)
+{
+ struct nbd_request request;
+ struct nbd_reply reply;
+ ssize_t ret;
+
+ if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) {
+ return 0;
+ }
+
+ request.type = NBD_CMD_FLUSH;
+ if (client->nbdflags & NBD_FLAG_SEND_FUA) {
+ request.type |= NBD_CMD_FLAG_FUA;
+ }
+
+ request.from = 0;
+ request.len = 0;
+
+ nbd_coroutine_start(client, &request);
+ ret = nbd_co_send_request(client, &request, NULL, 0);
+ if (ret < 0) {
+ reply.error = -ret;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+ }
+ nbd_coroutine_end(client, &request);
+
+ return -reply.error;
+}
+
+int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors)
+{
+ struct nbd_request request;
+ struct nbd_reply reply;
+ ssize_t ret;
+
+ if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
+ return 0;
+ }
+
+ request.type = NBD_CMD_TRIM;
+ request.from = sector_num * 512;
+ request.len = nb_sectors * 512;
+
+ nbd_coroutine_start(client, &request);
+ ret = nbd_co_send_request(client, &request, NULL, 0);
+ if (ret < 0) {
+ reply.error = -ret;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+ }
+ nbd_coroutine_end(client, &request);
+
+ return -reply.error;
+}
+
+static void nbd_teardown_connection(NbdClientSession *client)
+{
+ struct nbd_request request;
+
+ request.type = NBD_CMD_DISC;
+ request.from = 0;
+ request.len = 0;
+ nbd_send_request(client->sock, &request);
+
+ qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL, NULL);
+ closesocket(client->sock);
+ client->sock = -1;
+}
+
+void nbd_client_session_close(NbdClientSession *client)
+{
+ nbd_teardown_connection(client);
+ g_free(client->export_name);
+}
+
+int nbd_client_session_init(NbdClientSession *client,
+ BlockDriverState *bs, int sock)
+{
+ int ret;
+
+ ret = nbd_receive_negotiate(sock, client->export_name,
+ &client->nbdflags, &client->size,
+ &client->blocksize);
+ if (ret < 0) {
+ logout("Failed to negotiate with the NBD server\n");
+ closesocket(sock);
+ return ret;
+ }
+
+ qemu_co_mutex_init(&client->send_mutex);
+ qemu_co_mutex_init(&client->free_sema);
+ client->bs = bs;
+ client->sock = sock;
+
+ /* Now that we're connected, set the socket to be non-blocking and
+ * kick the reply mechanism. */
+ qemu_set_nonblock(client->sock);
+ qemu_aio_set_fd_handler(client->sock, nbd_reply_ready, NULL,
+ nbd_have_request, client);
+
+ logout("Established connection with NBD server\n");
+ return 0;
+}
diff --git a/block/nbd-client.h b/block/nbd-client.h
new file mode 100644
index 0000000..c1a7871
--- /dev/null
+++ b/block/nbd-client.h
@@ -0,0 +1,52 @@
+#ifndef NBD_CLIENT_H
+#define NBD_CLIENT_H
+
+#include "qemu-common.h"
+#include "block/nbd.h"
+#include "block/block_int.h"
+
+/* #define DEBUG_NBD */
+
+#if defined(DEBUG_NBD)
+#define logout(fmt, ...) \
+ fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
+#else
+#define logout(fmt, ...) ((void)0)
+#endif
+
+#define MAX_NBD_REQUESTS 16
+
+typedef struct NbdClientSession {
+ CoMutex send_mutex;
+ CoMutex free_sema;
+
+ int sock;
+ uint32_t nbdflags;
+ off_t size;
+ size_t blocksize;
+
+ Coroutine *send_coroutine;
+ int in_flight;
+
+ Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
+ struct nbd_reply reply;
+
+ char *export_name; /* An NBD server may export several devices */
+ bool is_unix;
+
+ BlockDriverState *bs;
+} NbdClientSession;
+
+int nbd_client_session_init(NbdClientSession *client,
+ BlockDriverState *bs, int sock);
+void nbd_client_session_close(NbdClientSession *client);
+
+int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors);
+int nbd_client_session_co_flush(NbdClientSession *client);
+int nbd_client_session_co_writev(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov);
+int nbd_client_session_co_readv(NbdClientSession *client, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov);
+
+#endif /* NBD_CLIENT_H */
diff --git a/block/nbd.c b/block/nbd.c
index 30e3b78..79ba0a6 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -26,10 +26,8 @@
* THE SOFTWARE.
*/
-#include "qemu-common.h"
-#include "block/nbd.h"
+#include "nbd-client.h"
#include "qemu/uri.h"
-#include "block/block_int.h"
#include "qemu/module.h"
#include "qemu/sockets.h"
#include "qapi/qmp/qjson.h"
@@ -38,41 +36,14 @@
#include <sys/types.h>
#include <unistd.h>
-#define EN_OPTSTR ":exportname="
-
-/* #define DEBUG_NBD */
-
-#if defined(DEBUG_NBD)
-#define logout(fmt, ...) \
- fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
-#else
-#define logout(fmt, ...) ((void)0)
-#endif
-
-#define MAX_NBD_REQUESTS 16
-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index) ((index) ^ ((uint64_t)(intptr_t)bs))
-
typedef struct BDRVNBDState {
- int sock;
- uint32_t nbdflags;
- off_t size;
- size_t blocksize;
-
- CoMutex send_mutex;
- CoMutex free_sema;
- Coroutine *send_coroutine;
- int in_flight;
-
- Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
- struct nbd_reply reply;
+ NbdClientSession client;
- bool is_unix;
QemuOpts *socket_opts;
-
- char *export_name; /* An NBD server may export several devices */
} BDRVNBDState;
+#define EN_OPTSTR ":exportname="
+
static int nbd_parse_uri(const char *filename, QDict *options)
{
URI *uri;
@@ -218,9 +189,9 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
"be used at the same time.");
return -EINVAL;
}
- s->is_unix = true;
+ s->client.is_unix = true;
} else if (qdict_haskey(options, "host")) {
- s->is_unix = false;
+ s->client.is_unix = false;
} else {
return -EINVAL;
}
@@ -238,171 +209,20 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT);
}
- s->export_name = g_strdup(qdict_get_try_str(options, "export"));
- if (s->export_name) {
+ s->client.export_name = g_strdup(qdict_get_try_str(options, "export"));
+ if (s->client.export_name) {
qdict_del(options, "export");
}
return 0;
}
-
-static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
-{
- int i;
-
- /* Poor man semaphore. The free_sema is locked when no other request
- * can be accepted, and unlocked after receiving one reply. */
- if (s->in_flight >= MAX_NBD_REQUESTS - 1) {
- qemu_co_mutex_lock(&s->free_sema);
- assert(s->in_flight < MAX_NBD_REQUESTS);
- }
- s->in_flight++;
-
- for (i = 0; i < MAX_NBD_REQUESTS; i++) {
- if (s->recv_coroutine[i] == NULL) {
- s->recv_coroutine[i] = qemu_coroutine_self();
- break;
- }
- }
-
- assert(i < MAX_NBD_REQUESTS);
- request->handle = INDEX_TO_HANDLE(s, i);
-}
-
-static int nbd_have_request(void *opaque)
-{
- BDRVNBDState *s = opaque;
-
- return s->in_flight > 0;
-}
-
-static void nbd_reply_ready(void *opaque)
-{
- BDRVNBDState *s = opaque;
- uint64_t i;
- int ret;
-
- if (s->reply.handle == 0) {
- /* No reply already in flight. Fetch a header. It is possible
- * that another thread has done the same thing in parallel, so
- * the socket is not readable anymore.
- */
- ret = nbd_receive_reply(s->sock, &s->reply);
- if (ret == -EAGAIN) {
- return;
- }
- if (ret < 0) {
- s->reply.handle = 0;
- goto fail;
- }
- }
-
- /* There's no need for a mutex on the receive side, because the
- * handler acts as a synchronization point and ensures that only
- * one coroutine is called until the reply finishes. */
- i = HANDLE_TO_INDEX(s, s->reply.handle);
- if (i >= MAX_NBD_REQUESTS) {
- goto fail;
- }
-
- if (s->recv_coroutine[i]) {
- qemu_coroutine_enter(s->recv_coroutine[i], NULL);
- return;
- }
-
-fail:
- for (i = 0; i < MAX_NBD_REQUESTS; i++) {
- if (s->recv_coroutine[i]) {
- qemu_coroutine_enter(s->recv_coroutine[i], NULL);
- }
- }
-}
-
-static void nbd_restart_write(void *opaque)
-{
- BDRVNBDState *s = opaque;
- qemu_coroutine_enter(s->send_coroutine, NULL);
-}
-
-static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
- QEMUIOVector *qiov, int offset)
-{
- int rc, ret;
-
- qemu_co_mutex_lock(&s->send_mutex);
- s->send_coroutine = qemu_coroutine_self();
- qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
- nbd_have_request, s);
- if (qiov) {
- if (!s->is_unix) {
- socket_set_cork(s->sock, 1);
- }
- rc = nbd_send_request(s->sock, request);
- if (rc >= 0) {
- ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
- offset, request->len);
- if (ret != request->len) {
- rc = -EIO;
- }
- }
- if (!s->is_unix) {
- socket_set_cork(s->sock, 0);
- }
- } else {
- rc = nbd_send_request(s->sock, request);
- }
- qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
- nbd_have_request, s);
- s->send_coroutine = NULL;
- qemu_co_mutex_unlock(&s->send_mutex);
- return rc;
-}
-
-static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
- struct nbd_reply *reply,
- QEMUIOVector *qiov, int offset)
-{
- int ret;
-
- /* Wait until we're woken up by the read handler. TODO: perhaps
- * peek at the next reply and avoid yielding if it's ours? */
- qemu_coroutine_yield();
- *reply = s->reply;
- if (reply->handle != request->handle) {
- reply->error = EIO;
- } else {
- if (qiov && reply->error == 0) {
- ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov,
- offset, request->len);
- if (ret != request->len) {
- reply->error = EIO;
- }
- }
-
- /* Tell the read handler to read another header. */
- s->reply.handle = 0;
- }
-}
-
-static void nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
-{
- int i = HANDLE_TO_INDEX(s, request->handle);
- s->recv_coroutine[i] = NULL;
- if (s->in_flight-- == MAX_NBD_REQUESTS) {
- qemu_co_mutex_unlock(&s->free_sema);
- }
-}
-
static int nbd_establish_connection(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
int sock;
- int ret;
- off_t size;
- size_t blocksize;
- if (s->is_unix) {
+ if (s->client.is_unix) {
sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path"));
} else {
sock = tcp_socket_outgoing_opts(s->socket_opts);
@@ -417,50 +237,13 @@ static int nbd_establish_connection(BlockDriverState *bs)
return -errno;
}
- /* NBD handshake */
- ret = nbd_receive_negotiate(sock, s->export_name, &s->nbdflags, &size,
- &blocksize);
- if (ret < 0) {
- logout("Failed to negotiate with the NBD server\n");
- closesocket(sock);
- return ret;
- }
-
- /* Now that we're connected, set the socket to be non-blocking and
- * kick the reply mechanism. */
- qemu_set_nonblock(sock);
- qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL,
- nbd_have_request, s);
-
- s->sock = sock;
- s->size = size;
- s->blocksize = blocksize;
-
- logout("Established connection with NBD server\n");
- return 0;
-}
-
-static void nbd_teardown_connection(BlockDriverState *bs)
-{
- BDRVNBDState *s = bs->opaque;
- struct nbd_request request;
-
- request.type = NBD_CMD_DISC;
- request.from = 0;
- request.len = 0;
- nbd_send_request(s->sock, &request);
-
- qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL, NULL);
- closesocket(s->sock);
+ return sock;
}
static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVNBDState *s = bs->opaque;
- int result;
-
- qemu_co_mutex_init(&s->send_mutex);
- qemu_co_mutex_init(&s->free_sema);
+ int result, sock;
/* Pop the config into our state object. Exit if invalid. */
result = nbd_config(s, options);
@@ -471,172 +254,62 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
/* establish TCP connection, return error if it fails
* TODO: Configurable retry-until-timeout behaviour.
*/
- result = nbd_establish_connection(bs);
-
- return result;
-}
-
-static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov,
- int offset)
-{
- BDRVNBDState *s = bs->opaque;
- struct nbd_request request;
- struct nbd_reply reply;
- ssize_t ret;
-
- request.type = NBD_CMD_READ;
- request.from = sector_num * 512;
- request.len = nb_sectors * 512;
-
- nbd_coroutine_start(s, &request);
- ret = nbd_co_send_request(s, &request, NULL, 0);
- if (ret < 0) {
- reply.error = -ret;
- } else {
- nbd_co_receive_reply(s, &request, &reply, qiov, offset);
- }
- nbd_coroutine_end(s, &request);
- return -reply.error;
-
-}
-
-static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov,
- int offset)
-{
- BDRVNBDState *s = bs->opaque;
- struct nbd_request request;
- struct nbd_reply reply;
- ssize_t ret;
-
- request.type = NBD_CMD_WRITE;
- if (!bdrv_enable_write_cache(bs) && (s->nbdflags & NBD_FLAG_SEND_FUA)) {
- request.type |= NBD_CMD_FLAG_FUA;
+ sock = nbd_establish_connection(bs);
+ if (sock < 0) {
+ return sock;
}
- request.from = sector_num * 512;
- request.len = nb_sectors * 512;
-
- nbd_coroutine_start(s, &request);
- ret = nbd_co_send_request(s, &request, qiov, offset);
- if (ret < 0) {
- reply.error = -ret;
- } else {
- nbd_co_receive_reply(s, &request, &reply, NULL, 0);
- }
- nbd_coroutine_end(s, &request);
- return -reply.error;
+ /* NBD handshake */
+ return nbd_client_session_init(&s->client, bs, sock);
}
-/* qemu-nbd has a limit of slightly less than 1M per request. Try to
- * remain aligned to 4K. */
-#define NBD_MAX_SECTORS 2040
-
static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
- int offset = 0;
- int ret;
- while (nb_sectors > NBD_MAX_SECTORS) {
- ret = nbd_co_readv_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
- if (ret < 0) {
- return ret;
- }
- offset += NBD_MAX_SECTORS * 512;
- sector_num += NBD_MAX_SECTORS;
- nb_sectors -= NBD_MAX_SECTORS;
- }
- return nbd_co_readv_1(bs, sector_num, nb_sectors, qiov, offset);
+ BDRVNBDState *s = bs->opaque;
+
+ return nbd_client_session_co_readv(&s->client, sector_num,
+ nb_sectors, qiov);
}
static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
- int offset = 0;
- int ret;
- while (nb_sectors > NBD_MAX_SECTORS) {
- ret = nbd_co_writev_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
- if (ret < 0) {
- return ret;
- }
- offset += NBD_MAX_SECTORS * 512;
- sector_num += NBD_MAX_SECTORS;
- nb_sectors -= NBD_MAX_SECTORS;
- }
- return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset);
+ BDRVNBDState *s = bs->opaque;
+
+ return nbd_client_session_co_writev(&s->client, sector_num,
+ nb_sectors, qiov);
}
static int nbd_co_flush(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
- struct nbd_request request;
- struct nbd_reply reply;
- ssize_t ret;
-
- if (!(s->nbdflags & NBD_FLAG_SEND_FLUSH)) {
- return 0;
- }
-
- request.type = NBD_CMD_FLUSH;
- if (s->nbdflags & NBD_FLAG_SEND_FUA) {
- request.type |= NBD_CMD_FLAG_FUA;
- }
-
- request.from = 0;
- request.len = 0;
- nbd_coroutine_start(s, &request);
- ret = nbd_co_send_request(s, &request, NULL, 0);
- if (ret < 0) {
- reply.error = -ret;
- } else {
- nbd_co_receive_reply(s, &request, &reply, NULL, 0);
- }
- nbd_coroutine_end(s, &request);
- return -reply.error;
+ return nbd_client_session_co_flush(&s->client);
}
static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
BDRVNBDState *s = bs->opaque;
- struct nbd_request request;
- struct nbd_reply reply;
- ssize_t ret;
- if (!(s->nbdflags & NBD_FLAG_SEND_TRIM)) {
- return 0;
- }
- request.type = NBD_CMD_TRIM;
- request.from = sector_num * 512;
- request.len = nb_sectors * 512;
-
- nbd_coroutine_start(s, &request);
- ret = nbd_co_send_request(s, &request, NULL, 0);
- if (ret < 0) {
- reply.error = -ret;
- } else {
- nbd_co_receive_reply(s, &request, &reply, NULL, 0);
- }
- nbd_coroutine_end(s, &request);
- return -reply.error;
+ return nbd_client_session_co_discard(&s->client, sector_num,
+ nb_sectors);
}
static void nbd_close(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
- g_free(s->export_name);
- qemu_opts_del(s->socket_opts);
- nbd_teardown_connection(bs);
+ qemu_opts_del(s->socket_opts);
+ nbd_client_session_close(&s->client);
}
static int64_t nbd_getlength(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
- return s->size;
+ return s->client.size;
}
static BlockDriver bdrv_nbd = {
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 05/12] nbd: pass export name as init argument
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (3 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 04/12] Split nbd block client code Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 06/12] nbd: make session_close() idempotent Marc-André Lureau
` (6 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
There is no need to keep the export name around, and it seems a better
fit as an argument in the init() call.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block/nbd-client.c | 8 ++++----
block/nbd-client.h | 5 ++---
block/nbd.c | 13 ++++++++-----
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 6d5f39c..b7eea21 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -353,15 +353,15 @@ static void nbd_teardown_connection(NbdClientSession *client)
void nbd_client_session_close(NbdClientSession *client)
{
nbd_teardown_connection(client);
- g_free(client->export_name);
}
-int nbd_client_session_init(NbdClientSession *client,
- BlockDriverState *bs, int sock)
+int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
+ int sock, const char *export)
{
int ret;
- ret = nbd_receive_negotiate(sock, client->export_name,
+ logout("session init %s\n", export);
+ ret = nbd_receive_negotiate(sock, export,
&client->nbdflags, &client->size,
&client->blocksize);
if (ret < 0) {
diff --git a/block/nbd-client.h b/block/nbd-client.h
index c1a7871..9c5246b 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -31,14 +31,13 @@ typedef struct NbdClientSession {
Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
struct nbd_reply reply;
- char *export_name; /* An NBD server may export several devices */
bool is_unix;
BlockDriverState *bs;
} NbdClientSession;
-int nbd_client_session_init(NbdClientSession *client,
- BlockDriverState *bs, int sock);
+int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
+ int sock, const char *export_name);
void nbd_client_session_close(NbdClientSession *client);
int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
diff --git a/block/nbd.c b/block/nbd.c
index 79ba0a6..18c3b78 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -179,7 +179,7 @@ out:
g_free(file);
}
-static int nbd_config(BDRVNBDState *s, QDict *options)
+static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
{
Error *local_err = NULL;
@@ -209,8 +209,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT);
}
- s->client.export_name = g_strdup(qdict_get_try_str(options, "export"));
- if (s->client.export_name) {
+ *export = g_strdup(qdict_get_try_str(options, "export"));
+ if (*export) {
qdict_del(options, "export");
}
@@ -243,10 +243,11 @@ static int nbd_establish_connection(BlockDriverState *bs)
static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
{
BDRVNBDState *s = bs->opaque;
+ char *export = NULL;
int result, sock;
/* Pop the config into our state object. Exit if invalid. */
- result = nbd_config(s, options);
+ result = nbd_config(s, options, &export);
if (result != 0) {
return result;
}
@@ -260,7 +261,9 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
}
/* NBD handshake */
- return nbd_client_session_init(&s->client, bs, sock);
+ result = nbd_client_session_init(&s->client, bs, sock, export);
+ g_free(export);
+ return result;
}
static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 06/12] nbd: make session_close() idempotent
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (4 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 05/12] nbd: pass export name as init argument Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState Marc-André Lureau
` (5 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block/nbd-client.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index b7eea21..c49be30 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -352,7 +352,12 @@ static void nbd_teardown_connection(NbdClientSession *client)
void nbd_client_session_close(NbdClientSession *client)
{
+ if (!client->bs) {
+ return;
+ }
+
nbd_teardown_connection(client);
+ client->bs = NULL;
}
int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (5 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 06/12] nbd: make session_close() idempotent Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 22:25 ` Paolo Bonzini
2013-06-20 17:46 ` [Qemu-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open() Marc-André Lureau
` (4 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
This allows the Spice block driver to eject the associated device.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block.c | 46 +++++++++++++++++++++++++++++-----------------
include/block/block_int.h | 1 +
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/block.c b/block.c
index b88ad2f..f502eed 100644
--- a/block.c
+++ b/block.c
@@ -294,7 +294,8 @@ void bdrv_register(BlockDriver *bdrv)
}
/* create a new block device (by default it is empty) */
-BlockDriverState *bdrv_new(const char *device_name)
+static BlockDriverState *bdrv_new_int(const char *device_name,
+ BlockDriverState *child)
{
BlockDriverState *bs;
@@ -305,10 +306,16 @@ BlockDriverState *bdrv_new(const char *device_name)
}
bdrv_iostatus_disable(bs);
notifier_list_init(&bs->close_notifiers);
+ bs->child = child;
return bs;
}
+BlockDriverState *bdrv_new(const char *device_name)
+{
+ return bdrv_new_int(device_name, NULL);
+}
+
void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
{
notifier_list_add(&bs->close_notifiers, notify);
@@ -769,16 +776,8 @@ free_and_fail:
return ret;
}
-/*
- * Opens a file using a protocol (file, host_device, nbd, ...)
- *
- * options is a QDict of options to pass to the block drivers, or NULL for an
- * empty set of options. The reference to the QDict belongs to the block layer
- * after the call (even on failure), so if the caller intends to reuse the
- * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
- */
-int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags)
+static int bdrv_file_open_int(BlockDriverState **pbs, const char *filename,
+ QDict *options, int flags, BlockDriverState *child)
{
BlockDriverState *bs;
BlockDriver *drv;
@@ -790,7 +789,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
options = qdict_new();
}
- bs = bdrv_new("");
+ bs = bdrv_new_int("", child);
bs->options = options;
options = qdict_clone_shallow(options);
@@ -873,6 +872,20 @@ fail:
}
/*
+ * Opens a file using a protocol (file, host_device, nbd, ...)
+ *
+ * options is a QDict of options to pass to the block drivers, or NULL for an
+ * empty set of options. The reference to the QDict belongs to the block layer
+ * after the call (even on failure), so if the caller intends to reuse the
+ * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
+ */
+int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+ QDict *options, int flags)
+{
+ return bdrv_file_open_int(pbs, filename, options, flags, NULL);
+}
+
+/*
* Opens the backing file for a BlockDriverState if not yet open
*
* options is a QDict of options to pass to the block drivers, or NULL for an
@@ -904,7 +917,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
return 0;
}
- bs->backing_hd = bdrv_new("");
+ bs->backing_hd = bdrv_new_int("", bs);
bdrv_get_full_backing_filename(bs, backing_filename,
sizeof(backing_filename));
@@ -990,7 +1003,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
instead of opening 'filename' directly */
/* if there is a backing file, use it */
- bs1 = bdrv_new("");
+ bs1 = bdrv_new_int("", bs);
ret = bdrv_open(bs1, filename, NULL, 0, drv);
if (ret < 0) {
bdrv_delete(bs1);
@@ -1043,9 +1056,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
extract_subqdict(options, &file_options, "file.");
-
- ret = bdrv_file_open(&file, filename, file_options,
- bdrv_open_flags(bs, flags));
+ ret = bdrv_file_open_int(&file, filename, file_options,
+ bdrv_open_flags(bs, flags), bs);
if (ret < 0) {
goto fail;
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ba52247..9c72b32 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -245,6 +245,7 @@ struct BlockDriverState {
BlockDriverState *backing_hd;
BlockDriverState *file;
+ BlockDriverState *child;
NotifierList close_notifiers;
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open()
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (6 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 09/12] block: add "snapshot.size" option to avoid extra bdrv_open() Marc-André Lureau
` (3 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block.c | 107 +++++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 62 insertions(+), 45 deletions(-)
diff --git a/block.c b/block.c
index f502eed..5db8fa1 100644
--- a/block.c
+++ b/block.c
@@ -959,6 +959,65 @@ static void extract_subqdict(QDict *src, QDict **dst, const char *start)
}
}
+static int make_snapshot(BlockDriverState *bs, int64_t total_size,
+ const char **pfilename, BlockDriver **pdrv)
+{
+ const char *filename = *pfilename;
+ BlockDriver *drv = *pdrv;
+ int ret;
+ BlockDriver *bdrv_qcow2;
+ QEMUOptionParameter *create_options;
+ char backing_filename[PATH_MAX];
+ /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
+ char tmp_filename[PATH_MAX + 1];
+
+ assert(filename != NULL);
+ total_size &= BDRV_SECTOR_MASK;
+
+ /* if snapshot, we create a temporary backing file and open it
+ instead of opening 'filename' directly */
+
+ ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+ if (ret < 0) {
+ goto fail;
+ }
+
+ /* Real path is meaningless for protocols */
+ if (path_has_protocol(filename)) {
+ snprintf(backing_filename, sizeof(backing_filename),
+ "%s", filename);
+ } else if (!realpath(filename, backing_filename)) {
+ ret = -errno;
+ goto fail;
+ }
+
+ bdrv_qcow2 = bdrv_find_format("qcow2");
+ create_options = parse_option_parameters("", bdrv_qcow2->create_options,
+ NULL);
+
+ set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
+ set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE,
+ backing_filename);
+ if (drv) {
+ set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT,
+ drv->format_name);
+ }
+
+ ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+ free_option_parameters(create_options);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ *pfilename = tmp_filename;
+ *pdrv = bdrv_qcow2;
+ bs->is_temporary = 1;
+ return 0;
+
+fail:
+ return ret;
+}
+
/*
* Opens a disk image (raw, qcow2, vmdk, ...)
*
@@ -971,8 +1030,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
int flags, BlockDriver *drv)
{
int ret;
- /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
- char tmp_filename[PATH_MAX + 1];
BlockDriverState *file = NULL;
QDict *file_options = NULL;
@@ -988,66 +1045,26 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
if (flags & BDRV_O_SNAPSHOT) {
BlockDriverState *bs1;
int64_t total_size;
- BlockDriver *bdrv_qcow2;
- QEMUOptionParameter *create_options;
- char backing_filename[PATH_MAX];
if (qdict_size(options) != 0) {
error_report("Can't use snapshot=on with driver-specific options");
ret = -EINVAL;
goto fail;
}
- assert(filename != NULL);
- /* if snapshot, we create a temporary backing file and open it
- instead of opening 'filename' directly */
-
- /* if there is a backing file, use it */
- bs1 = bdrv_new_int("", bs);
+ bs1 = bdrv_new_int("", NULL);
ret = bdrv_open(bs1, filename, NULL, 0, drv);
if (ret < 0) {
bdrv_delete(bs1);
goto fail;
}
- total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
-
+ total_size = bdrv_getlength(bs1);
bdrv_delete(bs1);
- ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+ ret = make_snapshot(bs, total_size, &filename, &drv);
if (ret < 0) {
goto fail;
}
-
- /* Real path is meaningless for protocols */
- if (path_has_protocol(filename)) {
- snprintf(backing_filename, sizeof(backing_filename),
- "%s", filename);
- } else if (!realpath(filename, backing_filename)) {
- ret = -errno;
- goto fail;
- }
-
- bdrv_qcow2 = bdrv_find_format("qcow2");
- create_options = parse_option_parameters("", bdrv_qcow2->create_options,
- NULL);
-
- set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
- set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE,
- backing_filename);
- if (drv) {
- set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT,
- drv->format_name);
- }
-
- ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
- free_option_parameters(create_options);
- if (ret < 0) {
- goto fail;
- }
-
- filename = tmp_filename;
- drv = bdrv_qcow2;
- bs->is_temporary = 1;
}
/* Open image file without format layer */
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 09/12] block: add "snapshot.size" option to avoid extra bdrv_open()
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (7 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open() Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 10/12] block: learn to open a driver with a given opaque Marc-André Lureau
` (2 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 5db8fa1..b421083 100644
--- a/block.c
+++ b/block.c
@@ -1046,20 +1046,25 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
BlockDriverState *bs1;
int64_t total_size;
+ total_size = qdict_get_try_int(options, "snapshot.size", -1);
+ qdict_del(options, "snapshot.size");
+
if (qdict_size(options) != 0) {
error_report("Can't use snapshot=on with driver-specific options");
ret = -EINVAL;
goto fail;
}
- bs1 = bdrv_new_int("", NULL);
- ret = bdrv_open(bs1, filename, NULL, 0, drv);
- if (ret < 0) {
+ if (total_size == -1) {
+ bs1 = bdrv_new_int("", NULL);
+ ret = bdrv_open(bs1, filename, NULL, 0, drv);
+ if (ret < 0) {
+ bdrv_delete(bs1);
+ goto fail;
+ }
+ total_size = bdrv_getlength(bs1);
bdrv_delete(bs1);
- goto fail;
}
- total_size = bdrv_getlength(bs1);
- bdrv_delete(bs1);
ret = make_snapshot(bs, total_size, &filename, &drv);
if (ret < 0) {
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 10/12] block: learn to open a driver with a given opaque
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (8 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 09/12] block: add "snapshot.size" option to avoid extra bdrv_open() Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 12/12] block: add spice block device backend Marc-André Lureau
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
If the block driver is given an opaque data, there is no need to
allocate a new one. This allows to pass an existing driver state to the
new driver.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block.c | 47 ++++++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/block.c b/block.c
index b421083..bdffb42 100644
--- a/block.c
+++ b/block.c
@@ -295,7 +295,7 @@ void bdrv_register(BlockDriver *bdrv)
/* create a new block device (by default it is empty) */
static BlockDriverState *bdrv_new_int(const char *device_name,
- BlockDriverState *child)
+ BlockDriverState *child, void *opaque)
{
BlockDriverState *bs;
@@ -307,13 +307,14 @@ static BlockDriverState *bdrv_new_int(const char *device_name,
bdrv_iostatus_disable(bs);
notifier_list_init(&bs->close_notifiers);
bs->child = child;
+ bs->opaque = opaque;
return bs;
}
BlockDriverState *bdrv_new(const char *device_name)
{
- return bdrv_new_int(device_name, NULL);
+ return bdrv_new_int(device_name, NULL, NULL);
}
void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
@@ -729,7 +730,9 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
}
bs->drv = drv;
- bs->opaque = g_malloc0(drv->instance_size);
+ if (bs->opaque == NULL) {
+ bs->opaque = g_malloc0(drv->instance_size);
+ }
bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
@@ -777,7 +780,7 @@ free_and_fail:
}
static int bdrv_file_open_int(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags, BlockDriverState *child)
+ QDict *options, int flags, BlockDriverState *child, void *opaque)
{
BlockDriverState *bs;
BlockDriver *drv;
@@ -789,7 +792,7 @@ static int bdrv_file_open_int(BlockDriverState **pbs, const char *filename,
options = qdict_new();
}
- bs = bdrv_new_int("", child);
+ bs = bdrv_new_int("", child, opaque);
bs->options = options;
options = qdict_clone_shallow(options);
@@ -882,18 +885,11 @@ fail:
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
QDict *options, int flags)
{
- return bdrv_file_open_int(pbs, filename, options, flags, NULL);
+ return bdrv_file_open_int(pbs, filename, options, flags, NULL, NULL);
}
-/*
- * Opens the backing file for a BlockDriverState if not yet open
- *
- * options is a QDict of options to pass to the block drivers, or NULL for an
- * empty set of options. The reference to the QDict is transferred to this
- * function (even on failure), so if the caller intends to reuse the dictionary,
- * it needs to use QINCREF() before calling bdrv_file_open.
- */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+static int bdrv_open_backing_file_int(BlockDriverState *bs,
+ QDict *options, void *opaque)
{
char backing_filename[PATH_MAX];
int back_flags, ret;
@@ -917,7 +913,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
return 0;
}
- bs->backing_hd = bdrv_new_int("", bs);
+ bs->backing_hd = bdrv_new_int("", bs, opaque);
bdrv_get_full_backing_filename(bs, backing_filename,
sizeof(backing_filename));
@@ -940,6 +936,19 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
return 0;
}
+/*
+ * Opens the backing file for a BlockDriverState if not yet open
+ *
+ * options is a QDict of options to pass to the block drivers, or NULL for an
+ * empty set of options. The reference to the QDict is transferred to this
+ * function (even on failure), so if the caller intends to reuse the dictionary,
+ * it needs to use QINCREF() before calling bdrv_file_open.
+ */
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+{
+ return bdrv_open_backing_file_int(bs, options, NULL);
+}
+
static void extract_subqdict(QDict *src, QDict **dst, const char *start)
{
const QDictEntry *entry, *next;
@@ -1056,7 +1065,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
if (total_size == -1) {
- bs1 = bdrv_new_int("", NULL);
+ bs1 = bdrv_new_int("", NULL, NULL);
ret = bdrv_open(bs1, filename, NULL, 0, drv);
if (ret < 0) {
bdrv_delete(bs1);
@@ -1079,7 +1088,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
extract_subqdict(options, &file_options, "file.");
ret = bdrv_file_open_int(&file, filename, file_options,
- bdrv_open_flags(bs, flags), bs);
+ bdrv_open_flags(bs, flags), bs, NULL);
if (ret < 0) {
goto fail;
}
@@ -1109,7 +1118,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
QDict *backing_options;
extract_subqdict(options, &backing_options, "backing.");
- ret = bdrv_open_backing_file(bs, backing_options);
+ ret = bdrv_open_backing_file_int(bs, backing_options, NULL);
if (ret < 0) {
goto close_and_fail;
}
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (9 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 10/12] block: learn to open a driver with a given opaque Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 12/12] block: add spice block device backend Marc-André Lureau
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
If the block driver already has a bs->opaque when calling bdrv_open(),
pass it down to the file driver.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index bdffb42..ff9cb0b 100644
--- a/block.c
+++ b/block.c
@@ -1041,6 +1041,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
int ret;
BlockDriverState *file = NULL;
QDict *file_options = NULL;
+ void *backing_opaque = NULL;
/* NULL means an empty set of options */
if (options == NULL) {
@@ -1064,6 +1065,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
goto fail;
}
+ backing_opaque = bs->opaque;
+ bs->opaque = NULL;
if (total_size == -1) {
bs1 = bdrv_new_int("", NULL, NULL);
ret = bdrv_open(bs1, filename, NULL, 0, drv);
@@ -1088,7 +1091,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
extract_subqdict(options, &file_options, "file.");
ret = bdrv_file_open_int(&file, filename, file_options,
- bdrv_open_flags(bs, flags), bs, NULL);
+ bdrv_open_flags(bs, flags), bs, bs->opaque);
+ bs->opaque = NULL;
if (ret < 0) {
goto fail;
}
@@ -1118,7 +1122,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
QDict *backing_options;
extract_subqdict(options, &backing_options, "backing.");
- ret = bdrv_open_backing_file_int(bs, backing_options, NULL);
+ ret = bdrv_open_backing_file_int(bs, backing_options, backing_opaque);
if (ret < 0) {
goto close_and_fail;
}
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 12/12] block: add spice block device backend
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
` (10 preceding siblings ...)
2013-06-20 17:46 ` [Qemu-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque Marc-André Lureau
@ 2013-06-20 17:46 ` Marc-André Lureau
11 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-20 17:46 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
block/Makefile.objs | 1 +
block/spice.c | 523 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 524 insertions(+)
create mode 100644 block/spice.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 5890b5c..0170011 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -16,6 +16,7 @@ block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
block-obj-$(CONFIG_LIBSSH2) += ssh.o
+common-obj-$(CONFIG_SPICE) += spice.o
endif
common-obj-y += stream.o
diff --git a/block/spice.c b/block/spice.c
new file mode 100644
index 0000000..b2c669d
--- /dev/null
+++ b/block/spice.c
@@ -0,0 +1,523 @@
+/*
+ * Spice block backend for QEMU.
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ * Author: Marc-André Lureau <marcandre.lureau@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <spice/protocol.h>
+
+#include "nbd-client.h"
+#include "ui/qemu-spice.h"
+#include "block/block_int.h"
+#include "qemu/sockets.h"
+#include "qemu/uri.h"
+#include "qapi/qmp/qint.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/char.h"
+
+#ifndef DEBUG_SPICE
+#define DEBUG_SPICE 0
+#endif
+
+#define SOCKET_CHR 0
+#define SOCKET_NBD 1
+
+#define DPRINTF(fmt, ...) \
+ do { \
+ if (DEBUG_SPICE) { \
+ fprintf(stderr, "spice: %-15s " fmt "\n", \
+ __func__, ##__VA_ARGS__); \
+ } \
+ } while (0)
+
+typedef struct Buffer {
+ uint8_t data[4096];
+ uint8_t *p;
+ char left;
+} Buffer;
+
+typedef struct BDRVSpiceState {
+ BlockDriverState *bs;
+ NbdClientSession client;
+ char *export;
+
+ /* our spicechr-fd pipe */
+ int sv[2];
+ Buffer readb;
+ Buffer writeb;
+
+ int aio_count;
+ CharDriverState *chr;
+ guint chr_watch;
+
+ Coroutine *coroutine;
+ bool need_read;
+ bool need_write;
+ bool opened;
+} BDRVSpiceState;
+
+static void nbd_read_handler(void *opaque);
+static void update_chr_handlers(BDRVSpiceState *s);
+
+static int parse_uri(const char *filename, QDict *options, Error **errp)
+{
+ URI *uri = NULL;
+
+ uri = uri_parse(filename);
+ if (!uri) {
+ return -EINVAL;
+ }
+
+ if (strcmp(uri->scheme, "spicebd") != 0) {
+ error_setg(errp, "URI scheme must be 'spicebd'");
+ goto err;
+ }
+
+ if (uri->path && *uri->path) {
+ qdict_put(options, "export", qstring_from_str(uri->path));
+ }
+
+ uri_free(uri);
+ return 0;
+
+ err:
+ if (uri) {
+ uri_free(uri);
+ }
+ return -EINVAL;
+}
+
+static void spice_parse_filename(const char *filename, QDict *options,
+ Error **errp)
+{
+ if (qdict_haskey(options, "export")) {
+ error_setg(errp, "export cannot be used at the same time "
+ "as a file option");
+ return;
+ }
+
+ parse_uri(filename, options, errp);
+}
+
+static void co_restart(void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+
+ qemu_coroutine_enter(s->coroutine, NULL);
+}
+
+static void close_socketpair(BDRVSpiceState *s)
+{
+ /* this is catching various error paths, and deals with it by
+ closing socketpair, so that both ends can cleanup. It may need
+ more specific error handling. */
+ DPRINTF("closing socketpair");
+ if (!s->opened) {
+ return;
+ }
+
+ if (s->sv[SOCKET_NBD] >= 0) {
+ qemu_aio_set_fd_handler(s->sv[SOCKET_NBD], NULL, NULL, NULL, NULL);
+ closesocket(s->sv[SOCKET_NBD]);
+ s->sv[SOCKET_NBD] = -1;
+ }
+
+ if (s->sv[SOCKET_CHR] >= 0) {
+ qemu_aio_set_fd_handler(s->sv[SOCKET_CHR], NULL, NULL, NULL, NULL);
+ closesocket(s->sv[SOCKET_CHR]);
+ s->sv[SOCKET_CHR] = -1;
+ }
+
+ s->opened = FALSE;
+ if (s->coroutine && s->coroutine != qemu_coroutine_self()) {
+ co_restart(s);
+ }
+}
+
+static int chardev_can_read(void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+ int retval = 0;
+
+ GPollFD pfd = {
+ .fd = s->sv[SOCKET_CHR],
+ .events = G_IO_OUT
+ };
+ g_poll(&pfd, 1, 0);
+ if (pfd.revents & G_IO_OUT) {
+ retval = s->writeb.left == 0 ? sizeof(s->writeb.data) : 0;
+ }
+
+ return retval;
+}
+
+static void chardev_read(void *opaque, const uint8_t *buf, int size)
+{
+ BDRVSpiceState *s = opaque;
+ int written;
+
+ DPRINTF("reply from client %d", size);
+ written = write(s->sv[SOCKET_CHR], buf, size);
+ if (written == -1) {
+ if (errno != EAGAIN) {
+ close_socketpair(s);
+ return;
+ } else {
+ written = 0;
+ }
+ }
+
+ if (s->writeb.left == 0) {
+ size -= written;
+ assert(size <= sizeof(s->writeb.data));
+ memcpy(s->writeb.data, buf, size);
+ s->writeb.p = s->writeb.data;
+ s->writeb.left = size;
+ } else {
+ s->writeb.left -= written;
+ s->writeb.p += written;
+ }
+
+ s->need_write = s->writeb.left > 0;
+ update_chr_handlers(s);
+}
+
+static void coroutine_fn co_init(void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+ BlockDriverState *bs = s->bs;
+ int bdrv_flags;
+
+ DPRINTF("temporary coroutine for session initialization %p", s);
+
+ qemu_set_nonblock(s->sv[SOCKET_NBD]);
+ /* After session_init, the fd_handler is managed by nbd-client */
+ qemu_aio_set_fd_handler(s->sv[SOCKET_NBD], co_restart, NULL, NULL, s);
+ if (nbd_client_session_init(&s->client, bs,
+ s->sv[SOCKET_NBD], s->export) < 0) {
+ goto error;
+ }
+
+ bs->opaque = NULL;
+
+ while (bs->child) {
+ bs = bs->child;
+ }
+
+ if (bdrv_in_use(bs)) {
+ goto error;
+ }
+ if (!bdrv_dev_has_removable_media(bs)) {
+ goto error;
+ }
+ if (bdrv_dev_is_medium_locked(bs) && !bdrv_dev_is_tray_open(bs)) {
+ bdrv_dev_eject_request(bs, true);
+ }
+
+ /* could the switch be racy with other threads and need to be
+ called from main loop ? */
+ bdrv_close(bs);
+
+ QDict *options = NULL;
+ bs->opaque = s;
+
+ bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+ if (bdrv_is_snapshot(bs)) {
+ bdrv_flags |= BDRV_O_SNAPSHOT;
+ options = qdict_new();
+ qdict_put(options, "snapshot.size", qint_from_int(s->client.size));
+ }
+
+ DPRINTF("reopen %p", s);
+ if (bdrv_open(bs, "spicebd:", options, bdrv_flags, NULL) < 0) {
+ DPRINTF("failed to re-open spicebd");
+ goto error;
+ }
+
+ goto end;
+
+error:
+ close_socketpair(s);
+end:
+ s->coroutine = NULL;
+ DPRINTF("end of temporary coroutine");
+}
+
+static void spice_init(BDRVSpiceState *s)
+{
+ if (s->opened) {
+ return;
+ }
+
+ s->opened = TRUE;
+ /* gosh that sucks :) */
+ if (socketpair(PF_UNIX, SOCK_STREAM, 0, s->sv) == -1) {
+ fprintf(stderr, "failed to create socketpair\n");
+ return;
+ }
+
+ s->need_read = TRUE;
+ update_chr_handlers(s);
+
+ /* tell server we are ready */
+ qemu_chr_fe_event(s->chr, SPICE_PORT_EVENT_OPENED);
+ s->coroutine = qemu_coroutine_create(co_init);
+ qemu_coroutine_enter(s->coroutine, s);
+}
+
+static void chardev_event(void *opaque, int event)
+{
+ BDRVSpiceState *s = opaque;
+ DPRINTF("chardev_event %d", event);
+
+ switch (event) {
+ case CHR_EVENT_CLOSED:
+ DPRINTF("chardev close");
+ close_socketpair(s);
+ break;
+ case CHR_EVENT_BREAK:
+ DPRINTF("chardev break");
+ if (s->coroutine) {
+ DPRINTF("already waiting for incoming session");
+ return;
+ }
+ close_socketpair(s);
+ /* fall-through */
+ case CHR_EVENT_OPENED:
+ spice_init(s);
+ break;
+ default:
+ DPRINTF("unhandled chardev event %d", event);
+ }
+}
+
+static gboolean write_to_chr(GIOChannel *chan, GIOCondition cond,
+ void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+ int r;
+
+ r = qemu_chr_fe_write(s->chr, s->readb.p, s->readb.left);
+ DPRINTF("write_to_chr %d/%d", r, s->readb.left);
+ if (r <= 0) {
+ close_socketpair(s);
+ return FALSE;
+ }
+
+ s->readb.p += r;
+ s->readb.left -= r;
+
+ if (s->readb.left > 0) {
+ if (!s->chr_watch) {
+ s->chr_watch = qemu_chr_fe_add_watch(s->chr, G_IO_OUT,
+ write_to_chr, s);
+ }
+ return TRUE;
+ } else {
+ s->need_read = TRUE;
+ update_chr_handlers(s);
+ }
+
+ return FALSE;
+}
+
+static void nbd_read_handler(void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+
+ DPRINTF("read from nbd");
+
+ if (s->readb.left > 0) {
+ abort();
+ }
+
+ do {
+ s->readb.left = recv(s->sv[SOCKET_CHR], s->readb.data,
+ sizeof(s->readb.data), MSG_DONTWAIT);
+ } while (s->readb.left == -1 && errno == EAGAIN);
+
+ if (s->readb.left <= 0) {
+ close_socketpair(s);
+ return;
+ }
+
+ s->need_read = FALSE;
+ update_chr_handlers(s);
+
+ s->readb.p = s->readb.data;
+ write_to_chr(NULL, 0, s);
+}
+
+static void nbd_write_handler(void *opaque)
+{
+ BDRVSpiceState *s = opaque;
+
+ DPRINTF("resuming chardev_read left: %d", s->writeb.left);
+
+ chardev_read(s, s->writeb.data, s->writeb.left);
+}
+
+static void update_chr_handlers(BDRVSpiceState *s)
+{
+ qemu_aio_set_fd_handler(s->sv[SOCKET_CHR],
+ s->need_read ? nbd_read_handler : NULL,
+ s->need_write ? nbd_write_handler : NULL,
+ NULL, s);
+}
+
+static int spice_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
+{
+ BDRVSpiceState *s = bs->opaque;
+ int ret = -1;
+
+ s->bs = bs;
+ if (s->opened) {
+ DPRINTF("re-open spicebd");
+ s->client.bs = bs;
+ return 0;
+ }
+
+ if (qdict_haskey(options, "export")) {
+ s->export = strdup(qdict_get_str(options, "export"));
+ qdict_del(options, "export");
+ }
+
+ DPRINTF("open %p flags=0x%x export=%s", s, bdrv_flags, s->export);
+ if (bdrv_flags & BDRV_O_RDWR) {
+ fprintf(stderr, "spicebd: only read-only supported\n");
+ return -1;
+ }
+
+ s->chr = qemu_chr_open_spice_vmc("nbd");
+ if (!s->chr) {
+ goto err;
+ }
+
+ qemu_chr_add_handlers(s->chr, chardev_can_read,
+ chardev_read, chardev_event, s);
+ spice_init(s);
+
+ return 0;
+
+ err:
+ return ret;
+}
+
+static void spice_close(BlockDriverState *bs)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ DPRINTF("spice close %p\n", s);
+
+ if (s == NULL) {
+ /* changing bd */
+ return;
+ }
+
+ s->bs = NULL;
+
+ nbd_client_session_close(&s->client);
+ close_socketpair(s);
+ assert(!s->coroutine); /* after close_socketpair */
+
+ if (s->chr) {
+ s->chr->chr_close(s->chr);
+ g_free(s->chr);
+ s->chr = NULL;
+ }
+
+ g_free(s->export);
+ s->export = NULL;
+}
+
+static coroutine_fn int spice_co_readv(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ return nbd_client_session_co_readv(&s->client, sector_num,
+ nb_sectors, qiov);
+}
+
+static coroutine_fn int spice_co_writev(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ return nbd_client_session_co_writev(&s->client, sector_num,
+ nb_sectors, qiov);
+}
+
+static coroutine_fn int spice_co_flush(BlockDriverState *bs)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ if (s == NULL) {
+ /* changing bd */
+ return -1;
+ }
+
+ return nbd_client_session_co_flush(&s->client);
+}
+
+static coroutine_fn int spice_co_discard(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ return nbd_client_session_co_discard(&s->client, sector_num, nb_sectors);
+}
+
+static coroutine_fn int64_t spice_getlength(BlockDriverState *bs)
+{
+ BDRVSpiceState *s = bs->opaque;
+
+ DPRINTF("length=%" PRIi64, s->client.size);
+
+ return s->client.size;
+}
+
+static BlockDriver bdrv_spice = {
+ .format_name = "spicebd",
+ .protocol_name = "spicebd",
+ .instance_size = sizeof(BDRVSpiceState),
+ .bdrv_parse_filename = spice_parse_filename,
+ .bdrv_file_open = spice_file_open,
+ .bdrv_close = spice_close,
+ .bdrv_co_readv = spice_co_readv,
+ .bdrv_co_writev = spice_co_writev,
+ .bdrv_getlength = spice_getlength,
+ .bdrv_co_flush_to_os = spice_co_flush,
+ .bdrv_co_discard = spice_co_discard,
+};
+
+static void bdrv_spice_init(void)
+{
+ bdrv_register(&bdrv_spice);
+}
+
+block_init(bdrv_spice_init);
--
1.8.3.rc1.49.g8d97506
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-20 17:46 ` [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState Marc-André Lureau
@ 2013-06-20 22:25 ` Paolo Bonzini
2013-06-21 8:36 ` Marc-André Lureau
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2013-06-20 22:25 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: spice-devel, qemu-devel
Il 20/06/2013 19:46, Marc-André Lureau ha scritto:
> This allows the Spice block driver to eject the associated device.
The child can change when you have for example a streaming operation.
What exactly are you trying to do here (I guess I'll understand more
when I get to the later patches)?
Can you draw the relationships between all the BlockDriverStates in a
spicebd: drive?
Paolo
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> block.c | 46 +++++++++++++++++++++++++++++-----------------
> include/block/block_int.h | 1 +
> 2 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/block.c b/block.c
> index b88ad2f..f502eed 100644
> --- a/block.c
> +++ b/block.c
> @@ -294,7 +294,8 @@ void bdrv_register(BlockDriver *bdrv)
> }
>
> /* create a new block device (by default it is empty) */
> -BlockDriverState *bdrv_new(const char *device_name)
> +static BlockDriverState *bdrv_new_int(const char *device_name,
> + BlockDriverState *child)
> {
> BlockDriverState *bs;
>
> @@ -305,10 +306,16 @@ BlockDriverState *bdrv_new(const char *device_name)
> }
> bdrv_iostatus_disable(bs);
> notifier_list_init(&bs->close_notifiers);
> + bs->child = child;
>
> return bs;
> }
>
> +BlockDriverState *bdrv_new(const char *device_name)
> +{
> + return bdrv_new_int(device_name, NULL);
> +}
> +
> void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
> {
> notifier_list_add(&bs->close_notifiers, notify);
> @@ -769,16 +776,8 @@ free_and_fail:
> return ret;
> }
>
> -/*
> - * Opens a file using a protocol (file, host_device, nbd, ...)
> - *
> - * options is a QDict of options to pass to the block drivers, or NULL for an
> - * empty set of options. The reference to the QDict belongs to the block layer
> - * after the call (even on failure), so if the caller intends to reuse the
> - * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
> - */
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> - QDict *options, int flags)
> +static int bdrv_file_open_int(BlockDriverState **pbs, const char *filename,
> + QDict *options, int flags, BlockDriverState *child)
> {
> BlockDriverState *bs;
> BlockDriver *drv;
> @@ -790,7 +789,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> options = qdict_new();
> }
>
> - bs = bdrv_new("");
> + bs = bdrv_new_int("", child);
> bs->options = options;
> options = qdict_clone_shallow(options);
>
> @@ -873,6 +872,20 @@ fail:
> }
>
> /*
> + * Opens a file using a protocol (file, host_device, nbd, ...)
> + *
> + * options is a QDict of options to pass to the block drivers, or NULL for an
> + * empty set of options. The reference to the QDict belongs to the block layer
> + * after the call (even on failure), so if the caller intends to reuse the
> + * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
> + */
> +int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> + QDict *options, int flags)
> +{
> + return bdrv_file_open_int(pbs, filename, options, flags, NULL);
> +}
> +
> +/*
> * Opens the backing file for a BlockDriverState if not yet open
> *
> * options is a QDict of options to pass to the block drivers, or NULL for an
> @@ -904,7 +917,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
> return 0;
> }
>
> - bs->backing_hd = bdrv_new("");
> + bs->backing_hd = bdrv_new_int("", bs);
> bdrv_get_full_backing_filename(bs, backing_filename,
> sizeof(backing_filename));
>
> @@ -990,7 +1003,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
> instead of opening 'filename' directly */
>
> /* if there is a backing file, use it */
> - bs1 = bdrv_new("");
> + bs1 = bdrv_new_int("", bs);
> ret = bdrv_open(bs1, filename, NULL, 0, drv);
> if (ret < 0) {
> bdrv_delete(bs1);
> @@ -1043,9 +1056,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
> }
>
> extract_subqdict(options, &file_options, "file.");
> -
> - ret = bdrv_file_open(&file, filename, file_options,
> - bdrv_open_flags(bs, flags));
> + ret = bdrv_file_open_int(&file, filename, file_options,
> + bdrv_open_flags(bs, flags), bs);
> if (ret < 0) {
> goto fail;
> }
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index ba52247..9c72b32 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -245,6 +245,7 @@ struct BlockDriverState {
>
> BlockDriverState *backing_hd;
> BlockDriverState *file;
> + BlockDriverState *child;
>
> NotifierList close_notifiers;
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event()
2013-06-20 17:46 ` [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() Marc-André Lureau
@ 2013-06-21 7:35 ` Gerd Hoffmann
2013-06-21 8:40 ` Marc-André Lureau
0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2013-06-21 7:35 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: spice-devel, qemu-devel, Marc-André Lureau
Hi,
> +static void spice_chr_fe_event(struct CharDriverState *chr, int event)
> +{
> +#if SPICE_SERVER_VERSION >= 0x000c02
> + SpiceCharDriver *s = chr->opaque;
> +
> + spice_server_port_event(&s->sin, event);
> +#endif
> +}
No way. You are passing an qemu-internal value (event) to an external
library here. That is going to cause major grief in case the internal
change some day, so please don't.
I'd suggest to have something like this instead:
switch (event) {
case OPEN:
spice_server_port_open()
break;
[ ... ]
}
cheers,
Gerd
PS: Small historical lesson: spice-server 0.4.x (IIRC) was full of
these, which was a major blocker of the upstream merge of spice
support. spice-server 0.6.x got a radically different library
interface to fix that. A few places escaped review, so we still
have this in a few minor places, mouse button numbering for
example. Luckily this is a place where changes are unlikely.
But please don't let us add new ones.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-20 22:25 ` Paolo Bonzini
@ 2013-06-21 8:36 ` Marc-André Lureau
2013-06-21 11:57 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-21 8:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: spice-devel, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5882 bytes --]
Hi
On Fri, Jun 21, 2013 at 12:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 20/06/2013 19:46, Marc-André Lureau ha scritto:
> > This allows the Spice block driver to eject the associated device.
>
> The child can change when you have for example a streaming operation.
>
Ah, can you point me to some function or some way I can reproduce? I don't
know what you mean by a streaming operation here.
What exactly are you trying to do here (I guess I'll understand more
> when I get to the later patches)?
>
Getting to the bottom BlockDriverState to be able to eject & change it. (it
could also be named the "parent", but other parts of the code suggest the
"child" name)
>
> Can you draw the relationships between all the BlockDriverStates in a
> spicebd: drive?
>
Hopefully, but I have only tested with raw images (w/wo snapshot).
> Paolo
>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > block.c | 46
> +++++++++++++++++++++++++++++-----------------
> > include/block/block_int.h | 1 +
> > 2 files changed, 30 insertions(+), 17 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index b88ad2f..f502eed 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -294,7 +294,8 @@ void bdrv_register(BlockDriver *bdrv)
> > }
> >
> > /* create a new block device (by default it is empty) */
> > -BlockDriverState *bdrv_new(const char *device_name)
> > +static BlockDriverState *bdrv_new_int(const char *device_name,
> > + BlockDriverState *child)
> > {
> > BlockDriverState *bs;
> >
> > @@ -305,10 +306,16 @@ BlockDriverState *bdrv_new(const char *device_name)
> > }
> > bdrv_iostatus_disable(bs);
> > notifier_list_init(&bs->close_notifiers);
> > + bs->child = child;
> >
> > return bs;
> > }
> >
> > +BlockDriverState *bdrv_new(const char *device_name)
> > +{
> > + return bdrv_new_int(device_name, NULL);
> > +}
> > +
> > void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
> > {
> > notifier_list_add(&bs->close_notifiers, notify);
> > @@ -769,16 +776,8 @@ free_and_fail:
> > return ret;
> > }
> >
> > -/*
> > - * Opens a file using a protocol (file, host_device, nbd, ...)
> > - *
> > - * options is a QDict of options to pass to the block drivers, or NULL
> for an
> > - * empty set of options. The reference to the QDict belongs to the
> block layer
> > - * after the call (even on failure), so if the caller intends to reuse
> the
> > - * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
> > - */
> > -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> > - QDict *options, int flags)
> > +static int bdrv_file_open_int(BlockDriverState **pbs, const char
> *filename,
> > + QDict *options, int flags, BlockDriverState *child)
> > {
> > BlockDriverState *bs;
> > BlockDriver *drv;
> > @@ -790,7 +789,7 @@ int bdrv_file_open(BlockDriverState **pbs, const
> char *filename,
> > options = qdict_new();
> > }
> >
> > - bs = bdrv_new("");
> > + bs = bdrv_new_int("", child);
> > bs->options = options;
> > options = qdict_clone_shallow(options);
> >
> > @@ -873,6 +872,20 @@ fail:
> > }
> >
> > /*
> > + * Opens a file using a protocol (file, host_device, nbd, ...)
> > + *
> > + * options is a QDict of options to pass to the block drivers, or NULL
> for an
> > + * empty set of options. The reference to the QDict belongs to the
> block layer
> > + * after the call (even on failure), so if the caller intends to reuse
> the
> > + * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
> > + */
> > +int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> > + QDict *options, int flags)
> > +{
> > + return bdrv_file_open_int(pbs, filename, options, flags, NULL);
> > +}
> > +
> > +/*
> > * Opens the backing file for a BlockDriverState if not yet open
> > *
> > * options is a QDict of options to pass to the block drivers, or NULL
> for an
> > @@ -904,7 +917,7 @@ int bdrv_open_backing_file(BlockDriverState *bs,
> QDict *options)
> > return 0;
> > }
> >
> > - bs->backing_hd = bdrv_new("");
> > + bs->backing_hd = bdrv_new_int("", bs);
> > bdrv_get_full_backing_filename(bs, backing_filename,
> > sizeof(backing_filename));
> >
> > @@ -990,7 +1003,7 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> > instead of opening 'filename' directly */
> >
> > /* if there is a backing file, use it */
> > - bs1 = bdrv_new("");
> > + bs1 = bdrv_new_int("", bs);
> > ret = bdrv_open(bs1, filename, NULL, 0, drv);
> > if (ret < 0) {
> > bdrv_delete(bs1);
> > @@ -1043,9 +1056,8 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> > }
> >
> > extract_subqdict(options, &file_options, "file.");
> > -
> > - ret = bdrv_file_open(&file, filename, file_options,
> > - bdrv_open_flags(bs, flags));
> > + ret = bdrv_file_open_int(&file, filename, file_options,
> > + bdrv_open_flags(bs, flags), bs);
> > if (ret < 0) {
> > goto fail;
> > }
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index ba52247..9c72b32 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -245,6 +245,7 @@ struct BlockDriverState {
> >
> > BlockDriverState *backing_hd;
> > BlockDriverState *file;
> > + BlockDriverState *child;
> >
> > NotifierList close_notifiers;
> >
> >
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 7668 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event()
2013-06-21 7:35 ` Gerd Hoffmann
@ 2013-06-21 8:40 ` Marc-André Lureau
0 siblings, 0 replies; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-21 8:40 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: spice-devel, qemu-devel, Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]
On Fri, Jun 21, 2013 at 9:35 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> > +static void spice_chr_fe_event(struct CharDriverState *chr, int event)
> > +{
> > +#if SPICE_SERVER_VERSION >= 0x000c02
> > + SpiceCharDriver *s = chr->opaque;
> > +
> > + spice_server_port_event(&s->sin, event);
> > +#endif
> > +}
>
> No way. You are passing an qemu-internal value (event) to an external
> library here. That is going to cause major grief in case the internal
> change some day, so please don't.
>
> I'd suggest to have something like this instead:
>
> switch (event) {
> case OPEN:
> spice_server_port_open()
> break;
> [ ... ]
> }
>
Oh yeah, agreed.
Since the Spice server API already has spice_server_port_event() and
events, I will change it to:
switch (event) {
case OPEN:
spice_server_port_open(SPICE_PORT_EVENT_OPENED)
> cheers,
> Gerd
>
> PS: Small historical lesson: spice-server 0.4.x (IIRC) was full of
> these, which was a major blocker of the upstream merge of spice
> support. spice-server 0.6.x got a radically different library
> interface to fix that. A few places escaped review, so we still
> have this in a few minor places, mouse button numbering for
> example. Luckily this is a place where changes are unlikely.
> But please don't let us add new ones.
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2153 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-21 8:36 ` Marc-André Lureau
@ 2013-06-21 11:57 ` Paolo Bonzini
2013-06-21 13:30 ` Marc-André Lureau
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2013-06-21 11:57 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: spice-devel, qemu-devel, Richard W.M. Jones
Il 21/06/2013 10:36, Marc-André Lureau ha scritto:
> Hi
>
>
> On Fri, Jun 21, 2013 at 12:25 AM, Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
>
> Il 20/06/2013 19:46, Marc-André Lureau ha scritto:
> > This allows the Spice block driver to eject the associated device.
>
> The child can change when you have for example a streaming operation.
>
> Ah, can you point me to some function or some way I can reproduce? I
> don't know what you mean by a streaming operation here.
Let's just discuss the design so that it just works and you don't have
to worry about it. :)
> What exactly are you trying to do here (I guess I'll understand more
> when I get to the later patches)?
>
> Getting to the bottom BlockDriverState to be able to eject & change it.
> (it could also be named the "parent", but other parts of the code
> suggest the "child" name)
There is already an interface for eject/change, which is the monitor.
To signal an eject or medium change, the SPICE client should simply ask
libvirt to do so. It is fine to change "from" spicebd: "to" spicebd:,
the guest would still perceive it as a change.
I'm not sure how libvirt communicates the change back to the SPICE
client, and whether it is asynchronous or synchronous.
BTW, note that IDE or virtio-blk do not support removable media, and are
not able to pass eject or media change notifications. SCSI devices
(such as usb-bot, usb-uas and virtio-scsi) can.
> Can you draw the relationships between all the BlockDriverStates in a
> spicebd: drive?
>
>
> Hopefully, but I have only tested with raw images (w/wo snapshot).
Then draw it. :) But from the above description it looks like it is not
necessary, it should simply be "raw" on top of "spicebd". Parsing
formats should be done on the client side, perhaps by invoking qemu-nbd
and tunnelling the NBD data on the SPICE channel.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-21 11:57 ` Paolo Bonzini
@ 2013-06-21 13:30 ` Marc-André Lureau
2013-06-21 13:39 ` Paolo Bonzini
0 siblings, 1 reply; 20+ messages in thread
From: Marc-André Lureau @ 2013-06-21 13:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: spice-devel, qemu-devel, Richard W.M. Jones
[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]
Hi
On Fri, Jun 21, 2013 at 1:57 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> > What exactly are you trying to do here (I guess I'll understand more
> > when I get to the later patches)?
> >
> > Getting to the bottom BlockDriverState to be able to eject & change it.
> > (it could also be named the "parent", but other parts of the code
> > suggest the "child" name)
>
> There is already an interface for eject/change, which is the monitor.
> To signal an eject or medium change, the SPICE client should simply ask
> libvirt to do so. It is fine to change "from" spicebd: "to" spicebd:,
> the guest would still perceive it as a change.
>
> I'm not sure how libvirt communicates the change back to the SPICE
> client, and whether it is asynchronous or synchronous.
>
>
Spice is not using libvirt, and doesn't have access to monitor. I looked at
the monitor code. Basically, the spice block driver I proposed uses a
similar code to forcefully eject and change.
Perhaps it's possible for the Spice qemu-side code to have access to the
monitor, but the end result would be similar anyway.
> BTW, note that IDE or virtio-blk do not support removable media, and are
> not able to pass eject or media change notifications. SCSI devices
> (such as usb-bot, usb-uas and virtio-scsi) can.
>
> > Can you draw the relationships between all the BlockDriverStates in a
> > spicebd: drive?
> >
> >
> > Hopefully, but I have only tested with raw images (w/wo snapshot).
>
> Then draw it. :) But from the above description it looks like it is not
> necessary, it should simply be "raw" on top of "spicebd". Parsing
> formats should be done on the client side, perhaps by invoking qemu-nbd
> and tunnelling the NBD data on the SPICE channel.
>
>
Right, "raw" on top of "spicebd" for iso/raw images (and "qcow2" on top for
snapshot support - even in readonly).
I wonder what would happen if spicebd image would be something else than
raw/iso, I suppose there would be more BlockDriverStates to handle the
various format.
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2941 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState
2013-06-21 13:30 ` Marc-André Lureau
@ 2013-06-21 13:39 ` Paolo Bonzini
0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2013-06-21 13:39 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: spice-devel, qemu-devel, Richard W.M. Jones
Il 21/06/2013 15:30, Marc-André Lureau ha scritto:
> > Getting to the bottom BlockDriverState to be able to eject &
> > change it. (it could also be named the "parent", but other parts
> > of the code suggest the "child" name)
>
> There is already an interface for eject/change, which is the monitor.
> To signal an eject or medium change, the SPICE client should simply ask
> libvirt to do so. It is fine to change "from" spicebd: "to" spicebd:,
> the guest would still perceive it as a change.
>
> I'm not sure how libvirt communicates the change back to the SPICE
> client, and whether it is asynchronous or synchronous.
>
> Spice is not using libvirt, and doesn't have access to monitor. I looked
> at the monitor code. Basically, the spice block driver I proposed uses a
> similar code to forcefully eject and change.
Similar, but it also violates encapsulation by adding bs->child. :)
> Perhaps it's possible for the Spice qemu-side code to have access to the
> monitor, but the end result would be similar anyway.
That would work better. Spice qemu-side would keep an association
between Spice channels and block device names, and use that to convert
Spice commands to monitor commands (the API is
qmp_change_blockdev/qmp_eject).
Paolo
>
> BTW, note that IDE or virtio-blk do not support removable media, and are
> not able to pass eject or media change notifications. SCSI devices
> (such as usb-bot, usb-uas and virtio-scsi) can.
>
> > Can you draw the relationships between all the
> BlockDriverStates in a
> > spicebd: drive?
> >
> >
> > Hopefully, but I have only tested with raw images (w/wo snapshot).
>
> Then draw it. :) But from the above description it looks like it is not
> necessary, it should simply be "raw" on top of "spicebd". Parsing
> formats should be done on the client side, perhaps by invoking qemu-nbd
> and tunnelling the NBD data on the SPICE channel.
>
>
> Right, "raw" on top of "spicebd" for iso/raw images (and "qcow2" on top
> for snapshot support - even in readonly).
>
> I wonder what would happen if spicebd image would be something else than
> raw/iso, I suppose there would be more BlockDriverStates to handle the
> various format.
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-06-21 13:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-20 17:45 [Qemu-devel] [PATCH 00/12] RFC: add Spice block device Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() Marc-André Lureau
2013-06-21 7:35 ` Gerd Hoffmann
2013-06-21 8:40 ` Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 03/12] nbd: don't change socket block during negotiate Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 04/12] Split nbd block client code Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 05/12] nbd: pass export name as init argument Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 06/12] nbd: make session_close() idempotent Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState Marc-André Lureau
2013-06-20 22:25 ` Paolo Bonzini
2013-06-21 8:36 ` Marc-André Lureau
2013-06-21 11:57 ` Paolo Bonzini
2013-06-21 13:30 ` Marc-André Lureau
2013-06-21 13:39 ` Paolo Bonzini
2013-06-20 17:46 ` [Qemu-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open() Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 09/12] block: add "snapshot.size" option to avoid extra bdrv_open() Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 10/12] block: learn to open a driver with a given opaque Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque Marc-André Lureau
2013-06-20 17:46 ` [Qemu-devel] [PATCH 12/12] block: add spice block device backend Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).