From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA Date: Tue, 4 Aug 2015 11:06:50 +0100 Message-ID: <1438682810.31129.86.camel@citrix.com> References: <1438617395-20329-1-git-send-email-andrew.cooper3@citrix.com> <1438617395-20329-3-git-send-email-andrew.cooper3@citrix.com> <1438681238.31129.68.camel@citrix.com> <55C08923.2000009@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55C08923.2000009@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Xen-devel Cc: Wei Liu , Ian Jackson List-Id: xen-devel@lists.xenproject.org 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 > > > --- > > > CC: Ian Campbell > > > CC: Ian Jackson > > > CC: Wei Liu > > > > > > 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.