Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected

[Anthony Liguori <aliguori@us.ibm.com>]

Alon Levy <alevy@redhat.com> writes:

> Note that the handler is called chr_is_guest_connected and not
> chr_is_fe_connected, consistent with other members of CharDriverState.

Sorry, I don't get it.

There isn't a notion of "connected" for the front-ends in the char
layer.  The closest thing is whether add_handlers() have been called or
not.

I really dislike the idea of introduction a new concept to the char
layer in a half baked way.

Why can't migration notifiers be used for this?  I think Gerd objected
to using a migration *handler* but not necessarily a state notifier.

Regards,

Anthony Liguori

>
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/virtio-console.c |  9 +++++++++
>  include/char/char.h | 11 +++++++++++
>  qemu-char.c         |  9 +++++++++
>  3 files changed, 29 insertions(+)
>
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index e2d1c58..643e24e 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -120,6 +120,13 @@ static void chr_event(void *opaque, int event)
>      }
>  }
>  
> +static bool chr_is_guest_connected(void *opaque)
> +{
> +    VirtConsole *vcon = opaque;
> +
> +    return vcon->port.guest_connected;
> +}
> +
>  static int virtconsole_initfn(VirtIOSerialPort *port)
>  {
>      VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> @@ -133,6 +140,8 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
>      if (vcon->chr) {
>          qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
>                                vcon);
> +        /* only user of chr_is_guest_connected so leave it as special cased*/
> +        vcon->chr->chr_is_guest_connected = chr_is_guest_connected;
>      }
>  
>      return 0;
> diff --git a/include/char/char.h b/include/char/char.h
> index 0326b2a..b41ddc0 100644
> --- a/include/char/char.h
> +++ b/include/char/char.h
> @@ -52,6 +52,7 @@ typedef struct {
>  #define CHR_TIOCM_RTS	0x004
>  
>  typedef void IOEventHandler(void *opaque, int event);
> +typedef bool IOIsGuestConnectedHandler(void *opaque);
>  
>  struct CharDriverState {
>      void (*init)(struct CharDriverState *s);
> @@ -64,6 +65,7 @@ struct CharDriverState {
>      IOEventHandler *chr_event;
>      IOCanReadHandler *chr_can_read;
>      IOReadHandler *chr_read;
> +    IOIsGuestConnectedHandler *chr_is_guest_connected;
>      void *handler_opaque;
>      void (*chr_close)(struct CharDriverState *chr);
>      void (*chr_accept_input)(struct CharDriverState *chr);
> @@ -229,6 +231,15 @@ void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
>   */
>  void qemu_chr_be_event(CharDriverState *s, int event);
>  
> +/**
> + * @qemu_chr_be_is_fe_connected:
> + *
> + * Back end calls this to check if the front end is connected.
> + *
> + * Returns: true if the guest (front end) is connected, false otherwise.
> + */
> +bool qemu_chr_be_is_fe_connected(CharDriverState *s);
> +
>  void qemu_chr_add_handlers(CharDriverState *s,
>                             IOCanReadHandler *fd_can_read,
>                             IOReadHandler *fd_read,
> diff --git a/qemu-char.c b/qemu-char.c
> index 4e011df..77a501a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -120,6 +120,15 @@ void qemu_chr_be_event(CharDriverState *s, int event)
>      s->chr_event(s->handler_opaque, event);
>  }
>  
> +bool qemu_chr_be_is_fe_connected(CharDriverState *s)
> +{
> +    if (s->chr_is_guest_connected) {
> +        return s->chr_is_guest_connected(s->handler_opaque);
> +    }
> +    /* default to always connected */
> +    return true;
> +}
> +
>  static gboolean qemu_chr_generic_open_bh(gpointer opaque)
>  {
>      CharDriverState *s = opaque;
> -- 
> 1.8.1.4