From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxGhJ-0000lX-9x for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:00:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxGhF-00085h-J0 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:00:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49716) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bxGhF-00085S-Cn for qemu-devel@nongnu.org; Thu, 20 Oct 2016 13:00:01 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 8ECBAC04B92C for ; Thu, 20 Oct 2016 17:00:00 +0000 (UTC) Date: Thu, 20 Oct 2016 17:59:57 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161020165956.GM2039@work-vm> References: <87wph4g44n.fsf@dusky.pond.sub.org> <20161019081210.GA2035@work-vm> <20161019084235.GE11194@redhat.com> <87twc8d60e.fsf@dusky.pond.sub.org> <20161019100552.GD2035@work-vm> <20161019101616.GL11194@redhat.com> <87a8e0bkl6.fsf@dusky.pond.sub.org> <20161019122158.GS11194@redhat.com> <20161019180616.GF2035@work-vm> <20161020083730.GC12145@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161020083730.GC12145@redhat.com> 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: Markus Armbruster , qemu-devel@nongnu.org * Daniel P. Berrange (berrange@redhat.com) wrote: > On Wed, Oct 19, 2016 at 07:06:16PM +0100, Dr. David Alan Gilbert wrote: > > * Daniel P. Berrange (berrange@redhat.com) wrote: > > > On Wed, Oct 19, 2016 at 02:16:05PM +0200, Markus Armbruster wrote: > > > > "Daniel P. Berrange" writes: > > > > > > > > > On Wed, Oct 19, 2016 at 11:05:53AM +0100, Dr. David Alan Gilbert wrote: > > > > >> > > > > >> We need a way to be able to report an error without plumbing error_setg > > > > >> up the stack; if you're saying error_report isn't suitable then we > > > > >> should just recommend we switch everything in migration back to > > > > >> fprintf(stderr, > > > > > > > > In the cases where error_report() isn't suitable, fprintf() is just as > > > > unsuitable for the exact same reasons. > > > > > > > > > Well both error_report() + fprintf are broken from POV of anything > > > > > using QMP. error_report() is slightly less broken for HMP, > > > > > > > > error_report() is not broken at all for HMP code. The trouble is code > > > > that can't know whether it's running in a context where error_report() > > > > is suitable. > > > > > > > > > but doesn't > > > > > help QMP. > > > > > > > > Correct. > > > > > > > > > In the short term we should just make error_report be threadsafe in > > > > > its usage of the monitor. > > > > > > > > Any problems left once cur_mon is thread-local (which it should be > > > > anyway)? > > > > > > If we make cur_mon a thread-local, then error_report() is equivalent > > > to fprintf(stderr) for the migration code, since the migration > > > code runs in a different thread thread, and so would always see > > > cur_mon == NULL. > > > > Yes, that would become safe; it does sound the best fix for the current > > worry. > > > > If we had that, then why not wire up error_report to pass errors back to QMP > > as well? > > You have a problem of context - if you have multiple monitors, how do > you know which to send the error back to if you're not in the event > loop thread, and thus cur_mon is NULL. With Marc-Andre's series which > allows proper async command processing it gets even harder, because > there's potentially many outstanding commands associated with a monitor > and you need to decide which the error should be given to. Are those async commands operating in a particular coroutine? i.e. should it be a coroutine-local pointer and if it's not in that then a thread-local? Dave > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK