Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc

[Michael S. Tsirkin]

On Wed, Jun 17, 2015 at 04:13:52PM +0200, Paolo Bonzini wrote:
> 
> 
> On 17/06/2015 16:11, Michael S. Tsirkin wrote:
> > On Fri, Jun 12, 2015 at 10:05:47AM -0400, Don Slutz wrote:
> >> Changes v6 to v7:
> >>   Rebase to master
> >>
> >>   Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 
> >> now patch #1
> > 
> >>   Added patch #2 to switch to using trace in vm,port.c.
> >>
> >>   Delay call on g_strndup till after key length check.
> >>
> >>   Switched e-mail address in MAINTAINERS.
> >>
> >>   Eric Blake
> >>     Why not assert(find) instead of leaving it to the comment?
> >>       Switch to assert.
> >>     Is it worth marking arg const here and in the VMPortRpcFind struct
> >>       Switch to const.
> >>     I'd rather abort() if someone compiled with -NDEBUG
> >>       Done.
> >>     Still mismatches on ---- line length (several sites).
> >>       Done
> >>
> >>
> >> Changes v5 to v6:
> >>
> >>   Rebase to master
> >>
> >>   Eric Blake
> >>     Returning a non-dictionary is not extensible.
> >>       Added new type VmportGuestInfoValue.
> >>       s/VmportGuestInfo/VmportGuestInfoKey/
> >>       s/type/struct/
> >>     Issues with examples
> >>       Fixed.
> >>
> >>
> >> Changes v4 to v5:
> >>
> >>   Paolo Bonzini
> >>     What is VMPORT_SHORT about?
> >>       Dropped this.
> >>     Why not use a bool in CPUX86State?
> >>       Took his sugestion and moved to a bool in X86CPU.
> >>
> >> Changes v3 to v4:
> >>
> >>   Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
> >>     Does this compile on non-x86 targets?
> >>       Nope.  Fixed.
> >>
> >> Changes v2 to v3:
> >>
> >>   s/2.3/2.4
> >>
> >> Changes v1 to v2:
> >>
> >>    Added live migration code.
> >>    Adjust data structures for migration.
> >>    Switch to GHashTable.
> >>
> >>   Eric Blake
> >>     s/spawened/spawned/
> >>       Done
> >>     s/traceing/tracing/
> >>       Done
> >>     Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
> >>     "error_setg(errp, "
> >>       Done
> >>     Why two commands (inject-vmport-reboot, inject-vmport-halt)?
> >>       Switched to inject-vmport-action.
> >>     format=base64 "bug" statements.
> >>       Dropped.
> >>
> >> Much more on format=base64:
> >>
> >> If there is a bug it is in GLIB.  However the Glib reference manual
> >> refers to RFC 1421 and RFC 2045 and MIME encoding.  Based on all
> >> that (which seems to match:
> >>
> >> http://en.wikipedia.org/wiki/Base64
> >>
> >> ) MIME states that all characters outside the (base64) alphabet are
> >> to be ignored.  Testing shows that g_base64_decode() does this.
> >>
> >> The confusion is that most non-MIME uses reject a base64 string that
> >> contain characters outside the alphabet.  I was just following the
> >> other uses of base64 in this file.
> >>
> >> DataFormat refers to RFC 3548, which has the info:
> >>
> >> "
> >>    Implementations MUST reject the encoding if it contains
> >>    characters outside the base alphabet when interpreting base
> >>    encoded data, unless the specification referring to this document
> >>    explicitly states otherwise.  Such specifications may, as MIME
> >>    does, instead state that characters outside the base encoding
> >>    alphabet should simply be ignored when interpreting data ("be
> >>    liberal in what you accept").
> >> "
> >>
> >> So with GLIB going the MIME way, I do not think this is a QEMU bug
> >> (you could consider this a GLIB bug, but the document I found says
> >> that GLIB goes the MIME way and so does not reject anything).
> > 
> > To me it looks like this will break cross-version migration as you are
> > adding a bunch of devices unconditionally.  Will it not?
> 
> Yes, that's what was done for parallel and pcspk as well.  There's no
> infrastructure to avoid it.
> 
> Paolo

How do you mean? We have multiple ways to keep devices
compatible with old versions.
Set a new property to skip the extra stuff.

Since we enable this thing by default (why do we?)
this seems like a big deal ...

-- 
MST