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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 03/22/2013 02:50 PM, Anthony Liguori wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>
>> Hi,
>>
>> On 03/21/2013 07:18 PM, Anthony Liguori wrote:
>>> 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.
>>
>> 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.

A pipe does not have 2 ends 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.

The notion / concept of both ends of the pipe being opened and closed
certainly does exist. See your own example of a plain 16x50 uart and
the guest using it or not using it. Just because we cannot reliable /
practically detect the open/close does not mean the concept of it is not
there.

>  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().

1) If the guest does not have an uart driver, nothing will be written
to the uart, so it wont call qemu_chr_fe_write and we won't assume
a qemu_chr_fe_open

2) But I agree the assuming of qemu_chr_fe_open on the first write is
a hack, it stems from before we had qemu_chr_fe_open/_close and now that
we do have we should remove it.

Note btw that many backends also don't handle sending CHR_EVENT_OPENED /
_CLOSED themselves, they simply use qemu_chr_generic_open and that generated
the opened event once on creation of the device. So the concept is just
as broken / not broken on the backend side.

We could do the same and have a qemu_fe_generic_open for frontends which
does the qemu_chr_fe_open call.

<snip>

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

With the device I assume you mean the frontend? That is what we originally
suggested and submitted a patch for (for virtio-console) but Amit didn't
like it.

Regards,

Hans