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

[Anthony Liguori]

Hans de Goede <address@hidden> writes:

> Hi,
>
> On 03/21/2013 07:18 PM, Anthony Liguori wrote:
>> Alon Levy <address@hidden> 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.
>
> And that is simply completely and utterly wrong. We've tried to explain
> this to you a number of times already. Yet you claim we've not explained
> it. Or we've not explained it properly. I must say however that I'm
> getting the feeling the problem is not us not explaining, but that you
> are not listening.

As usual, you make way too many assumptions without stopping to actually
think about what I'm saying.

> Still let me try to explain it 1 more time, in 2 different ways:
>
> 1) At an abstract level a chardev fe + be is a pipe between somewhere
> and where-ever. Both ends of the pipe can be opened or closed, not just
> one.

No, this is not the case in reality.  The notion of the front-end being
open or closed only exists for virtio-serial, usb-redir, and spice-*.

For every other aspect of the character device layer, the concept
doesn't exist.  We should have never allowed that in the first place and
I object strongly to extending the concept without making it make sense
for everything else.

> Frontends end inside the guest usually in the form of some piece of
> virtual hardware inside the guest. Just because the virtual hardware
> is there does not mean the guest has a driver,

Okay, let's use your example here with a standard UART.  In the
following sequence, I should receive:

1) Starts guest
2) When guest initializes the UART, qemu_chr_fe_open()
3) Reboot guest
4) Receive qemu_chr_fe_close()
5) Boot new guest without a UART driver
6) Nothing is received

But this doesn't happen today because the only backend that cares
(spice-*) assumes qemu_chr_fe_open() on the first qemu_chr_fe_write().
So it will certainly approximate steps 1-3 but will not behave correct
with steps 4-6.

Likewise, if you are dealing with something like a PCI hotpluggable UART
and the card is ejected (but the CDS isn't deleted), you won't receive a
close() event either.

Even just in the world of spice-* which is all ya'll seem to care about,
the behavior is broken today.

So before we go adding more hacks just for virtio-serial/spice-*, let's
consider how this applies to all character devices and do what makes
sense for all of them.

And for me, the most logical thing is to call qemu_chr_fe_open() in
post_load for the device.

Regards,

Anthony Liguori

> if the guest has
> a driver that creates a device-node for it, that does not mean there
> is a userspace process inside the guest which actually has the
> device-node open. So you see the front-end device-node inside the guest
> can be opened and closed. Most frontends cannot signal this to the
> backend but virtio-console can, and we want to know about it since
> we want to know if the user-space agent is there.



>
> 2) At the code level it clearly is there too, backend open-close
> is signalled to the frontend by means of the backend calling
> qemu_chr_be_event with an event of CHR_EVENT_OPENED and
> CHR_EVENT_CLOSED. Frontend open-close is signalled by the
> frontend calling qemu_chr_fe_open and qemu_chr_fe_close.
>
> Now the problem we have is that after migration the CHR_EVENT_OPENED
> gets replayed on the destination side but the qemu_chr_fe_open call
> does not.
>
> The logical place to replay the qemu_chr_fe_open would be in the
> frontend's migration handling code, as suggested here:
> http://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02721.html
>
> But Amit did not like this approach, suggesting that instead we
> took care of this inside spice-qemu-char.c. Which is what we're
> trying to do now, but this requires spice-qemu-char.c being
> able to query the open state of the frontend, which is already
> being migrate by the virtio-console code in the form of the
> guest_connected variable, we just need to be able to get to that
> variable from the backend.
>
> Regards,
>
> Hans