Re: [Spice-devel] [Engine-devel] SPICE Foreign Menu Using REST

2013-06-20 Thread Christophe Fergeau
Hey Itamar,

On Wed, Jun 19, 2013 at 06:00:47PM +0300, Itamar Heim wrote:
 On 06/19/2013 04:33 PM, Tomas Jelinek wrote:
 Hi all,
 
 for the non plugin invocation of the console 
 (http://www.ovirt.org/Features/Non_plugin_console_invocation) the 
 dynamicMenu property is not usable to enrich the SPICE client menu.
 
 This feature is about requesting this menu using oVirt's REST API and than 
 calling it back once some item from this menu has been selected.
 
 The oVirt's feature page with specific details: 
 http://www.ovirt.org/Features/Foreign_Menu_Using_REST
 
 Any comments are more than welcomed.
 
 Why are we putting the logic of the menu into the REST API?
 can't the menu be builded by client based on getting the list of
 ISO's and current CD?

The client needs a way to get the list of available ISOs in order to build
the menu, as well as a way to let the engine know when the user clicked on
an ISO in the menu. One way of doing that is to have the menu stuff exposed
in the REST API. Another way would be to expose the list of ISOs in the
REST API (I'd have a slight preference for that I think).
Or do you have anything in mind for the client to get the list of ISOs
available to the engine?

Christophe


pgpvc5GXRZRae.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [Engine-devel] SPICE Foreign Menu Using REST

2013-06-20 Thread Itamar Heim

On 06/20/2013 02:07 PM, Christophe Fergeau wrote:

Hey Itamar,

On Wed, Jun 19, 2013 at 06:00:47PM +0300, Itamar Heim wrote:

On 06/19/2013 04:33 PM, Tomas Jelinek wrote:

Hi all,

for the non plugin invocation of the console 
(http://www.ovirt.org/Features/Non_plugin_console_invocation) the dynamicMenu 
property is not usable to enrich the SPICE client menu.

This feature is about requesting this menu using oVirt's REST API and than 
calling it back once some item from this menu has been selected.

The oVirt's feature page with specific details: 
http://www.ovirt.org/Features/Foreign_Menu_Using_REST

Any comments are more than welcomed.


Why are we putting the logic of the menu into the REST API?
can't the menu be builded by client based on getting the list of
ISO's and current CD?


The client needs a way to get the list of available ISOs in order to build
the menu, as well as a way to let the engine know when the user clicked on
an ISO in the menu. One way of doing that is to have the menu stuff exposed
in the REST API. Another way would be to expose the list of ISOs in the
REST API (I'd have a slight preference for that I think).


that's my preference as well, and the REST API should already do that today:
- get list of ISOs of available in a DC (the one the VM is in)
- ability to change the CD of the VM


Or do you have anything in mind for the client to get the list of ISOs
available to the engine?


my preference is to provide the data, not the semantics of the menu.
iiuc, we are on the same page.



Christophe



___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [Engine-devel] SPICE Foreign Menu Using REST

2013-06-20 Thread Tomas Jelinek


- Original Message -
 From: Itamar Heim ih...@redhat.com
 To: Christophe Fergeau cferg...@redhat.com
 Cc: Tomas Jelinek tjeli...@redhat.com, engine-devel 
 engine-de...@ovirt.org,
 spice-devel@lists.freedesktop.org, Marc-André Lureau mlur...@redhat.com
 Sent: Thursday, June 20, 2013 1:09:21 PM
 Subject: Re: [Engine-devel] [Spice-devel] SPICE Foreign Menu Using REST
 
 On 06/20/2013 02:07 PM, Christophe Fergeau wrote:
  Hey Itamar,
 
  On Wed, Jun 19, 2013 at 06:00:47PM +0300, Itamar Heim wrote:
  On 06/19/2013 04:33 PM, Tomas Jelinek wrote:
  Hi all,
 
  for the non plugin invocation of the console
  (http://www.ovirt.org/Features/Non_plugin_console_invocation) the
  dynamicMenu property is not usable to enrich the SPICE client menu.
 
  This feature is about requesting this menu using oVirt's REST API and
  than calling it back once some item from this menu has been selected.
 
  The oVirt's feature page with specific details:
  http://www.ovirt.org/Features/Foreign_Menu_Using_REST
 
  Any comments are more than welcomed.
 
  Why are we putting the logic of the menu into the REST API?
  can't the menu be builded by client based on getting the list of
  ISO's and current CD?
 
  The client needs a way to get the list of available ISOs in order to build
  the menu, as well as a way to let the engine know when the user clicked on
  an ISO in the menu. One way of doing that is to have the menu stuff exposed
  in the REST API. Another way would be to expose the list of ISOs in the
  REST API (I'd have a slight preference for that I think).
 
 that's my preference as well, and the REST API should already do that today:
 - get list of ISOs of available in a DC (the one the VM is in)
 - ability to change the CD of the VM
Well, and what about the stop/suspend VM for example, what we have in the menu 
today?
Also this should be something that the client decide to have or not have?

 
  Or do you have anything in mind for the client to get the list of ISOs
  available to the engine?
 
 my preference is to provide the data, not the semantics of the menu.
We should clarify something here. There are two possibilities who should be 
responsible for deciding
what should the foreign menu contain and also what to do if it the item is 
selected:
1: the client - e.g. we will not provide any support for the menu in REST 
because the client reads what needs
   and knows how to call the oVirt back

2: the server side - e.g. we will provide in REST side the menu structure with 
the description what to do if something is
   selected

Both have some advantages and disadvantages but we need to clarify which way to 
go. So, who is the one responsible to decide
what is in the foreign menu (now or in the future) and how to react if 
something is selected? Client or engine?

 iiuc, we are on the same page.
 
 
  Christophe
 
 
 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [Engine-devel] SPICE Foreign Menu Using REST

2013-06-20 Thread Itamar Heim

On 06/20/2013 03:13 PM, Tomas Jelinek wrote:



- Original Message -

From: Itamar Heim ih...@redhat.com
To: Christophe Fergeau cferg...@redhat.com
Cc: Tomas Jelinek tjeli...@redhat.com, engine-devel 
engine-de...@ovirt.org,
spice-devel@lists.freedesktop.org, Marc-André Lureau mlur...@redhat.com
Sent: Thursday, June 20, 2013 1:09:21 PM
Subject: Re: [Engine-devel] [Spice-devel] SPICE Foreign Menu Using REST

On 06/20/2013 02:07 PM, Christophe Fergeau wrote:

Hey Itamar,

On Wed, Jun 19, 2013 at 06:00:47PM +0300, Itamar Heim wrote:

On 06/19/2013 04:33 PM, Tomas Jelinek wrote:

Hi all,

for the non plugin invocation of the console
(http://www.ovirt.org/Features/Non_plugin_console_invocation) the
dynamicMenu property is not usable to enrich the SPICE client menu.

This feature is about requesting this menu using oVirt's REST API and
than calling it back once some item from this menu has been selected.

The oVirt's feature page with specific details:
http://www.ovirt.org/Features/Foreign_Menu_Using_REST

Any comments are more than welcomed.


Why are we putting the logic of the menu into the REST API?
can't the menu be builded by client based on getting the list of
ISO's and current CD?


The client needs a way to get the list of available ISOs in order to build
the menu, as well as a way to let the engine know when the user clicked on
an ISO in the menu. One way of doing that is to have the menu stuff exposed
in the REST API. Another way would be to expose the list of ISOs in the
REST API (I'd have a slight preference for that I think).


that's my preference as well, and the REST API should already do that today:
- get list of ISOs of available in a DC (the one the VM is in)
- ability to change the CD of the VM

Well, and what about the stop/suspend VM for example, what we have in the menu 
today?


supported as well.


Also this should be something that the client decide to have or not have?


I think the client should decide on the features relevant to the client.






Or do you have anything in mind for the client to get the list of ISOs
available to the engine?


my preference is to provide the data, not the semantics of the menu.

We should clarify something here. There are two possibilities who should be 
responsible for deciding
what should the foreign menu contain and also what to do if it the item is 
selected:
1: the client - e.g. we will not provide any support for the menu in REST 
because the client reads what needs
and knows how to call the oVirt back

2: the server side - e.g. we will provide in REST side the menu structure with 
the description what to do if something is
selected

Both have some advantages and disadvantages but we need to clarify which way to 
go. So, who is the one responsible to decide
what is in the foreign menu (now or in the future) and how to react if 
something is selected? Client or engine?


my preference is the client will decide this, as there are various 
clients to the REST API, and each should decide which features are 
important for them.





iiuc, we are on the same page.



Christophe






___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 00/12] RFC: add Spice block device

2013-06-20 Thread 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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 01/12] include: add missing config-host.h include

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 02/12] char: add qemu_chr_fe_event()

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 03/12] nbd: don't change socket block during negotiate

2013-06-20 Thread 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.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 04/12] Split nbd block client code

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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 000..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.viv...@bull.net
+ *
+ * Some parts:
+ *Copyright (C) 2007 Anthony Liguori anth...@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;
+

[Spice-devel] [PATCH 06/12] nbd: make session_close() idempotent

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 05/12] nbd: pass export name as init argument

2013-06-20 Thread 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.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 07/12] block: save the associated child in BlockDriverState

2013-06-20 Thread Marc-André Lureau
This allows the Spice block driver to eject the associated device.

Signed-off-by: Marc-André Lureau marcandre.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open()

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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 

[Spice-devel] [PATCH 09/12] block: add snapshot.size option to avoid extra bdrv_open()

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 10/12] block: learn to open a driver with a given opaque

