[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event()
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() |
Date: |
Fri, 21 Jun 2013 09:35:13 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130513 Thunderbird/17.0.6 |
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.
- [Qemu-devel] [PATCH 00/12] RFC: add Spice block device, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 01/12] include: add missing config-host.h include, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 10/12] block: learn to open a driver with a given opaque, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 08/12] block: extract make_snapshot() from bdrv_open(), Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 12/12] block: add spice block device backend, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 06/12] nbd: make session_close() idempotent, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 04/12] Split nbd block client code, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 03/12] nbd: don't change socket block during negotiate, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event(), Marc-André Lureau, 2013/06/20
- Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event(),
Gerd Hoffmann <=
- [Qemu-devel] [PATCH 11/12] block: allow to call bdrv_open() with an opaque, Marc-André Lureau, 2013/06/20
- [Qemu-devel] [PATCH 07/12] block: save the associated child in BlockDriverState, Marc-André Lureau, 2013/06/20
[Qemu-devel] [PATCH 09/12] block: add "snapshot.size" option to avoid extra bdrv_open(), Marc-André Lureau, 2013/06/20
[Qemu-devel] [PATCH 05/12] nbd: pass export name as init argument, Marc-André Lureau, 2013/06/20