On Thu, Dec 16, 2010 at 02:21:16PM +0100, Gerd Hoffmann wrote:
> On 12/16/10 12:29, Alon Levy wrote:
> >Adding a chardev backend for spice, for usage by spice vdagent in
> >conjunction with a properly named virtio-serial device.
>
> Usage example would be nice here.
>
> >+#ifdef CONFIG_SPICE
> >+#include "spice-qemu-char.h"
> >+#endif
>
> #ifdef can be dropped.
>
ok.
> >+#ifdef CONFIG_SPICE
> >+ {
> >+ .name = "name",
> >+ .type = QEMU_OPT_STRING,
> >+ },{
> >+ .name = "debug",
> >+ .type = QEMU_OPT_NUMBER,
> >+ },
> >+#endif
>
> This too.
ok.
>
> >@@ -1381,7 +1384,10 @@ Backend is one of:
> > @option{stdio},
> > @option{braille},
> > @option{tty},
> >-...@option{parport}.
> >+...@option{parport}
> >+#if defined(CONFIG_SPICE)
> >+...@option{spicevmc}.
> >+#endif
>
> This too, documentation should be there unconditionally.
>
ok.
> >+//#define SPICE_QEMU_CHAR_USE_IOCTL
>
> Why is this disabled?
> Does it depend on the chardev patches from Amit?
>
There was a long discussion that concluded we don't want IOCTL's at all,
and that there should be some other mechanism for connection state
communication between the two sides. Meanwhile I found out I don't need
these (I don't remember exactly what I used instead, but basically just
the regular results of write/read).
> >diff --git a/spice-qemu-char.h b/spice-qemu-char.h
> >new file mode 100644
> >index 0000000..32d5009
> >--- /dev/null
> >+++ b/spice-qemu-char.h
> >@@ -0,0 +1,9 @@
> >+#ifndef __SPICE_QEMU_CHAR_H__
> >+#define __SPICE_QEMU_CHAR_H__
> >+
> >+#include "qemu-char.h"
> >+
> >+CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
> >+
> >+#endif // __SPICE_QEMU_CHAR_H__
> >+
>
> Hmm, maybe add this to ui/qemu-spice.h instead, so we don't clutter
> the tree with lots of tiny includes?
ok.
>
> cheers,
> Gerd
