Re: [Xen-devel] [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-08-04 at 10:42 +0100, Andrew Cooper wrote:
> On 04/08/15 10:40, Ian Campbell wrote:
> > On Mon, 2015-08-03 at 16:56 +0100, Andrew Cooper wrote:
> > > The legacy "toolstack" record as implemented in libxl turns out not 
> > > to
> > > be 32/64bit safe.  As migration v2 has not shipped yet, take this
> > > opportunity to adjust the specification and fix the incompatibility.
> > > 
> > > Libxl shall loose all knowledge of the old "toolstack" blob and use 
> > > this
> > > EMULATOR_XENSTORE_DATA record instead.  Compatibility shall be 
> > > handled
> > > by the legacy conversion script.
> > > 
> > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > ---
> > > CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> > > CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> > > CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > 
> > > v2:
> > >  * More adjustments to the libxl spec.
> > >  * Make the emulator id/index table common and move it up beside the
> > >    record type/length table.
> > >  * Be more specific about the format and encoding of the xenstore
> > >    key/value data.
> > >  * Add a note about the unspecified nature of the emulator context 
> > > blob.
> > > ---
> > >  docs/specs/libxl-migration-stream.pandoc   |   83 
> > > ++++++++++++++++++++--
> > > ------
> > >  tools/libxl/libxl_sr_stream_format.h       |   11 ++--
> > >  tools/python/scripts/convert-legacy-stream |    2 +-
> > >  tools/python/xen/migration/legacy.py       |   40 +++++++++++++-
> > >  tools/python/xen/migration/libxl.py        |   71 +++++++++++++++++-
> > > ----
> > > --
> > >  tools/python/xen/migration/tests.py        |    2 +-
> > >  6 files changed, 155 insertions(+), 54 deletions(-)
> > > 
> > > diff --git a/docs/specs/libxl-migration-stream.pandoc 
> > > b/docs/specs/libxl
> > > -migration-stream.pandoc
> > > index cdec168..85adbf0 100644
> > > --- a/docs/specs/libxl-migration-stream.pandoc
> > > +++ b/docs/specs/libxl-migration-stream.pandoc
> > > @@ -90,8 +90,8 @@ i386, x86_64, or arm host.
> > >  \clearpage
> > >  
> > >  
> > > -Records
> > > -=======
> > > +Record Overview
> > > +===============
> > >  
> > >  A record has a record header, type specific data and a trailing 
> > > footer. 
> > >  If
> > >  `length` is not a multiple of 8, the body is padded with zeroes to 
> > > align 
> > > the
> > > @@ -113,7 +113,7 @@ type         0x00000000: END
> > >  
> > >               0x00000001: LIBXC_CONTEXT
> > >  
> > > -             0x00000002: XENSTORE_DATA
> > > +             0x00000002: EMULATOR_XENSTORE_DATA
> > >  
> > >               0x00000003: EMULATOR_CONTEXT
> > >  
> > > @@ -135,6 +135,39 @@ padding      0 to 7 octets of zeros to pad the 
> > > whole 
> > > record to a multiple
> > >  
> > >  \clearpage
> > >  
> > > +Emulator Records
> > > +----------------
> > > +
> > > +Several records are specifically for emulators, and have a common 
> > > sub 
> > > header.
> > It would be useful to mention in the specific records which have this
> > header that they do, I think.
> 
> Each record containing this sub header identify all the fields,
> including id and index.  I can list them, although they are currently
> all the EMULATOR_* records.

Ah, I saw the removal hunk for context data, but missed the out-of-patch
-context presence of the octet layout thing, so this is fine, thanks.

> > > +
> > > +     0     1     2     3     4     5     6     7 octet
> > > +    +------------------------+------------------------+
> > > +    | emulator_id            | index                  |
> > > +    +------------------------+------------------------+
> > > +    | record specific data                            |
> > > +    ...
> > > +    +-------------------------------------------------+
> > > +
> > > +--------------------------------------------------------------------
> > > +Field            Description
> > > +------------     ---------------------------------------------------
> > > +emulator_id      0x00000000: Unknown (In the case of a legacy 
> > > stream)
> > > +
> > > +                 0x00000001: Qemu Traditional
> > > +
> > > +                 0x00000002: Qemu Upstream
> > > +
> > > +                 0x00000003 - 0xFFFFFFFF: Reserved for future 
> > > emulators.
> > > +
> > > +index            Index of this emulator for the domain, if multiple
> > > +                 emulators are in use.
> > This is old wording, but what value does this take if no emulators are 
> > in
> > use? If the answer is "undefined" then I suppose there is a field 
> > somewhere
> > else which indicates the number of emulators?
> 
> If no emulators are in use, the record is not sent in the first place.

Sorry, I meant no multiple emulators. Which is a complicated way of saying
one...

I saw in a later patch that this field appears to be zero then, which is
reasonable and removes the need to treat multiple emulators specially. IOW
I think you could just remove the second clause above.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel