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/28/20 11:47 AM, Dr. David Alan Gilbert wrote:
> > * 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?
> > 
> 
> Left at the traffic lights, but there was a thought that we'd be able to
> get transactionable save-memory support in QMP if we could use
> userfaultfd to do just-in-time copies of memory as needed, until the job
> is complete.
> 
> This way we could support it properly in QMP and we'd have a replacement
> for the HMP version which -- from memory -- is not appropriate for the
> QMP channel.
> 
> Maybe I imagined this restriction.

The restriction is there; but it's not related to the memory saving;
savevm mostly uses the core migration code (which would normally run in
a separate thread) but uses it itself in it's own loop in the main
thread with a bunch of bdrv code glued around it to do the snapshots
there. 
It shouldn't be that hard to convert it to be much more like a normal
migration - except it needs some hook then to do the snapshotting stuff
at the end.

Dave

> --js
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK