[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2)
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2) |
Date: |
Thu, 13 Oct 2016 13:36:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 13/10/2016 13:14, Marc-André Lureau wrote:
> Hi,
>
> Commit 949055a2 "char: use a fixed idx for child muxed chr" introduced
> a regression in mux usage, since it wrongly interpreted mux as muxing
> various chr backend. Instead, it muxes frontends.
>
> The first patch reverts the broken change, the following patches add
> tracking to frontend handler, finally the last patch adds some tests
> that would have helped to track the crash and the regression. There is
> also a small fix for ringbuf.
In general I like the solution, but I dislike the API.
Would it work if we had something like
struct CharBackend {
CharDriverState *chr;
int tag;
}
and we modified all qemu_chr_fe_* functions (plus
qemu_chr_add_handlers[1]) to take a struct CharBackend. chardev
properties would also take a struct CharBackend. Then removing handlers
can still be done in release_chr, making the patches much smaller.
The conversion is a bit tedious, but I think it's much easier compared
to patch 4. I feel bad for having you redo everything and in particular
patch 7, but this is the model that the block layer uses and it works
very well there.
[1] while at it, it's probably best to rename qemu_chr_add_handlers
to qemu_chr_fe_add_handlers as the first patch in the series,
so that qemu_chr_add_handlers(CharDriverState *chr, ..., int tag)
can take the role of qemu_chr_set_handlers in this series.
Thanks,
Paolo
- [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2), Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 1/9] Revert "char: use a fixed idx for child muxed chr", Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 2/9] char: return a tag when adding the fe handlers, Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 3/9] char: add qemu_chr_remove_handlers(), Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 5/9] char: warn on unused qemu_chr_add_handlers() result, Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 4/9] char: keep track of qemu_chr_add_handlers(), Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 6/9] qdev: remove call to qemu_chr_add_handlers(), Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 8/9] ringbuf: fix chr_write return value, Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 7/9] char: handle qemu_chr_add_handlers() error, Marc-André Lureau, 2016/10/13
- [Qemu-devel] [PATCH 9/9] tests: start chardev unit tests, Marc-André Lureau, 2016/10/13
- Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2),
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2), Peter Maydell, 2016/10/13