2013-06-20 Thread 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.lur...@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 = 

[Spice-devel] [PATCH 12/12] block: add spice block device backend

2013-06-20 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau marcandre.lur...@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 000..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.lur...@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]);
+

[Spice-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque

2013-06-20 Thread 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.lur...@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

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk 1/4] smartcard: Report failure when software reader is missing

2013-06-20 Thread Marc-André Lureau
ack series,



On Wed, Jun 19, 2013 at 5:33 PM, Christophe Fergeau cferg...@redhat.comwrote:

 As there is no easy way to know if the SpiceSmartcardManager
 has an associated software reader or not, it's better to report
 failure instead of g_return_if_fail on attempts to use
 spice_smartcard_manager_insert/remove_card with no software reader
 available.
 ---
  gtk/smartcard-manager.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/gtk/smartcard-manager.c b/gtk/smartcard-manager.c
 index 51f228a..3b86dfb 100644
 --- a/gtk/smartcard-manager.c
 +++ b/gtk/smartcard-manager.c
 @@ -552,7 +552,8 @@ gboolean
 spice_smartcard_manager_insert_card(SpiceSmartcardManager *manager)
  {
  VCardEmulError status;

 -g_return_val_if_fail(manager-priv-software_reader != NULL, FALSE);
 +if (manager-priv-software_reader != NULL)
 +return FALSE;

  status = vcard_emul_force_card_insert(manager-priv-software_reader);

 @@ -574,7 +575,8 @@ gboolean
 spice_smartcard_manager_remove_card(SpiceSmartcardManager *manager)
  {
  VCardEmulError status;

 -g_return_val_if_fail(manager-priv-software_reader != NULL, FALSE);
 +if (manager-priv-software_reader != NULL)
 +return FALSE;

  status = vcard_emul_force_card_remove(manager-priv-software_reader);

 --
 1.8.2.1

 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel




-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH 07/12] block: save the associated child in BlockDriverState

2013-06-20 Thread Paolo Bonzini
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.lur...@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;
  
 

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel