[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.
+
+     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.
+--------------------------------------------------------------------
+
+\clearpage
+
+Records
+=======
+
 END
 ----
 
@@ -163,17 +196,33 @@ The libxc context record contains no fields; its 
body_length is 0[^1].
 might write into the stream, especially for live migration where the quantity
 of data is partially proportional to the elapsed time.
 
-XENSTORE\_DATA
--------------
+EMULATOR\_XENSTORE\_DATA
+------------------------
 
-A record containing xenstore key/value pairs of data.
+A set of xenstore key/value pairs for a specific emulator associated with the
+domain.
 
      0     1     2     3     4     5     6     7 octet
-    +-------------------------------------------------+
-    | xenstore key/value pairs                        |
+    +------------------------+------------------------+
+    | emulator_id            | index                  |
+    +------------------------+------------------------+
+    | xenstore key/value data                         |
     ...
     +-------------------------------------------------+
 
+Xenstore key/value data are encoded as a packed sequence of (key, value)
+tuples.  Each (key, value) tuple is a packed pair of NUL terminated octets,
+conforming to xenstore protocol character encoding (keys strictly as
+alphanumeric ASCII and `-/_@`, values expected to be human-readable ASCII).
+
+Keys shall be relative to to the device models xenstore tree for the new
+domain.  At the time of writing, keys are relative to the path
+
+> `/local/domain/$dm_domid/device-model/$domid/`
+
+although this path is free to change moving forward, thus should not be
+assumed.
+
 EMULATOR\_CONTEXT
 ----------------
 
@@ -187,22 +236,8 @@ A context blob for a specific emulator associated with the 
domain.
     ...
     +-------------------------------------------------+
 
---------------------------------------------------------------------
-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.
-
-emulator_ctx     Emulator context blob.
---------------------------------------------------------------------
+The *emulator_ctx* is a binary blob interpreted by the emulator identified by
+*emulator_id*.  Its format is unspecified.
 
 CHECKPOINT\_END
 ---------------
diff --git a/tools/libxl/libxl_sr_stream_format.h 
b/tools/libxl/libxl_sr_stream_format.h
index 3f3c497..4c23367 100644
--- a/tools/libxl/libxl_sr_stream_format.h
+++ b/tools/libxl/libxl_sr_stream_format.h
@@ -31,11 +31,12 @@
 /* All records must be aligned up to an 8 octet boundary */
 #define REC_ALIGN_ORDER              3U
 
-#define REC_TYPE_END                 0x00000000U
-#define REC_TYPE_LIBXC_CONTEXT       0x00000001U
-#define REC_TYPE_XENSTORE_DATA       0x00000002U
-#define REC_TYPE_EMULATOR_CONTEXT    0x00000003U
-#define REC_TYPE_CHECKPOINT_END      0x00000004U
+#define REC_TYPE_END                    0x00000000U
+#define REC_TYPE_LIBXC_CONTEXT          0x00000001U
+#define REC_TYPE_XENSTORE_DATA          0x00000002U /* TOOLSTACK COMPAT */
+#define REC_TYPE_EMULATOR_XENSTORE_DATA 0x00000002U
+#define REC_TYPE_EMULATOR_CONTEXT       0x00000003U
+#define REC_TYPE_CHECKPOINT_END         0x00000004U
 
 typedef struct libxl__sr_emulator_hdr
 {
diff --git a/tools/python/scripts/convert-legacy-stream 
b/tools/python/scripts/convert-legacy-stream
index d54fa22..16331a4 100755
--- a/tools/python/scripts/convert-legacy-stream
+++ b/tools/python/scripts/convert-legacy-stream
@@ -174,7 +174,7 @@ def write_libxl_xenstore_data(data):
 
 def write_libxl_emulator_context(blob):
     write_record(libxl.REC_TYPE_emulator_context,
-                 pack(libxl.EMULATOR_CONTEXT_FORMAT,
+                 pack(libxl.EMULATOR_HEADER_FORMAT,
                       libxl.EMULATOR_ID_unknown, 0) + blob)
 
 def rdexact(nr_bytes):
diff --git a/tools/python/xen/migration/legacy.py 
b/tools/python/xen/migration/legacy.py
index 2f2240a..6456d61 100644
--- a/tools/python/xen/migration/legacy.py
+++ b/tools/python/xen/migration/legacy.py
@@ -2,12 +2,15 @@
 # -*- coding: utf-8 -*-
 
 """
-Libxc legacy migration streams
+Legacy migration stream information.
 
-Documentation and record structures for legacy migration
+Documentation and record structures for legacy migration, for both libxc
+and libxl.
 """
 
 """
+Libxc:
+
 SAVE/RESTORE/MIGRATE PROTOCOL
 =============================
 
@@ -277,3 +280,36 @@ MAX_BATCH = 1024
 
 # Maximum #VCPUs currently supported for save/restore
 MAX_VCPU_ID = 4095
+
+
+"""
+Libxl:
+
+Legacy "toolstack" record layout:
+
+Version 1:
+  uint32_t version
+  QEMU physmap data:
+    uint32_t count
+    libxl__physmap_info * count
+
+The problem is that libxl__physmap_info was declared as:
+
+struct libxl__physmap_info {
+    uint64_t phys_offset;
+    uint64_t start_addr;
+    uint64_t size;
+    uint32_t namelen;
+    char name[];
+};
+
+Which has 4 bytes of padding at the end in a 64bit build, thus not the
+same between 32 and 64bit builds.
+
+Because of the pointer arithmatic used to construct the record, the 'name' was
+shifted up to start at the padding, leaving the erronious 4 bytes at the end
+of the name string, after the NUL terminator.
+
+Instead, the information described here has been changed to fit in a new
+EMULATOR_XENSTORE_DATA record made of NUL terminated strings.
+"""
diff --git a/tools/python/xen/migration/libxl.py 
b/tools/python/xen/migration/libxl.py
index 415502e..1a9ca87 100644
--- a/tools/python/xen/migration/libxl.py
+++ b/tools/python/xen/migration/libxl.py
@@ -10,7 +10,7 @@ verification routines.
 
 import sys
 
-from struct import calcsize, unpack
+from struct import calcsize, unpack, unpack_from
 from xen.migration.verify import StreamError, RecordError, VerifyBase
 from xen.migration.libxc import VerifyLibxc
 
@@ -32,22 +32,23 @@ HDR_OPT_RESZ_MASK = 0xfffc
 # Records
 RH_FORMAT = "II"
 
-REC_TYPE_end              = 0x00000000
-REC_TYPE_libxc_context    = 0x00000001
-REC_TYPE_xenstore_data    = 0x00000002
-REC_TYPE_emulator_context = 0x00000003
-REC_TYPE_checkpoint_end   = 0x00000004
+REC_TYPE_end                    = 0x00000000
+REC_TYPE_libxc_context          = 0x00000001
+REC_TYPE_xenstore_data          = 0x00000002 # TOOLSTACK COMPAT
+REC_TYPE_emulator_xenstore_data = 0x00000002
+REC_TYPE_emulator_context       = 0x00000003
+REC_TYPE_checkpoint_end         = 0x00000004
 
 rec_type_to_str = {
-    REC_TYPE_end              : "End",
-    REC_TYPE_libxc_context    : "Libxc context",
-    REC_TYPE_xenstore_data    : "Xenstore data",
-    REC_TYPE_emulator_context : "Emulator context",
-    REC_TYPE_checkpoint_end   : "Checkpoint end",
+    REC_TYPE_end                    : "End",
+    REC_TYPE_libxc_context          : "Libxc context",
+    REC_TYPE_emulator_xenstore_data : "Emulator xenstore data",
+    REC_TYPE_emulator_context       : "Emulator context",
+    REC_TYPE_checkpoint_end         : "Checkpoint end",
 }
 
-# emulator_context
-EMULATOR_CONTEXT_FORMAT = "II"
+# emulator_* header
+EMULATOR_HEADER_FORMAT = "II"
 
 EMULATOR_ID_unknown       = 0x00000000
 EMULATOR_ID_qemu_trad     = 0x00000001
@@ -155,22 +156,50 @@ class VerifyLibxl(VerifyBase):
         VerifyLibxc(self.info, self.read).verify()
 
 
-    def verify_record_xenstore_data(self, content):
-        """ Xenstore Data record """
+    def verify_record_emulator_xenstore_data(self, content):
+        """ Emulator Xenstore Data record """
+        minsz = calcsize(EMULATOR_HEADER_FORMAT)
 
-        if len(content) == 0:
-            raise RecordError("Xenstore data record with zero length")
+        if len(content) < minsz:
+            raise RecordError("Length must be at least %d bytes, got %d"
+                              % (minsz, len(content)))
+
+        emu_id, emu_idx = unpack(EMULATOR_HEADER_FORMAT, content[:minsz])
+
+        if emu_id not in emulator_id_to_str:
+            raise RecordError("Unrecognised emulator id 0x%x" % (emu_id, ))
+
+        self.info("Emulator Xenstore Data (%s, idx %d)"
+                  % (emulator_id_to_str[emu_id], emu_idx))
+
+        # Chop off the emulator header
+        content = content[minsz:]
+
+        if len(content):
+
+            if content[-1] != '\x00':
+                raise RecordError("Data not NUL terminated")
+
+            # Split without the final NUL, to get an even number of parts
+            parts = content[:-1].split("\x00")
+
+            if (len(parts) % 2) != 0:
+                raise RecordError("Expected an even number of strings, got %d"
+                                  % (len(parts), ))
+
+            for key, val in zip(parts[0::2], parts[1::2]):
+                self.info("  '%s' = '%s'" % (key, val))
 
 
     def verify_record_emulator_context(self, content):
         """ Emulator Context record """
-        minsz = calcsize(EMULATOR_CONTEXT_FORMAT)
+        minsz = calcsize(EMULATOR_HEADER_FORMAT)
 
         if len(content) < minsz:
             raise RecordError("Length must be at least %d bytes, got %d"
                               % (minsz, len(content)))
 
-        emu_id, emu_idx = unpack(EMULATOR_CONTEXT_FORMAT, content[:minsz])
+        emu_id, emu_idx = unpack(EMULATOR_HEADER_FORMAT, content[:minsz])
 
         if emu_id not in emulator_id_to_str:
             raise RecordError("Unrecognised emulator id 0x%x" % (emu_id, ))
@@ -190,8 +219,8 @@ record_verifiers = {
         VerifyLibxl.verify_record_end,
     REC_TYPE_libxc_context:
         VerifyLibxl.verify_record_libxc_context,
-    REC_TYPE_xenstore_data:
-        VerifyLibxl.verify_record_xenstore_data,
+    REC_TYPE_emulator_xenstore_data:
+        VerifyLibxl.verify_record_emulator_xenstore_data,
     REC_TYPE_emulator_context:
         VerifyLibxl.verify_record_emulator_context,
     REC_TYPE_checkpoint_end:
diff --git a/tools/python/xen/migration/tests.py 
b/tools/python/xen/migration/tests.py
index 91044cd..026cf38 100644
--- a/tools/python/xen/migration/tests.py
+++ b/tools/python/xen/migration/tests.py
@@ -37,7 +37,7 @@ class TestLibxl(unittest.TestCase):
         for fmt, sz in ( (libxl.HDR_FORMAT, 16),
                          (libxl.RH_FORMAT, 8),
 
-                         (libxl.EMULATOR_CONTEXT_FORMAT, 8),
+                         (libxl.EMULATOR_HEADER_FORMAT, 8),
                          ):
             self.assertEqual(calcsize(fmt), sz)
 
-- 
1.7.10.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.