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

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

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> 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.
>
> It isn't new.  There are qemu_chr_fe_open + qemu_chr_fe_close doing
> exactly that signaling (whenever someone has opened the virtio-serial
> port, i.e. whenever the guest agent is active or not).

anthony@titi:~/git/qemu$ find -name "*.[ch]" -exec grep -l qemu_chr_fe_open {} \;
./qemu-char.c
./hw/virtio-console.c
./hw/usb/redirect.c
./include/char/char.h

And:

anthony@titi:~/git/qemu$ find . -name "*.[ch]" -exec grep -l chr_guest_open  {} \;
./qemu-char.c
./include/char/char.h
./spice-qemu-char.c

So this whole interface exists specificially for the spice-qemu-char
backend and only for that backend.  This isn't a generic char layer
concept.  It's a thinly veiled call into spice.

Now spice appears to have a work around in it where the first write will
trigger affectively the same action as doing an open.  Presumably this
makes it usable with !virtio-serial or !usb-redir.

If we want to extend this concept of fe open, let's do it right.  Let's
make all char frontends explicitly open, close on reset, etc.

Let's do something that makes sense across the board.

>
> Problem is the chardev forgets this state over migration.
>
> So we need a way to restore the state.  virtio-serial knows it of course
> as this is guest state it has to keep track of it.  We just need a way
> the chardev can figure what the current state is after migration.
>
> The options I see:
>
>   (1) Have chardev store the state itself in a new migration section.
>       This is what I've NAK'ed.  First, because live-migration host
>       state is a can of worms I don't want open.  Second because it
>       actually isn't host state.
>
>   (2) virtio-serial could just call qemu_chr_fe_open in
>       post_migration hook.  Could have unwanted side effects as
>       the chardev can't figure whenever this is a post-load call
>       or a guest-open call.

The only implementation of qemu_chr_fe_open() is:

static void spice_chr_guest_open(struct CharDriverState *chr)
{
    SpiceCharDriver *s = chr->opaque;
    vmc_register_interface(s);
}

Is this what you would do anyway?

In a world where qemu_chr_fe_open() was actually used throughout the
device model, all devices would need to have a post_load()
implementation where they checked their state to determine whether to
generate an open() call.

Regards,

Anthony Liguori

>
>   (3) Add a new qemu_chr_fe_* call for virtio-serial to notify the
>       chardev.  This was the next proposal from Alon
>
>   (4) Do it the other way around:  Allow the chardev query what the
>       current state is.  This patch series.
>
>   (5) Cut off the chardev layer altogether and implement spicevmc as
>       virtio-serial port.  Your proposal if I understand it correctly.
>       Makes sense, but breaks backward compatibility, so even when
>       doing this we must fix the chardev spicevmc bug somehow.
>
> Hope this clarifies,
>   Gerd