* [PATCH for-4.6 v2 1/6] tools/libxl: Make libxl__conversion_helper_abort() safe to use
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
2015-08-04 9:30 ` Ian Campbell
2015-08-03 15:56 ` [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA Andrew Cooper
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Wei Liu
Previously, in the case of an error causing a call to
libxl__conversion_helper_abort() on a stream without legacy conversion,
libxl would fall over a NULL pointer because chs->ao was not set up.
Arrange for all ->ao's to be set up at _init() time, by having each
_init() function assert that their caller has done the right thing.
While doing so, introduce a previously-missing save_helper_init() in
stream_read_init().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
v2: Make sure ->ao is always valid
---
tools/libxl/libxl_convert_callout.c | 2 ++
tools/libxl/libxl_internal.h | 4 ++--
tools/libxl/libxl_stream_read.c | 11 ++++++++---
tools/libxl/libxl_stream_write.c | 5 +++++
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_convert_callout.c b/tools/libxl/libxl_convert_callout.c
index 65b4df9..5e5678b 100644
--- a/tools/libxl/libxl_convert_callout.c
+++ b/tools/libxl/libxl_convert_callout.c
@@ -34,6 +34,8 @@ static void helper_done(libxl__egc *egc,
void libxl__conversion_helper_init(libxl__conversion_helper_state *chs)
{
+ assert(chs->ao);
+
chs->v2_carefd = NULL;
chs->rc = 0;
libxl__ao_abortable_init(&chs->abrt);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 911de2d..22d6a26 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2948,9 +2948,9 @@ _hidden void libxl__remus_devices_commit(libxl__egc *egc,
typedef struct libxl__conversion_helper_state libxl__conversion_helper_state;
struct libxl__conversion_helper_state {
- /* public */
+ /* Public - Must be filled by caller unless noted. */
libxl__ao *ao;
- int legacy_fd;
+ int legacy_fd; /* fd to read the legacy stream from. */
bool hvm; /* pv or hvm domain? */
libxl__carefd *v2_carefd; /* Filled by successful call to
* libxl__convert_legacy_stream(). Caller
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index fd3675c..c555542 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -173,12 +173,18 @@ static void free_record(libxl__sr_record_buf *rec)
void libxl__stream_read_init(libxl__stream_read_state *stream)
{
+ assert(stream->ao);
+
+ stream->shs.ao = stream->ao;
+ libxl__save_helper_init(&stream->shs);
+
+ stream->chs.ao = stream->ao;
+ libxl__conversion_helper_init(&stream->chs);
+
stream->rc = 0;
stream->running = false;
stream->in_checkpoint = false;
stream->sync_teardown = false;
- libxl__save_helper_init(&stream->shs);
- libxl__conversion_helper_init(&stream->chs);
FILLZERO(stream->dc);
FILLZERO(stream->hdr);
LIBXL_STAILQ_INIT(&stream->record_queue);
@@ -205,7 +211,6 @@ void libxl__stream_read_start(libxl__egc *egc,
/* Convert the legacy stream. */
libxl__conversion_helper_state *chs = &stream->chs;
- chs->ao = stream->ao;
chs->legacy_fd = stream->fd;
chs->hvm =
(stream->dcs->guest_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM);
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 9e9c998..be8f548 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -155,6 +155,11 @@ static void write_done(libxl__egc *egc,
void libxl__stream_write_init(libxl__stream_write_state *stream)
{
+ assert(stream->ao);
+
+ stream->shs.ao = stream->ao;
+ libxl__save_helper_init(&stream->shs);
+
stream->rc = 0;
stream->running = false;
stream->in_checkpoint = false;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH for-4.6 v2 1/6] tools/libxl: Make libxl__conversion_helper_abort() safe to use
2015-08-03 15:56 ` [PATCH for-4.6 v2 1/6] tools/libxl: Make libxl__conversion_helper_abort() safe to use Andrew Cooper
@ 2015-08-04 9:30 ` Ian Campbell
0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2015-08-04 9:30 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson
On Mon, 2015-08-03 at 16:56 +0100, Andrew Cooper wrote:
> Previously, in the case of an error causing a call to
> libxl__conversion_helper_abort() on a stream without legacy conversion,
> libxl would fall over a NULL pointer because chs->ao was not set up.
>
> Arrange for all ->ao's to be set up at _init() time, by having each
> _init() function assert that their caller has done the right thing.
> While doing so, introduce a previously-missing save_helper_init() in
> stream_read_init().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> v2: Make sure ->ao is always valid
I believe this + the extra comment in libxl__conversion_helper_state
addresses Ian J's feedback from the previous round, so:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
2015-08-03 15:56 ` [PATCH for-4.6 v2 1/6] tools/libxl: Make libxl__conversion_helper_abort() safe to use Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
2015-08-04 9:40 ` Ian Campbell
2015-08-03 15:56 ` [PATCH for-4.6 v2 3/6] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content Andrew Cooper
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Wei Liu
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@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA
2015-08-03 15:56 ` [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA Andrew Cooper
@ 2015-08-04 9:40 ` Ian Campbell
2015-08-04 9:42 ` Andrew Cooper
0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2015-08-04 9:40 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson
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@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> 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.
> +
> + 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?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA
2015-08-04 9:40 ` Ian Campbell
@ 2015-08-04 9:42 ` Andrew Cooper
2015-08-04 10:06 ` Ian Campbell
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2015-08-04 9:42 UTC (permalink / raw)
To: Ian Campbell, Xen-devel; +Cc: Wei Liu, Ian Jackson
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@citrix.com>
>> ---
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>>
>> 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.
>
>> +
>> + 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.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA
2015-08-04 9:42 ` Andrew Cooper
@ 2015-08-04 10:06 ` Ian Campbell
2015-08-04 10:16 ` Andrew Cooper
0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2015-08-04 10:06 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson
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@citrix.com>
> > > ---
> > > CC: Ian Campbell <Ian.Campbell@citrix.com>
> > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > > CC: Wei Liu <wei.liu2@citrix.com>
> > >
> > > 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA
2015-08-04 10:06 ` Ian Campbell
@ 2015-08-04 10:16 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2015-08-04 10:16 UTC (permalink / raw)
To: Ian Campbell, Xen-devel; +Cc: Wei Liu, Ian Jackson
On 04/08/15 11:06, Ian Campbell wrote:
>
>>>> +
>>>> + 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.
Ah yes - I will make that adjustment.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-4.6 v2 3/6] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
2015-08-03 15:56 ` [PATCH for-4.6 v2 1/6] tools/libxl: Make libxl__conversion_helper_abort() safe to use Andrew Cooper
2015-08-03 15:56 ` [PATCH for-4.6 v2 2/6] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
2015-08-04 9:55 ` Ian Campbell
2015-08-03 15:56 ` [PATCH for-4.6 v2 4/6] tools/libxl: Prepare to write multiple records with EMULATOR headers Andrew Cooper
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Wei Liu
The new EMULATOR_XENSTORE_DATA content is a sequence of NUL terminated
key/value strings, with the key relative to the device model's xenstore
tree.
A sample might look like (as decoded by verify-stream-v2):
Emulator Xenstore Data (Qemu Upstream, idx 0)
'physmap/1f00000/start_addr' = 'f0000000'
'physmap/1f00000/size' = '800000'
'physmap/1f00000/name' = 'vga.vram'
This patch introduces libxl helpers to save and restore this new format,
which reimplement the existing libxl__toolstack_{save,restore}() logic.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
v2:
* Factor out pointer arithmatic into helper functions
NB: I was unable to remove the rel_start bit, but its use has changed
slightly and is hopefully more clear now.
---
tools/libxl/libxl_dom.c | 138 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 4 ++
2 files changed, 142 insertions(+)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5555fea..feed7b6 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1151,6 +1151,76 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *ptr,
return ret;
}
+/*
+ * Inspect the buffer between start and end, and return a pointer to the
+ * character following the NUL terminator of start, or NULL if start is not
+ * terminated before end.
+ */
+static const char *_next_string(const char *start, const char *end)
+{
+ if (start >= end) return NULL;
+
+ size_t total_len = end - start;
+ size_t len = strnlen(start, total_len);
+
+ if (len == total_len)
+ return NULL;
+ else
+ return start + len + 1;
+}
+
+int libxl__restore_emulator_xenstore_data(libxl__domain_create_state *dcs,
+ const char *ptr, uint32_t size)
+{
+ STATE_AO_GC(dcs->ao);
+ const char *next = ptr, *end = ptr + size, *key, *val;
+ int rc;
+
+ const uint32_t domid = dcs->guest_domid;
+ const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+ const char *xs_root = libxl__device_model_xs_path(gc, dm_domid, domid, "");
+
+ while (next < end) {
+ key = next;
+ next = _next_string(next, end);
+
+ /* Sanitise 'key'. */
+ if (!next) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Key in xenstore data not NUL terminated");
+ goto out;
+ }
+ if (key[0] == '\0') {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "empty key found in xenstore data");
+ goto out;
+ }
+ if (key[0] == '/') {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Key in xenstore data not relative");
+ goto out;
+ }
+
+ val = next;
+ next = _next_string(next, end);
+
+ /* Sanitise 'val'. */
+ if (!next) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Val in xenstore data not NUL terminated");
+ goto out;
+ }
+
+ libxl__xs_write(gc, XBT_NULL,
+ GCSPRINTF("%s/%s", xs_root, key), "%s", val);
+ }
+
+ rc = 0;
+
+ out:
+ return rc;
+}
+
/*==================== Domain suspend (save) ====================*/
static void stream_done(libxl__egc *egc,
@@ -1487,6 +1557,74 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
return ret;
}
+/*
+ * Expand the buffer 'buf' of length 'len', to append 'str' including its NUL
+ * terminator.
+ */
+static void _append_string(libxl__gc *gc, char **buf, uint32_t *len,
+ const char *str)
+{
+ size_t extralen = strlen(str) + 1;
+ char *new = libxl__realloc(gc, *buf, *len + extralen);
+
+ *buf = new;
+ memcpy(new + *len, str, extralen);
+ *len += extralen;
+}
+
+int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
+ char **callee_buf,
+ uint32_t *callee_len)
+{
+ STATE_AO_GC(dss->ao);
+ const char *xs_path;
+ char **entries, *buf = NULL;
+ unsigned int nr_entries, rel_start, i, j, len = 0;
+ int rc;
+
+ const uint32_t domid = dss->domid;
+ const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+
+ rel_start = strlen(libxl__device_model_xs_path(gc, dm_domid, domid, "/"));
+ /* path + rel_start is the xenstore path start from the dm root. */
+
+ xs_path = libxl__device_model_xs_path(gc, dm_domid, domid, "/physmap");
+ if (!xs_path) { rc = 0; goto out; }
+
+ entries = libxl__xs_directory(gc, 0, xs_path, &nr_entries);
+ if (!entries || nr_entries == 0) { rc = 0; goto out; }
+
+ for (i = 0; i < nr_entries; ++i) {
+ static const char *const physmap_subkeys[] = {
+ "start_addr", "size", "name"
+ };
+
+ for (j = 0; j < ARRAY_SIZE(physmap_subkeys); ++j) {
+ const char *key = libxl__device_model_xs_path(gc, dm_domid, domid,
+ "/physmap/%s/%s", entries[i], physmap_subkeys[j]);
+
+ if (!key) { rc = ERROR_FAIL; goto out; }
+
+ const char *val = libxl__xs_read(gc, XBT_NULL, key);
+
+ if (!val) { rc = ERROR_FAIL; goto out; }
+
+ _append_string(gc, &buf, &len, key + rel_start);
+ _append_string(gc, &buf, &len, val);
+ }
+ }
+
+ rc = 0;
+
+ out:
+ if (!rc) {
+ *callee_buf = buf;
+ *callee_len = len;
+ }
+
+ return rc;
+}
+
/*----- remus callbacks -----*/
static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss, int ok);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 22d6a26..da2a785 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3439,6 +3439,10 @@ void libxl__xc_domain_saverestore_async_callback_done(libxl__egc *egc,
(int domid, unsigned int enable, void *data);
_hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
uint32_t *len, void *data);
+_hidden int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
+ char **buf, uint32_t *len);
+_hidden int libxl__restore_emulator_xenstore_data
+ (libxl__domain_create_state *dcs, const char *ptr, uint32_t size);
/* calls libxl__xc_domain_restore_done when done */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH for-4.6 v2 3/6] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content
2015-08-03 15:56 ` [PATCH for-4.6 v2 3/6] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content Andrew Cooper
@ 2015-08-04 9:55 ` Ian Campbell
0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2015-08-04 9:55 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson
On Mon, 2015-08-03 at 16:56 +0100, Andrew Cooper wrote:
> The new EMULATOR_XENSTORE_DATA content is a sequence of NUL terminated
> key/value strings, with the key relative to the device model's xenstore
> tree.
>
> A sample might look like (as decoded by verify-stream-v2):
>
> Emulator Xenstore Data (Qemu Upstream, idx 0)
> 'physmap/1f00000/start_addr' = 'f0000000'
> 'physmap/1f00000/size' = '800000'
> 'physmap/1f00000/name' = 'vga.vram'
>
> This patch introduces libxl helpers to save and restore this new format,
> which reimplement the existing libxl__toolstack_{save,restore}() logic.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> v2:
> * Factor out pointer arithmatic into helper functions
>
> NB: I was unable to remove the rel_start bit
I think you got almost all the way there and it is a pretty easy series of
changes to get rid of it. See below.
> , but its use has changed
> slightly and is hopefully more clear now.
> ---
> tools/libxl/libxl_dom.c | 138 ++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 4 ++
> 2 files changed, 142 insertions(+)
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5555fea..feed7b6 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1151,6 +1151,76 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *ptr,
> return ret;
> }
>
> +/*
> + * Inspect the buffer between start and end, and return a pointer to the
> + * character following the NUL terminator of start, or NULL if start is not
> + * terminated before end.
> + */
> +static const char *_next_string(const char *start, const char *end)
I can never remember who owns the _[a-z].* namespace and I can't be
bothered to look it up right now, but even if it is the application I think
there is no real reason to use it here or for append_string anyway, their
names are OK for static functions without the leading _.
> +int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
> + char **callee_buf,
> + uint32_t *callee_len)
> +{
> + STATE_AO_GC(dss->ao);
> + const char *xs_path;
> + char **entries, *buf = NULL;
> + unsigned int nr_entries, rel_start, i, j, len = 0;
> + int rc;
> +
> + const uint32_t domid = dss->domid;
> + const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> +
> + rel_start = strlen(libxl__device_model_xs_path(gc, dm_domid, domid, "/"));
Instead of allocating this string just to count it's length, keep the
string itself as "xs_path" and...
> + /* path + rel_start is the xenstore path start from the dm root. */
> +
> + xs_path = libxl__device_model_xs_path(gc, dm_domid, domid, "/physmap");
> + if (!xs_path) { rc = 0; goto out; }
... drop this, and ...
> +
> + entries = libxl__xs_directory(gc, 0, xs_path, &nr_entries);
s|xs_path|GCSPRINTF("%s/%s", xs_path, "physmap")|
(maybe fiddle with where the / lives depending on what xs_path looks like).
> + if (!entries || nr_entries == 0) { rc = 0; goto out; }
> +
> + for (i = 0; i < nr_entries; ++i) {
> + static const char *const physmap_subkeys[] = {
> + "start_addr", "size", "name"
> + };
> +
> + for (j = 0; j < ARRAY_SIZE(physmap_subkeys); ++j) {
> + const char *key = libxl__device_model_xs_path(gc, dm_domid, domid,
> + "/physmap/%s/%s", entries[i], physmap_subkeys[j]);
... make this: GCSPRINTF("physmap/%s/%s", entries[i], subkeys[j]). and...
> + if (!key) { rc = ERROR_FAIL; goto out; }
> +
> + const char *val = libxl__xs_read(gc, XBT_NULL, key);
... s|key|GCSPRINTF("%s/%s", xs_path, key)| (or use another variable for
clarity if you prefer)
(Before you complain about the second allocation
libxl__device_model_xs_path is two anyway)
Then...
> +
> + if (!val) { rc = ERROR_FAIL; goto out; }
> +
> + _append_string(gc, &buf, &len, key + rel_start);
append_string(gc, ..., key)
> + _append_string(gc, &buf, &len, val);
append_string(gc, ..., val)
Ian.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-4.6 v2 4/6] tools/libxl: Prepare to write multiple records with EMULATOR headers
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
` (2 preceding siblings ...)
2015-08-03 15:56 ` [PATCH for-4.6 v2 3/6] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
2015-08-04 9:59 ` Ian Campbell
2015-08-03 15:56 ` [PATCH for-4.6 v2 5/6] libxl/save&restore&convert: Switch to new EMULATOR_XENSTORE_DATA records Andrew Cooper
2015-08-03 15:56 ` [PATCH for-4.6 v2 6/6] tools/libxl: Drop all legacy "toolstack" record infrastructure Andrew Cooper
5 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Wei Liu
With the newly specified EMULATOR_XENSTORE_DATA record, there are two
libxl records with an emulator subheader. Refactor the existing code to
make future additions easier, and rename some functions for consistency
with the new scheme.
* Calculate the subheader at stream start time, rather than on the fly.
Its contents are not going to change.
* Introduce a new setup_emulator_write() to insert a sub header in the
appropriate place before a blob of data.
* Rename *toolstack_* to *emulator_xenstore_*
* Rename *emulator_* to *emulator_context_*
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
v2:
* Extra renaming for consistency.
---
tools/libxl/libxl_internal.h | 3 +-
tools/libxl/libxl_stream_write.c | 182 +++++++++++++++++++++++---------------
2 files changed, 113 insertions(+), 72 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index da2a785..fdef45e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3016,10 +3016,11 @@ struct libxl__stream_write_state {
libxl__datacopier_state dc;
sws_record_done_cb record_done_callback;
- /* Only used when constructing an EMULATOR record. */
+ /* Only used when constructing EMULATOR records. */
libxl__datacopier_state emu_dc;
libxl__carefd *emu_carefd;
libxl__sr_rec_hdr emu_rec_hdr;
+ libxl__sr_emulator_hdr emu_sub_hdr;
void *emu_body;
};
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index be8f548..55439dd 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -38,14 +38,16 @@
* The main loop for a plain VM writes:
* - Stream header
* - Libxc record
- * - Toolstack record
- * - if (hvm), Qemu record
+ * - (optional) Emulator xenstore record
+ * - if (hvm)
+ * - Emulator context record
* - End record
*
* For checkpointed stream, there is a second loop which is triggered by a
* save-helper checkpoint callback. It writes:
- * - Toolstack record
- * - if (hvm), Qemu record
+ * - (optional) Emulator xenstore record
+ * - if (hvm)
+ * - Emulator context record
* - Checkpoint end record
*/
@@ -69,17 +71,17 @@ static void stream_header_done(libxl__egc *egc,
static void libxc_header_done(libxl__egc *egc,
libxl__stream_write_state *stream);
/* libxl__xc_domain_save_done() lives here, event-order wise. */
-static void write_toolstack_record(libxl__egc *egc,
- libxl__stream_write_state *stream);
-static void toolstack_record_done(libxl__egc *egc,
- libxl__stream_write_state *stream);
-static void write_emulator_record(libxl__egc *egc,
- libxl__stream_write_state *stream);
-static void emulator_read_done(libxl__egc *egc,
- libxl__datacopier_state *dc,
- int rc, int onwrite, int errnoval);
-static void emulator_record_done(libxl__egc *egc,
- libxl__stream_write_state *stream);
+static void write_emulator_xenstore_record(libxl__egc *egc,
+ libxl__stream_write_state *stream);
+static void emulator_xenstore_record_done(libxl__egc *egc,
+ libxl__stream_write_state *stream);
+static void write_emulator_context_record(libxl__egc *egc,
+ libxl__stream_write_state *stream);
+static void emulator_context_read_done(libxl__egc *egc,
+ libxl__datacopier_state *dc,
+ int rc, int onwrite, int errnoval);
+static void emulator_context_record_done(libxl__egc *egc,
+ libxl__stream_write_state *stream);
static void write_end_record(libxl__egc *egc,
libxl__stream_write_state *stream);
@@ -95,12 +97,14 @@ static void write_done(libxl__egc *egc,
libxl__datacopier_state *dc,
int rc, int onwrite, int errnoval);
-/* Helper to set up reading some data from the stream. */
-static void setup_write(libxl__egc *egc,
- libxl__stream_write_state *stream,
- const char *what,
- libxl__sr_rec_hdr *hdr, void *body,
- sws_record_done_cb cb)
+/* Generic helper to set up writing some data to the stream. */
+static void setup_generic_write(libxl__egc *egc,
+ libxl__stream_write_state *stream,
+ const char *what,
+ libxl__sr_rec_hdr *hdr,
+ libxl__sr_emulator_hdr *emu_hdr,
+ void *body,
+ sws_record_done_cb cb)
{
static const uint8_t zero_padding[1U << REC_ALIGN_ORDER] = { 0 };
@@ -120,13 +124,21 @@ static void setup_write(libxl__egc *egc,
}
size_t padsz = ROUNDUP(hdr->length, REC_ALIGN_ORDER) - hdr->length;
+ uint32_t length = hdr->length;
/* Insert header */
libxl__datacopier_prefixdata(egc, dc, hdr, sizeof(*hdr));
+ /* Optional emulator sub-header */
+ if (emu_hdr) {
+ assert(length >= sizeof(*emu_hdr));
+ libxl__datacopier_prefixdata(egc, dc, emu_hdr, sizeof(*emu_hdr));
+ length -= sizeof(*emu_hdr);
+ }
+
/* Optional body */
if (body)
- libxl__datacopier_prefixdata(egc, dc, body, hdr->length);
+ libxl__datacopier_prefixdata(egc, dc, body, length);
/* Any required padding */
if (padsz > 0)
@@ -135,6 +147,30 @@ static void setup_write(libxl__egc *egc,
stream->record_done_callback = cb;
}
+/* Helper to set up writing a regular record to the stream. */
+static void setup_write(libxl__egc *egc,
+ libxl__stream_write_state *stream,
+ const char *what,
+ libxl__sr_rec_hdr *hdr,
+ void *body,
+ sws_record_done_cb cb)
+{
+ setup_generic_write(egc, stream, what, hdr, NULL, body, cb);
+}
+
+/* Helper to set up writing a record with an emulator prefix to the stream. */
+static void setup_emulator_write(libxl__egc *egc,
+ libxl__stream_write_state *stream,
+ const char *what,
+ libxl__sr_rec_hdr *hdr,
+ libxl__sr_emulator_hdr *emu_hdr,
+ void *body,
+ sws_record_done_cb cb)
+{
+ setup_generic_write(egc, stream, what, hdr, emu_hdr, body, cb);
+}
+
+
static void write_done(libxl__egc *egc,
libxl__datacopier_state *dc,
int rc, int onwrite, int errnoval)
@@ -169,6 +205,7 @@ void libxl__stream_write_init(libxl__stream_write_state *stream)
FILLZERO(stream->emu_dc);
stream->emu_carefd = NULL;
FILLZERO(stream->emu_rec_hdr);
+ FILLZERO(stream->emu_sub_hdr);
stream->emu_body = NULL;
}
@@ -176,6 +213,7 @@ void libxl__stream_write_start(libxl__egc *egc,
libxl__stream_write_state *stream)
{
libxl__datacopier_state *dc = &stream->dc;
+ libxl__domain_suspend_state *dss = stream->dss;
STATE_AO_GC(stream->ao);
struct libxl__sr_hdr hdr;
int rc = 0;
@@ -184,6 +222,24 @@ void libxl__stream_write_start(libxl__egc *egc,
stream->running = true;
+ if (dss->type == LIBXL_DOMAIN_TYPE_HVM) {
+ switch (libxl__device_model_version_running(gc, dss->domid)) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ stream->emu_sub_hdr.id = EMULATOR_QEMU_TRADITIONAL;
+ break;
+
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ stream->emu_sub_hdr.id = EMULATOR_QEMU_UPSTREAM;
+ break;
+
+ default:
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Unknown emulator for HVM domain\n");
+ goto err;
+ }
+ stream->emu_sub_hdr.index = 0;
+ }
+
dc->ao = ao;
dc->readfd = -1;
dc->writewhat = "stream header";
@@ -216,7 +272,7 @@ void libxl__stream_write_start_checkpoint(libxl__egc *egc,
assert(!stream->in_checkpoint);
stream->in_checkpoint = true;
- write_toolstack_record(egc, stream);
+ write_emulator_xenstore_record(egc, stream);
}
void libxl__stream_write_abort(libxl__egc *egc,
@@ -290,48 +346,47 @@ void libxl__xc_domain_save_done(libxl__egc *egc, void *dss_void,
* If the stream is not still alive, we must not continue any work.
*/
if (libxl__stream_write_inuse(stream))
- write_toolstack_record(egc, stream);
+ write_emulator_xenstore_record(egc, stream);
}
-static void write_toolstack_record(libxl__egc *egc,
- libxl__stream_write_state *stream)
+static void write_emulator_xenstore_record(libxl__egc *egc,
+ libxl__stream_write_state *stream)
{
libxl__domain_suspend_state *dss = stream->dss;
STATE_AO_GC(stream->ao);
struct libxl__sr_rec_hdr rec;
int rc;
- uint8_t *toolstack_buf = NULL; /* We must free this. */
- uint32_t toolstack_len;
+ uint8_t *buf = NULL; /* We must free this. */
+ uint32_t len;
- rc = libxl__toolstack_save(dss->domid, &toolstack_buf,
- &toolstack_len, dss);
+ rc = libxl__toolstack_save(dss->domid, &buf, &len, dss);
if (rc)
goto err;
FILLZERO(rec);
rec.type = REC_TYPE_XENSTORE_DATA;
- rec.length = toolstack_len;
+ rec.length = len;
- setup_write(egc, stream, "toolstack record",
- &rec, toolstack_buf,
- toolstack_record_done);
+ setup_write(egc, stream, "emulator xenstore record",
+ &rec, buf,
+ emulator_xenstore_record_done);
- free(toolstack_buf);
+ free(buf);
return;
err:
assert(rc);
- free(toolstack_buf);
+ free(buf);
stream_complete(egc, stream, rc);
}
-static void toolstack_record_done(libxl__egc *egc,
- libxl__stream_write_state *stream)
+static void emulator_xenstore_record_done(libxl__egc *egc,
+ libxl__stream_write_state *stream)
{
libxl__domain_suspend_state *dss = stream->dss;
if (dss->type == LIBXL_DOMAIN_TYPE_HVM)
- write_emulator_record(egc, stream);
+ write_emulator_context_record(egc, stream);
else {
if (stream->in_checkpoint)
write_checkpoint_end_record(egc, stream);
@@ -340,14 +395,13 @@ static void toolstack_record_done(libxl__egc *egc,
}
}
-static void write_emulator_record(libxl__egc *egc,
- libxl__stream_write_state *stream)
+static void write_emulator_context_record(libxl__egc *egc,
+ libxl__stream_write_state *stream)
{
libxl__domain_suspend_state *dss = stream->dss;
libxl__datacopier_state *dc = &stream->emu_dc;
STATE_AO_GC(stream->ao);
struct libxl__sr_rec_hdr *rec = &stream->emu_rec_hdr;
- struct libxl__sr_emulator_hdr *ehdr = NULL;
struct stat st;
int rc;
@@ -355,7 +409,6 @@ static void write_emulator_record(libxl__egc *egc,
/* Convenience aliases */
const char *const filename = dss->dm_savefile;
- const uint32_t domid = dss->domid;
libxl__carefd_begin();
int readfd = open(filename, O_RDONLY);
@@ -379,23 +432,8 @@ static void write_emulator_record(libxl__egc *egc,
}
rec->type = REC_TYPE_EMULATOR_CONTEXT;
- rec->length = st.st_size + sizeof(*ehdr);
- stream->emu_body = ehdr = libxl__malloc(NOGC, rec->length);
-
- switch(libxl__device_model_version_running(gc, domid)) {
- case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
- ehdr->id = EMULATOR_QEMU_TRADITIONAL;
- break;
-
- case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- ehdr->id = EMULATOR_QEMU_UPSTREAM;
- break;
-
- default:
- rc = ERROR_FAIL;
- goto err;
- }
- ehdr->index = 0;
+ rec->length = st.st_size + sizeof(stream->emu_sub_hdr);
+ stream->emu_body = libxl__malloc(NOGC, st.st_size);
FILLZERO(*dc);
dc->ao = stream->ao;
@@ -404,9 +442,9 @@ static void write_emulator_record(libxl__egc *egc,
dc->readfd = readfd;
dc->writefd = -1;
dc->maxsz = -1;
- dc->readbuf = stream->emu_body + sizeof(*ehdr);
- dc->bytes_to_read = rec->length - sizeof(*ehdr);
- dc->callback = emulator_read_done;
+ dc->readbuf = stream->emu_body;
+ dc->bytes_to_read = st.st_size;
+ dc->callback = emulator_context_read_done;
rc = libxl__datacopier_start(dc);
if (rc)
@@ -419,9 +457,9 @@ static void write_emulator_record(libxl__egc *egc,
stream_complete(egc, stream, rc);
}
-static void emulator_read_done(libxl__egc *egc,
- libxl__datacopier_state *dc,
- int rc, int onwrite, int errnoval)
+static void emulator_context_read_done(libxl__egc *egc,
+ libxl__datacopier_state *dc,
+ int rc, int onwrite, int errnoval)
{
libxl__stream_write_state *stream = CONTAINER_OF(dc, *stream, emu_dc);
STATE_AO_GC(stream->ao);
@@ -434,13 +472,15 @@ static void emulator_read_done(libxl__egc *egc,
libxl__carefd_close(stream->emu_carefd);
stream->emu_carefd = NULL;
- setup_write(egc, stream, "emulator record",
- &stream->emu_rec_hdr, stream->emu_body,
- emulator_record_done);
+ setup_emulator_write(egc, stream, "emulator record",
+ &stream->emu_rec_hdr,
+ &stream->emu_sub_hdr,
+ stream->emu_body,
+ emulator_context_record_done);
}
-static void emulator_record_done(libxl__egc *egc,
- libxl__stream_write_state *stream)
+static void emulator_context_record_done(libxl__egc *egc,
+ libxl__stream_write_state *stream)
{
free(stream->emu_body);
stream->emu_body = NULL;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH for-4.6 v2 4/6] tools/libxl: Prepare to write multiple records with EMULATOR headers
2015-08-03 15:56 ` [PATCH for-4.6 v2 4/6] tools/libxl: Prepare to write multiple records with EMULATOR headers Andrew Cooper
@ 2015-08-04 9:59 ` Ian Campbell
0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2015-08-04 9:59 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson
On Mon, 2015-08-03 at 16:56 +0100, Andrew Cooper wrote:
> With the newly specified EMULATOR_XENSTORE_DATA record, there are two
> libxl records with an emulator subheader. Refactor the existing code to
> make future additions easier, and rename some functions for consistency
> with the new scheme.
>
> * Calculate the subheader at stream start time, rather than on the fly.
> Its contents are not going to change.
> * Introduce a new setup_emulator_write() to insert a sub header in the
> appropriate place before a blob of data.
> * Rename *toolstack_* to *emulator_xenstore_*
> * Rename *emulator_* to *emulator_context_*
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH for-4.6 v2 5/6] libxl/save&restore&convert: Switch to new EMULATOR_XENSTORE_DATA records
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
` (3 preceding siblings ...)
2015-08-03 15:56 ` [PATCH for-4.6 v2 4/6] tools/libxl: Prepare to write multiple records with EMULATOR headers Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
2015-08-03 15:56 ` [PATCH for-4.6 v2 6/6] tools/libxl: Drop all legacy "toolstack" record infrastructure Andrew Cooper
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell
Read and write "toolstack" information using the new
EMULATOR_XENSTORE_DATA record, and have the conversion script take care
of the old format.
The entire libxc and libxl migration v2 streams are now bitness-neutral
in their records.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
Note: This is a binary incompatibility in the libxl migration stream over
the past few weeks, but we don't really care about maintaining
compatibility between development versions of Xen, and it does fix the
one remaining bitness issue in the stream.
---
tools/libxl/libxl_stream_read.c | 16 +++++--
tools/libxl/libxl_stream_write.c | 25 ++++++-----
tools/python/scripts/convert-legacy-stream | 66 +++++++++++++++++++++++++---
3 files changed, 85 insertions(+), 22 deletions(-)
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index c555542..4ec29da 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -538,14 +538,22 @@ static bool process_record(libxl__egc *egc,
libxl__xc_domain_restore(egc, dcs, &stream->shs, 0, 0, 0);
break;
- case REC_TYPE_XENSTORE_DATA:
- rc = libxl__toolstack_restore(dcs->guest_domid, rec->body,
- rec->hdr.length, &stream->shs);
+ case REC_TYPE_EMULATOR_XENSTORE_DATA:
+ if (rec->hdr.length < sizeof(libxl__sr_emulator_hdr)) {
+ rc = ERROR_FAIL;
+ LOG(ERROR,
+ "Emulator xenstore data record too short to contain header");
+ goto err;
+ }
+
+ rc = libxl__restore_emulator_xenstore_data(dcs,
+ rec->body + sizeof(libxl__sr_emulator_hdr),
+ rec->hdr.length - sizeof(libxl__sr_emulator_hdr));
if (rc)
goto err;
/*
- * libxl__toolstack_restore() is a synchronous function.
+ * libxl__restore_emulator_xenstore_data() is a synchronous function.
* Request that our caller queues another action for us.
*/
further_action_needed = true;
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 55439dd..10a9e0f 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -356,27 +356,30 @@ static void write_emulator_xenstore_record(libxl__egc *egc,
STATE_AO_GC(stream->ao);
struct libxl__sr_rec_hdr rec;
int rc;
- uint8_t *buf = NULL; /* We must free this. */
- uint32_t len;
+ char *buf = NULL;
+ uint32_t len = 0;
- rc = libxl__toolstack_save(dss->domid, &buf, &len, dss);
+ rc = libxl__save_emulator_xenstore_data(dss, &buf, &len);
if (rc)
goto err;
- FILLZERO(rec);
- rec.type = REC_TYPE_XENSTORE_DATA;
- rec.length = len;
+ /* No record? - All done. */
+ if (len == 0) {
+ emulator_xenstore_record_done(egc, stream);
+ return;
+ }
- setup_write(egc, stream, "emulator xenstore record",
- &rec, buf,
- emulator_xenstore_record_done);
+ FILLZERO(rec);
+ rec.type = REC_TYPE_EMULATOR_XENSTORE_DATA;
+ rec.length = len + sizeof(stream->emu_sub_hdr);
- free(buf);
+ setup_emulator_write(egc, stream, "emulator xenstore record",
+ &rec, &stream->emu_sub_hdr, buf,
+ emulator_xenstore_record_done);
return;
err:
assert(rc);
- free(buf);
stream_complete(egc, stream, rc);
}
diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream
index 16331a4..41fee10 100755
--- a/tools/python/scripts/convert-legacy-stream
+++ b/tools/python/scripts/convert-legacy-stream
@@ -70,7 +70,7 @@ class VM(object):
# libxl
self.libxl = fmt == "libxl"
- self.xenstore = [] # Deferred "toolstack" records
+ self.emu_xenstore = "" # NUL terminated key&val pairs from "toolstack" records
def write_libxc_ihdr():
stream_write(pack(libxc.IHDR_FORMAT,
@@ -169,8 +169,10 @@ def write_libxl_end():
def write_libxl_libxc_context():
write_record(libxl.REC_TYPE_libxc_context, "")
-def write_libxl_xenstore_data(data):
- write_record(libxl.REC_TYPE_xenstore_data, data)
+def write_libxl_emulator_xenstore_data(data):
+ write_record(libxl.REC_TYPE_emulator_xenstore_data,
+ pack(libxl.EMULATOR_HEADER_FORMAT,
+ libxl.EMULATOR_ID_unknown, 0) + data)
def write_libxl_emulator_context(blob):
write_record(libxl.REC_TYPE_emulator_context,
@@ -297,6 +299,57 @@ def read_pv_tail(vm):
write_record(libxc.REC_TYPE_end, "")
+def read_libxl_toolstack(vm, data):
+
+ if len(data) < 8:
+ raise StreamError("Overly short libxl toolstack data")
+
+ ver, count = unpack("=II", data[:8])
+ data = data[8:]
+
+ if ver != 1:
+ raise StreamError("Cannot decode libxl toolstack version %u" % (ver, ))
+ info(" Version %u, count %u" % (ver, count))
+
+ for x in range(count):
+
+ if len(data) < 28:
+ raise StreamError("Remaining data too short for physmap header")
+
+ phys, start, size, namelen = unpack("=QQQI", data[:28])
+ data = data[28:]
+
+ if namelen == 0:
+ raise StreamError("No physmap info name")
+
+ # 64bit leaked 4 bytes of padding onto the end of name
+ if twidth == 64:
+ namelen += 4
+
+ if len(data) < namelen:
+ raise StreamError("Remaining data too short for physmap name")
+
+ name = data[:namelen]
+ data = data[namelen:]
+
+ # Strip padding off the end of name
+ if twidth == 64:
+ name = name[:-4]
+
+ if name[-1] != '\x00':
+ raise StreamError("physmap name not NUL terminated")
+
+ root = "physmap/%x" % (phys,)
+ kv = [root + "/start_addr", "%x" % (start, ),
+ root + "/size", "%x" % (size, ),
+ root + "/name", name[:-1]]
+
+ for key, val in zip(kv[0::2], kv[1::2]):
+ info(" '%s' = '%s'" % (key, val))
+
+ vm.emu_xenstore += '\x00'.join(kv) + '\x00'
+
+
def read_chunks(vm):
hvm_params = []
@@ -441,7 +494,7 @@ def read_chunks(vm):
info(" Toolstack Data: sz 0x%x" % (sz, ))
if vm.libxl:
- vm.xenstore.append(data)
+ read_libxl_toolstack(vm, data)
else:
info(" Discarding")
@@ -544,9 +597,8 @@ def read_legacy_stream(vm):
else:
read_hvm_tail(vm)
- if vm.libxl:
- for rec in vm.xenstore:
- write_libxl_xenstore_data(rec)
+ if vm.libxl and len(vm.emu_xenstore):
+ write_libxl_emulator_xenstore_data(vm.emu_xenstore)
if not pv and (vm.libxl or qemu):
read_qemu(vm)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH for-4.6 v2 6/6] tools/libxl: Drop all legacy "toolstack" record infrastructure
2015-08-03 15:56 [PATCH for-4.6 v2 0/6] Fix libxl TOOLSTACK records for migration v2 Andrew Cooper
` (4 preceding siblings ...)
2015-08-03 15:56 ` [PATCH for-4.6 v2 5/6] libxl/save&restore&convert: Switch to new EMULATOR_XENSTORE_DATA records Andrew Cooper
@ 2015-08-03 15:56 ` Andrew Cooper
5 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2015-08-03 15:56 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Campbell
No functional change. It is not used any more.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
---
tools/libxl/libxl_dom.c | 223 ----------------------------------
tools/libxl/libxl_internal.h | 4 -
tools/libxl/libxl_sr_stream_format.h | 1 -
tools/python/xen/migration/libxl.py | 1 -
4 files changed, 229 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index feed7b6..3cc715a 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1031,126 +1031,6 @@ int libxl__qemu_traditional_cmd(libxl__gc *gc, uint32_t domid,
return libxl__xs_write(gc, XBT_NULL, path, "%s", cmd);
}
-struct libxl__physmap_info {
- uint64_t phys_offset;
- uint64_t start_addr;
- uint64_t size;
- uint32_t namelen;
- char name[];
-};
-
-/* Bump version every time when toolstack saved data changes.
- * Different types of data are arranged in the specified order.
- *
- * Version 1:
- * uint32_t version
- * QEMU physmap data:
- * uint32_t count
- * libxl__physmap_info * count
- */
-#define TOOLSTACK_SAVE_VERSION 1
-
-static inline char *restore_helper(libxl__gc *gc, uint32_t dm_domid,
- uint32_t domid,
- uint64_t phys_offset, char *node)
-{
- return libxl__device_model_xs_path(gc, dm_domid, domid,
- "/physmap/%"PRIx64"/%s",
- phys_offset, node);
-}
-
-static int libxl__toolstack_restore_qemu(libxl__gc *gc, uint32_t domid,
- const uint8_t *ptr, uint32_t size)
-{
- int ret, i;
- uint32_t count;
- char *xs_path;
- uint32_t dm_domid;
- struct libxl__physmap_info *pi;
-
- if (size < sizeof(count)) {
- LOG(ERROR, "wrong size");
- ret = -1;
- goto out;
- }
-
- memcpy(&count, ptr, sizeof(count));
- ptr += sizeof(count);
-
- if (size < sizeof(count) + count*(sizeof(struct libxl__physmap_info))) {
- LOG(ERROR, "wrong size");
- ret = -1;
- goto out;
- }
-
- dm_domid = libxl_get_stubdom_id(CTX, domid);
- for (i = 0; i < count; i++) {
- pi = (struct libxl__physmap_info*) ptr;
- ptr += sizeof(struct libxl__physmap_info) + pi->namelen;
-
- xs_path = restore_helper(gc, dm_domid, domid,
- pi->phys_offset, "start_addr");
- ret = libxl__xs_write(gc, 0, xs_path, "%"PRIx64, pi->start_addr);
- if (ret) goto out;
-
- xs_path = restore_helper(gc, dm_domid, domid, pi->phys_offset, "size");
- ret = libxl__xs_write(gc, 0, xs_path, "%"PRIx64, pi->size);
- if (ret) goto out;
-
- if (pi->namelen > 0) {
- xs_path = restore_helper(gc, dm_domid, domid,
- pi->phys_offset, "name");
- ret = libxl__xs_write(gc, 0, xs_path, "%s", pi->name);
- if (ret) goto out;
- }
- }
-
- ret = 0;
-out:
- return ret;
-
-}
-
-static int libxl__toolstack_restore_v1(libxl__gc *gc, uint32_t domid,
- const uint8_t *ptr, uint32_t size)
-{
- return libxl__toolstack_restore_qemu(gc, domid, ptr, size);
-}
-
-int libxl__toolstack_restore(uint32_t domid, const uint8_t *ptr,
- uint32_t size, void *user)
-{
- libxl__save_helper_state *shs = user;
- libxl__domain_create_state *dcs = shs->caller_state;
- STATE_AO_GC(dcs->ao);
- int ret;
- uint32_t version = 0, bufsize;
-
- LOG(DEBUG,"domain=%"PRIu32" toolstack data size=%"PRIu32, domid, size);
-
- if (size < sizeof(version)) {
- LOG(ERROR, "wrong size");
- ret = -1;
- goto out;
- }
-
- memcpy(&version, ptr, sizeof(version));
- ptr += sizeof(version);
- bufsize = size - sizeof(version);
-
- switch (version) {
- case 1:
- ret = libxl__toolstack_restore_v1(gc, domid, ptr, bufsize);
- break;
- default:
- LOG(ERROR, "wrong version");
- ret = -1;
- }
-
-out:
- return ret;
-}
-
/*
* Inspect the buffer between start and end, and return a pointer to the
* character following the NUL terminator of start, or NULL if start is not
@@ -1454,109 +1334,6 @@ static void switch_logdirty_done(libxl__egc *egc,
/*----- callbacks, called by xc_domain_save -----*/
-static inline char *physmap_path(libxl__gc *gc, uint32_t dm_domid,
- uint32_t domid,
- char *phys_offset, char *node)
-{
- return libxl__device_model_xs_path(gc, dm_domid, domid,
- "/physmap/%s/%s",
- phys_offset, node);
-}
-
-int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
- uint32_t *len, void *dss_void)
-{
- libxl__domain_suspend_state *dss = dss_void;
- int ret;
- STATE_AO_GC(dss->ao);
- int i = 0;
- uint32_t version = TOOLSTACK_SAVE_VERSION;
- uint8_t *ptr = NULL;
-
- ret = -1;
-
- /* Version number */
- *len = sizeof(version);
- *buf = calloc(1, *len);
- if (*buf == NULL) goto out;
- ptr = *buf;
- memcpy(ptr, &version, sizeof(version));
-
- /* QEMU physmap data */
- {
- char **entries = NULL, *xs_path;
- struct libxl__physmap_info *pi;
- uint32_t dm_domid;
- char *start_addr = NULL, *size = NULL, *phys_offset = NULL;
- char *name = NULL;
- unsigned int num = 0;
- uint32_t count = 0, namelen = 0;
-
- dm_domid = libxl_get_stubdom_id(CTX, domid);
-
- xs_path = libxl__device_model_xs_path(gc, dm_domid, domid,
- "/physmap");
- entries = libxl__xs_directory(gc, 0, xs_path, &num);
- count = num;
-
- *len += sizeof(count);
- *buf = realloc(*buf, *len);
- if (*buf == NULL) goto out;
- ptr = *buf + sizeof(version);
- memcpy(ptr, &count, sizeof(count));
- ptr += sizeof(count);
-
- for (i = 0; i < count; i++) {
- unsigned long offset;
- phys_offset = entries[i];
- if (phys_offset == NULL) {
- LOG(ERROR, "phys_offset %d is NULL", i);
- goto out;
- }
-
- xs_path = physmap_path(gc, dm_domid, domid, phys_offset,
- "start_addr");
- start_addr = libxl__xs_read(gc, 0, xs_path);
- if (start_addr == NULL) {
- LOG(ERROR, "%s is NULL", xs_path);
- goto out;
- }
-
- xs_path = physmap_path(gc, dm_domid, domid, phys_offset, "size");
- size = libxl__xs_read(gc, 0, xs_path);
- if (size == NULL) {
- LOG(ERROR, "%s is NULL", xs_path);
- goto out;
- }
-
- xs_path = physmap_path(gc, dm_domid, domid, phys_offset, "name");
- name = libxl__xs_read(gc, 0, xs_path);
- if (name == NULL)
- namelen = 0;
- else
- namelen = strlen(name) + 1;
- *len += namelen + sizeof(struct libxl__physmap_info);
- offset = ptr - (*buf);
- *buf = realloc(*buf, *len);
- if (*buf == NULL) goto out;
- ptr = (*buf) + offset;
- pi = (struct libxl__physmap_info *) ptr;
- pi->phys_offset = strtoll(phys_offset, NULL, 16);
- pi->start_addr = strtoll(start_addr, NULL, 16);
- pi->size = strtoll(size, NULL, 16);
- pi->namelen = namelen;
- memcpy(pi->name, name, namelen);
- ptr += sizeof(struct libxl__physmap_info) + namelen;
- }
- }
-
- LOG(DEBUG,"domain=%"PRIu32" toolstack data size=%"PRIu32, domid, *len);
-
- ret = 0;
-out:
- return ret;
-}
-
/*
* Expand the buffer 'buf' of length 'len', to append 'str' including its NUL
* terminator.
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fdef45e..ad2a090 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1091,8 +1091,6 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
const char *old_name, const char *new_name,
xs_transaction_t trans);
-_hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
- uint32_t size, void *data);
_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
_hidden const char *libxl__userdata_path(libxl__gc *gc, uint32_t domid,
@@ -3438,8 +3436,6 @@ void libxl__xc_domain_saverestore_async_callback_done(libxl__egc *egc,
_hidden void libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned int enable, void *data);
-_hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
- uint32_t *len, void *data);
_hidden int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
char **buf, uint32_t *len);
_hidden int libxl__restore_emulator_xenstore_data
diff --git a/tools/libxl/libxl_sr_stream_format.h b/tools/libxl/libxl_sr_stream_format.h
index 4c23367..54da360 100644
--- a/tools/libxl/libxl_sr_stream_format.h
+++ b/tools/libxl/libxl_sr_stream_format.h
@@ -33,7 +33,6 @@
#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
diff --git a/tools/python/xen/migration/libxl.py b/tools/python/xen/migration/libxl.py
index 1a9ca87..fc0acf6 100644
--- a/tools/python/xen/migration/libxl.py
+++ b/tools/python/xen/migration/libxl.py
@@ -34,7 +34,6 @@ RH_FORMAT = "II"
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
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread