From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bueWj-0003B1-0n for qemu-devel@nongnu.org; Thu, 13 Oct 2016 07:50:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bueWe-0005Ng-2W for qemu-devel@nongnu.org; Thu, 13 Oct 2016 07:50:21 -0400 Received: from mx4-phx2.redhat.com ([209.132.183.25]:53641) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bueWd-0005Ma-PS for qemu-devel@nongnu.org; Thu, 13 Oct 2016 07:50:16 -0400 Date: Thu, 13 Oct 2016 07:50:10 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <73175669.690710.1476359410901.JavaMail.zimbra@redhat.com> In-Reply-To: <7b7a92e4-ea4a-0a64-5b35-4d2ede2849e1@redhat.com> References: <20161013111449.29387-1-marcandre.lureau@redhat.com> <7b7a92e4-ea4a-0a64-5b35-4d2ede2849e1@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/9] Fix mux regression (commit 949055a2) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , qemu-devel@nongnu.org, berrange@redhat.com, imbrenda@linux.vnet.ibm.com Hi ----- Original Message ----- >=20 >=20 > On 13/10/2016 13:14, Marc-Andr=C3=A9 Lureau wrote: > > Hi, > >=20 > > 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. > >=20 > > 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. >=20 > In general I like the solution, but I dislike the API. >=20 > Would it work if we had something like >=20 > struct CharBackend { > CharDriverState *chr; > int tag; > } >=20 You mean front-end right? > 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. As long as they use chardev property, it's not always the case. > The conversion is a bit tedious, but I think it's much easier compared Yes, it's tedious :) Do you mind if I try to make the change on top? If it = really reduces the patch 4/7, we could try to squash it? > 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. Which function btw? >=20 > [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.