On Wed, Oct 19, 2016 at 02:16:05PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berra...@redhat.com> 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. > > Beyond the short term we have no choice but > > to plumb in error_setg throughout, otherwise QMP will continue to > > have useless error reporting in this area of code. > > Actually, no choice but propagate errors up the stack until they reach a > spot that knows how to report them. error_setg() is *one* way to do > that. Its prime advantage is ability to carry an error message. Its > disadvantage is boilerplate. Use it as needed. Just don't convert back > and forth between Error and other representations while propagating up > the stack. 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/ :|
