Re: [Qemu-devel] [PATCH v3 11/17] virtio-serial: switch to standard-headers

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Feb 18, 2015 at 03:34:13PM +0100, Thomas Huth wrote:
> On Mon, 16 Feb 2015 22:36:26 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > Drop duplicate code.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/hw/virtio/virtio-serial.h | 40 +--------------------------------------
> >  hw/char/virtio-serial-bus.c       |  1 +
> >  2 files changed, 2 insertions(+), 39 deletions(-)
> > 
> > diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> > index 11af978..ccf8459 100644
> > --- a/include/hw/virtio/virtio-serial.h
> > +++ b/include/hw/virtio/virtio-serial.h
> > @@ -15,53 +15,15 @@
> >  #ifndef _QEMU_VIRTIO_SERIAL_H
> >  #define _QEMU_VIRTIO_SERIAL_H
> > 
> > +#include "standard-headers/linux/virtio_console.h"
> >  #include "hw/qdev.h"
> >  #include "hw/virtio/virtio.h"
> > 
> > -/* == Interface shared between the guest kernel and qemu == */
> > -
> > -/* The Virtio ID for virtio console / serial ports */
> > -#define VIRTIO_ID_CONSOLE		3
> > -
> > -/* Features supported */
> > -#define VIRTIO_CONSOLE_F_MULTIPORT	1
> > -
> > -#define VIRTIO_CONSOLE_BAD_ID           (~(uint32_t)0)
> > -
> > -struct virtio_console_config {
> > -    /*
> > -     * These two fields are used by VIRTIO_CONSOLE_F_SIZE which
> > -     * isn't implemented here yet
> > -     */
> > -    uint16_t cols;
> > -    uint16_t rows;
> > -
> > -    uint32_t max_nr_ports;
> > -} QEMU_PACKED;
> > -
> > -struct virtio_console_control {
> > -    uint32_t id;		/* Port number */
> > -    uint16_t event;		/* The kind of control event (see below) */
> > -    uint16_t value;		/* Extra information for the key */
> > -};
> > -
> >  struct virtio_serial_conf {
> >      /* Max. number of ports we can have for a virtio-serial device */
> >      uint32_t max_virtserial_ports;
> >  };
> > 
> > -/* Some events for the internal messages (control packets) */
> > -#define VIRTIO_CONSOLE_DEVICE_READY	0
> > -#define VIRTIO_CONSOLE_PORT_ADD		1
> > -#define VIRTIO_CONSOLE_PORT_REMOVE	2
> > -#define VIRTIO_CONSOLE_PORT_READY	3
> > -#define VIRTIO_CONSOLE_CONSOLE_PORT	4
> > -#define VIRTIO_CONSOLE_RESIZE		5
> > -#define VIRTIO_CONSOLE_PORT_OPEN	6
> > -#define VIRTIO_CONSOLE_PORT_NAME	7
> > -
> > -/* == In-qemu interface == */
> > -
> >  #define TYPE_VIRTIO_SERIAL_PORT "virtio-serial-port"
> >  #define VIRTIO_SERIAL_PORT(obj) \
> >       OBJECT_CHECK(VirtIOSerialPort, (obj), TYPE_VIRTIO_SERIAL_PORT)
> > diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> > index 47fbb34..a2bac9b 100644
> > --- a/hw/char/virtio-serial-bus.c
> > +++ b/hw/char/virtio-serial-bus.c
> > @@ -18,6 +18,7 @@
> >   * GNU GPL, version 2 or (at your option) any later version.
> >   */
> > 
> > +#include "standard-headers/linux/virtio_ids.h"
> 
> Could you omit this include? Later in virtio-serial-bus.c, the code
> includes hw/virtio/virtio-serial.h, which in turn includes
> standard-headers/linux/virtio_console.h - and that one already includes
> the ids.h file. So I think it should work without the above change, too.
> 
>  Thomas

Yes but it's generally not a good idea to depend on headers
including each other.

-- 
MST