From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32988) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwUoX-0008Rb-Hp for qemu-devel@nongnu.org; Tue, 18 Oct 2016 09:52:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwUoU-0004p2-BK for qemu-devel@nongnu.org; Tue, 18 Oct 2016 09:52:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52234) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bwUoU-0004oY-3E for qemu-devel@nongnu.org; Tue, 18 Oct 2016 09:52:18 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 30F40C05680F for ; Tue, 18 Oct 2016 13:52:17 +0000 (UTC) Date: Tue, 18 Oct 2016 14:52:13 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161018135213.GI2190@work-vm> References: <20161012191502.GC16187@work-vm> <20161018100409.GH4349@redhat.com> <20161018113202.GE2190@work-vm> <20161018120121.GN4349@redhat.com> <20161018132524.GG2190@work-vm> <20161018133528.GD12728@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20161018133528.GD12728@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] chardev's and fd's in monitors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, armbru@redhat.com * Daniel P. Berrange (berrange@redhat.com) wrote: > On Tue, Oct 18, 2016 at 02:25:25PM +0100, Dr. David Alan Gilbert wrote: > > * Daniel P. Berrange (berrange@redhat.com) wrote: > > > On Tue, Oct 18, 2016 at 12:32:02PM +0100, Dr. David Alan Gilbert wr= ote: > > > > * Daniel P. Berrange (berrange@redhat.com) wrote: > > > > > On Wed, Oct 12, 2016 at 08:15:02PM +0100, Dr. David Alan Gilber= t wrote: > > > > > > Hi, > > > > > > I had a look at a couple of readline like libraries; > > > > > > editline and linenoise. A difficulty with using them is that > > > > > > they both want fd's or FILE*'s; editline takes either but > > > > > > from a brief look I think it's expecting to extract the fd. > > > > > > That makes them tricky to integrate into qemu, where > > > > > > the chardev's hide a whole bunch of non-fd things; in particu= lar > > > > > > tls, mux, ringbuffers etc. > > > > > >=20 > > > > > > If we could get away with just a FILE* then we could use fope= ncookie, > > > > > > but that's GNU only. > > > > > >=20 > > > > > > Is there any sane way of shepherding all chardev's into havin= g an > > > > > > fd? > > > > >=20 > > > > > The entire chardev abstraction model exists precisely because w= e cannot > > > > > make all chardevs look like a single fd. Even those which are f= d based > > > > > may have separate FDs for input and output. > > > >=20 > > > > Note that editline takes separate in/out streams, but it does wan= t those streams > > > > to be FILE*'s. > > > >=20 > > > > > IMHO the only viable approach would be to enhance linenoise/edi= tline to > > > > > not assume use of fd* or FILE * abstractions. > > > >=20 > > > > I think if it came to that then we'd probably end up sticking wit= h what we > > > > had for a very long time; I'd assume it would take a long time be= fore > > > > any mods we made to the libraries would come around to be general= ly useful. > > > >=20 > > > > > BTW, what is the actual thread issue you are facing ? Chardevs = at least > > > > > ought to be usable from a separate thread, as long as each dist= inct > > > > > chardev object instance was only used from one thread at a time= ? > > > >=20 > > > > Marc-Andr=E9 pointed that out; I hadn't realised they were thread= safe. > > > > But what are the rules? You say 'only used from one thread at a t= ime' - > > > > what happens if we have a mux and the different streams to the mu= x come > > > > from different threads? > > >=20 > > > Well there is no mutex locking on the CharDriverState objects, so t= he > > > exact rule is "you mustn't do anything from multiple threads that w= ill > > > race on contents of CharDriverState". That's too fuzzy to be useful= to > > > developers though, so I think the only sensible option right now is= to > > > say any "top level" CharDriverState should only be touch from one t= hread > > > at a time. IOW, if you have a mux, that that rule would apply to th= e > > > mux itself and the various children it owns as if they were a singl= e > > > unnit. > >=20 > > OK; I think we're probably saved by the big lock at the moment, so th= at > > all device emulation that outputs text is probably holding it and the= monitor > > is also. What about something like an error_report from a different = thread > > while something is happening in the monitor? >=20 > If we moved execution of monitor commands to separate thread from the > thread handling monitor I/O, then we'd have to modify error_report so > that it queued the text in some manner, such that it was only then > fed back to the client once the command thread completed. Alternatively > we'd have to introduced locking in the Monitor object, that serialized > access to the underling CharDriverState I/O funcs. I already use error_report's in places in migration threads of various types; I'm not sure if that's a problem. > > > > My actual thoughts for threads came from a few sides: > > > > a) Maybe I could have a shim thread that fed the editline fd fr= om a chardev > > > > b) I'd eventually like multiple monitor threads. > > >=20 > > > Can you expand on what you mean by multiple monitor threads ? Presu= mably > > > you're meaning a single monitor instance, with multiple threads pro= cessing > > > commands concurrently ? If so, I think that ought to be fine even = with > > > the current thread rules around chardevs. The processing of individ= ual > > > monitor commands doesn't interact with the CharDriverState AFAIR, a= s we > > > have clean separation between parsing the incoming command, running= the > > > command, and formatting the outgoing response. IOW, for a single mo= nitor > > > it is still sufficient to have a single thread deal with all I/O fo= r the > > > chardev - only the command execution needs to be delegated to other > > > threads, and those wouldn't be touching the chardev at all. > >=20 > > Hmm, I'd thought of the other way around - multiple individual monito= rs each > > running one command; ie each connection for a monitor would be it's o= wn > > thread. >=20 > So I guess there's two problems with the monitor handling right now wrt= . >=20 > - A long running command will block the event loop thread for too long > - A long running command prevents a client issuing other commands whil= e > waiting for the previous command to complete. >=20 > Running a thread per monitor server solves the first problem. If we mak= e > monitor command handling async though, then it solves both problems. There are some other error cases that cause the main thread to be blocked and my main interest in having multiple monitor threads is being able to = dig ourselves out of those error cases. The cases I've got are: a) COLO or migration, where the networking/storage dies during the fi= nal sync of a migration with the big lock held. b) Postcopy where the network dies and a device emulation tries to ac= cess memory where the memory isn't on the host yet; the device emulation has the lock and we can't issue a command to trigger a recovery. Neither of those are necessarily running an existing monitor command. Dave > Regards, > Daniel > --=20 > |: http://berrange.com -o- http://www.flickr.com/photos/dberran= ge/ :| > |: http://libvirt.org -o- http://virt-manager.= org :| > |: http://entangle-photo.org -o- http://search.cpan.org/~danbe= rr/ :| -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK