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