From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 19/27] tools/libxc+libxl+xl: Restore v2 streams Date: Tue, 16 Jun 2015 16:23:04 +0100 Message-ID: <55803F58.1070405@citrix.com> References: <1434375880-30914-1-git-send-email-andrew.cooper3@citrix.com> <1434375880-30914-20-git-send-email-andrew.cooper3@citrix.com> <1434466399.13744.205.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1434466399.13744.205.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Wei Liu , Yang Hongyang , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 16/06/15 15:53, Ian Campbell wrote: > On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote: >> @@ -377,6 +384,28 @@ static void record_body_done(libxl__egc *egc, >> stream_failed(egc, stream, ret); >> } >> >> +void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void, >> + int ret, int retval, int errnoval) >> +{ >> + libxl__domain_create_state *dcs = dcs_void; >> + STATE_AO_GC(dcs->ao); >> + >> + if (ret) >> + goto err; >> + >> + if (retval) { >> + LOGEV(ERROR, errnoval, "restoring domain"); >> + ret = ERROR_FAIL; >> + goto err; >> + } >> + >> + libxl__stream_read_continue(egc, &dcs->srs); > continue? Is this something to do with checkpointing? No. Simply "continue reading records from the stream". The code has changed since the first iteration, where libxl__stream_read_continue() was called from outside of this translation unit. As it currently stands, I should make it a static function. > >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> index 23f27d4..7418d92 100644 >> --- a/tools/libxl/libxl_types.idl >> +++ b/tools/libxl/libxl_types.idl >> @@ -346,6 +346,8 @@ libxl_domain_create_info = Struct("domain_create_info",[ >> >> libxl_domain_restore_params = Struct("domain_restore_params", [ > At some point we will need a LIBXL_HAVE #define. > >> ("checkpointed_stream", integer), >> + ("stream_version", uint32, {'init_val': '1'}), > If we aren't going to go for an IDL enum rather than a uint32 you > probably just want the bare integer 1. > > But, I suspect we would prefer an enum, i.e an explicit list of known > versions, rather than an integer? Ideally, this should match the version field in the libxl stream header record. Realistically, I never expect version 2 needing bumping to version 3. > > I wonder when, if ever, we will be able to flip this to 2? I suppose > whenever the legacy conversion stuff gets pulled out. > >> + ("legacy_width", uint32), > From what I've seen so far this is never user provided but is internal > to libxl and detected[0] at runtime. As such it belongs somewhere else > other than in the public API. > > [0] FVO "detected" == "hardcoded depending on the build arch" The intention was originally to expose it to the user. While this is possible for `xl restore /path/to/legacy/save --was-32bit-toolstack`, it is much harder for the `xl migrate` case where we cannot control the sending side. I am half tempted to say that anyone experiencing 32->64bit problems should just use the conversion helper themselves. It is deliberately written to be usable on the command line as well as automatically. ~Andrew