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

[Peter Krempa]

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.

> 
> 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.

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.

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