Message ID | 20191224151932.6304-6-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Support CPUID/MSR data in migration streams | expand |
Andrew Cooper writes ("[PATCH 05/12] tools/migration: Drop IHDR_VERSION constant from libxc and python"): > Migration v3 is in the process of being introduced, meaning that the code has > to cope with both versions. Use an explicit 2 for now. > > For the verify-stream-v2 and convert-legacy-stream scripts, update text to say > "v2 (or later)". What matters is the distinction vs legacy streams. How about introducing enum { IHDR_VERSION_2 = 2 } or some such ? In C it can be hard otherwise to find all the relevant tests. Being able to grep for IHDR_VERSION would help. So I would prefer manifest constants of some kind to unvarnished integers. Ian.
On 14/01/2020 16:05, Ian Jackson wrote: > Andrew Cooper writes ("[PATCH 05/12] tools/migration: Drop IHDR_VERSION constant from libxc and python"): >> Migration v3 is in the process of being introduced, meaning that the code has >> to cope with both versions. Use an explicit 2 for now. >> >> For the verify-stream-v2 and convert-legacy-stream scripts, update text to say >> "v2 (or later)". What matters is the distinction vs legacy streams. > How about introducing > enum { IHDR_VERSION_2 = 2 } > or some such ? > > In C it can be hard otherwise to find all the relevant tests. Being > able to grep for IHDR_VERSION would help. So I would prefer manifest > constants of some kind to unvarnished integers. There is exactly (and only ever) one place where this constant is checked. This is a consequence of it being read out of a pipe. It is highly unlikely that the code will gain a second check. ~Andrew
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index 7872b71ab5..0280e55d4b 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -35,10 +35,10 @@ static int read_headers(struct xc_sr_context *ctx) return -1; } - if ( ihdr.version != IHDR_VERSION ) + if ( ihdr.version != 2 ) { - ERROR("Invalid Version: Expected %d, Got %d", - IHDR_VERSION, ihdr.version); + ERROR("Invalid Version: Expected 2, Got %d", + ihdr.version); return -1; } diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 5467965b08..fa1a2e6c2a 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -13,7 +13,7 @@ static int write_headers(struct xc_sr_context *ctx, uint16_t guest_type) struct xc_sr_ihdr ihdr = { .marker = IHDR_MARKER, .id = htonl(IHDR_ID), - .version = htonl(IHDR_VERSION), + .version = htonl(2), .options = htons(IHDR_OPT_LITTLE_ENDIAN), }; struct xc_sr_dhdr dhdr = { diff --git a/tools/libxc/xc_sr_stream_format.h b/tools/libxc/xc_sr_stream_format.h index 37a7da6eab..ae7c0de393 100644 --- a/tools/libxc/xc_sr_stream_format.h +++ b/tools/libxc/xc_sr_stream_format.h @@ -23,7 +23,6 @@ struct xc_sr_ihdr #define IHDR_MARKER 0xffffffffffffffffULL #define IHDR_ID 0x58454E46U -#define IHDR_VERSION 2 #define _IHDR_OPT_ENDIAN 0 #define IHDR_OPT_LITTLE_ENDIAN (0 << _IHDR_OPT_ENDIAN) diff --git a/tools/python/scripts/convert-legacy-stream b/tools/python/scripts/convert-legacy-stream index 2922fb3185..02a194178f 100755 --- a/tools/python/scripts/convert-legacy-stream +++ b/tools/python/scripts/convert-legacy-stream @@ -79,7 +79,7 @@ def write_libxc_ihdr(): stream_write(pack(libxc.IHDR_FORMAT, libxc.IHDR_MARKER, # Marker libxc.IHDR_IDENT, # Ident - libxc.IHDR_VERSION, # Version + 2, # Version libxc.IHDR_OPT_LE, # Options 0, 0)) # Reserved @@ -632,13 +632,13 @@ def main(): usage = ("%prog [options] -i INPUT -o OUTPUT" " -w WIDTH -g GUEST"), description = - "Convert a legacy stream to a v2 stream") + "Convert a legacy stream to a v2 (or later) stream") # Required options parser.add_option("-i", "--in", dest = "fin", metavar = "<FD or FILE>", help = "Legacy input to convert") parser.add_option("-o", "--out", dest = "fout", metavar = "<FD or FILE>", - help = "v2 format output") + help = "v2 (or later) format output") parser.add_option("-w", "--width", dest = "twidth", metavar = "<32/64>", choices = ["32", "64"], help = "Legacy toolstack bitness") diff --git a/tools/python/scripts/verify-stream-v2 b/tools/python/scripts/verify-stream-v2 index 8bac04d566..fe82b86c11 100755 --- a/tools/python/scripts/verify-stream-v2 +++ b/tools/python/scripts/verify-stream-v2 @@ -108,7 +108,7 @@ def main(): parser = OptionParser(usage = "%prog [options]", description = - "Verify a stream according to the v2 spec") + "Verify a stream according to the v2 (or later) spec") # Optional options parser.add_option("-i", "--in", dest = "fin", metavar = "<FD or FILE>", diff --git a/tools/python/xen/migration/libxc.py b/tools/python/xen/migration/libxc.py index 8a800df980..63b3558029 100644 --- a/tools/python/xen/migration/libxc.py +++ b/tools/python/xen/migration/libxc.py @@ -19,7 +19,6 @@ IHDR_MARKER = 0xffffffffffffffff IHDR_IDENT = 0x58454E46 # "XENF" in ASCII -IHDR_VERSION = 2 IHDR_OPT_BIT_ENDIAN = 0 IHDR_OPT_LE = (0 << IHDR_OPT_BIT_ENDIAN) @@ -113,7 +112,7 @@ HVM_PARAMS_FORMAT = "II" class VerifyLibxc(VerifyBase): - """ Verify a Libxc v2 stream """ + """ Verify a Libxc v2 (or later) stream """ def __init__(self, info, read): VerifyBase.__init__(self, info, read) @@ -144,9 +143,9 @@ def verify_ihdr(self): raise StreamError("Bad image id: Expected 0x%x, got 0x%x" % (IHDR_IDENT, ident)) - if version != IHDR_VERSION: - raise StreamError("Unknown image version: Expected %d, got %d" % - (IHDR_VERSION, version)) + if version != 2: + raise StreamError("Unknown image version: Expected 2, got %d" % + (version, )) if options & IHDR_OPT_RESZ_MASK: raise StreamError("Reserved bits set in image options field: 0x%x" %
Migration v3 is in the process of being introduced, meaning that the code has to cope with both versions. Use an explicit 2 for now. For the verify-stream-v2 and convert-legacy-stream scripts, update text to say "v2 (or later)". What matters is the distinction vs legacy streams. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <Ian.Jackson@citrix.com> CC: Wei Liu <wl@xen.org> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- tools/libxc/xc_sr_restore.c | 6 +++--- tools/libxc/xc_sr_save.c | 2 +- tools/libxc/xc_sr_stream_format.h | 1 - tools/python/scripts/convert-legacy-stream | 6 +++--- tools/python/scripts/verify-stream-v2 | 2 +- tools/python/xen/migration/libxc.py | 9 ++++----- 6 files changed, 12 insertions(+), 14 deletions(-)