Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2)

[Paolo Bonzini]

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