Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* John Snow (jsnow@redhat.com) wrote:
> 
> 
> On 1/27/20 3:43 PM, Peter Krempa wrote:
> > On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote:
> >>
> >>
> >> On 1/27/20 5:36 AM, Maxim Levitsky wrote:
> >>> This patch series is bunch of cleanups
> >>> to the hmp monitor code.
> >>>
> >>> This series only touched blockdev related hmp handlers.
> >>>
> >>> No functional changes expected other that
> >>> light error message changes by the last patch.
> >>>
> >>> This was inspired by this bugzilla:
> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1719169
> >>>
> >>> Basically some users still parse hmp error messages,
> >>> and they would like to have them prefixed with 'Error:'
> >>>
> >>
> >> HMP isn't meant to be parsed. It's explicitly *not* API or ABI. I do
> >> like consistency in my UIs (it's useful for human eyes, too), but I'd
> >> like to know more about the request.
> > 
> > That's true as long as there's an stable replacement ... see below.
> > 
> 
> Thanks for the context!
> 
> >>
> >> Is this request coming from libvirt? Can we wean them off of this
> >> interface? What do they need as a replacement?
> > 
> > There are 5 commands that libvirt still has HMP interfaces for:
> > 
> > drive_add
> > drive_del
> > 
> > savevm
> > loadvm
> > delvm
> > 
> > From upstream point of view there's no value in adding the 'error'
> > prefix to drive_add/drive_del as libvirt now uses blockdev-add/del QMP
> > command instead which have implicit error propagation.
> > 
> 
> As thought.
> 
> > There are no replacements for the internal snapshot commands, but they
> > reported the 'error' prefix for some time even before this series.
> > 
> > Said that, please don't break savevm/loadvm/delvm until a QMP
> > replacement is added.
> > 
> 
> Yes, noted. I wonder where userfaultfd write support is these days...

How would that help you there?

Dave

> > The bug was reported at the time when libvirt didn't use blockdev yet,
> > but at this point it's pointless from our side. This wouldn't even fix
> > the scenario when old (pre-5.10) libvirt would use new qemu because the
> > drive-add handler never checked the error prefix.
> > 
> > [1] https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_monitor_text.c;h=9135a11f0a3aae718c86bb199112fba8d16d4d80;hb=HEAD
> > 
> 
> Thank you for the report from libvirtville :)
> 
> --js
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK