On Thu, Sep 25, 2025 at 12:54:13PM -0700, Richard Henderson wrote:
> On 9/25/25 02:44, Daniel P. Berrangé wrote:
> > + if (monitor_cur_is_hmp()) {
> > + cur = monitor_cur();
> > + }
>
> Didn't your last patch set return Montor* from monitor_cur_is_hmp?
> Because this takes the locks in monitor_cur() twice.
Yes, however, the intent of both this & the previous version is to
optimize the scenario where the monitor is NULL or QMP. If we take
an extra lock in the HMP case that's acceptable and doesn't cause
ill effets.
>
> > + if (message_with_timestamp && !cur) {
> > timestr = real_time_iso8601();
> > - error_printf("%s ", timestr);
> > + error_printf_mon(NULL, "%s ", timestr);
>
> Passing NULL to error_printf_mon is fprintf to stderr.
True, I could have rewritten this to just fprintf. It goes away
later in the series when its all moved into the new qcontext API
call, which is given stderr directly. I guess making it clear it
is stderr at this point in the series, gives reassurance that the
later qcontext conversion hasn't changed the output target.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
