[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected |
Date: |
Fri, 22 Mar 2013 12:11:48 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Hans de Goede <address@hidden> writes:
> Hi,
>
> On 03/22/2013 02:50 PM, Anthony Liguori wrote:
>> Hans de Goede <address@hidden> writes:
>>
>> 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.
If the qemu-char.c code did:
int qemu_chr_fe_write(...) {
if (!s->fe_open) {
qemu_chr_fe_open(s);
}
...
}
That would be one thing. It's a hack, but a more reasonable hack than
doing this in the backend like we do today.
And in fact, herein lies exactly what the problem is. There is no
"s->fe_open". And if such a thing did exist, we would note that this is
in fact guest state and that it needs to be taken care of during
migration.
> 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.
I know :-/
> We could do the same and have a qemu_fe_generic_open for frontends which
> does the qemu_chr_fe_open call.
You mean, in qemu_chr_fe_write()? Yes, I think not doing this bit in
the backend and making it part of qemu-char would be a significant
improvement and would also guide in solving this problem correctly.
Also, you can get reset handling for free by unconditionally clearly
s->fe_open with a reset handler. That would also simplify the
virtio-serial code since it would no longer need the reset logic.
>> 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.
As Avi would say, this is "derived guest state" and derived guest state
has to be recomputed in post_load handlers.
Regards,
Anthony Liguori
>
> Regards,
>
> Hans
- [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, (continued)
- [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Alon Levy, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Anthony Liguori, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Alon Levy, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Anthony Liguori, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Alon Levy, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Alon Levy, 2013/03/21
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Hans de Goede, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Anthony Liguori, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Gerd Hoffmann, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Hans de Goede, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected,
Anthony Liguori <=
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Hans de Goede, 2013/03/24
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Gerd Hoffmann, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Hans de Goede, 2013/03/22
- Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected, Anthony Liguori, 2013/03/22
- [Qemu-devel] [PATCH 2/2] spice-qemu-char: register interface on post load, Alon Levy, 2013/03/21
- Re: [Qemu-devel] [PATCH 2/2] spice-qemu-char: register interface on post load, Hans de Goede, 2013/03/22
- Re: [Qemu-devel] [PATCH 2/2] spice-qemu-char: register interface on post load, Alon Levy, 2013/03/22
- Re: [Qemu-devel] [PATCH 2/2] spice-qemu-char: register interface on post load, Hans de Goede, 2013/03/22
[Qemu-devel] [PATCH 3/4] virtio-console: implement post_load to call to qemu_chr_fe_post_load, Alon Levy, 2013/03/20
[Qemu-devel] [PATCH 4/4] spice-qemu-char: register interface on post load, Alon Levy, 2013/03/20