Re: [Xen-devel] [PATCH v4 --for 4.6 COLOPre 13/25] migration/save: pass checkpointed_stream from libxl to libxc

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-07-16 at 14:05 +0800, Yang Hongyang wrote:
> 
> On 07/15/2015 08:38 PM, Ian Campbell wrote:
> > On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
> >> Pass checkpointed_stream from libxl to libxc.
> >> It won't affact legacy migration because legacy migration
> >> won't use this param.
> >>
> >> Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
> >> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> >> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> ---
> >>   tools/libxc/include/xenguest.h   |  9 ++++++---
> >>   tools/libxc/xc_domain_save.c     |  6 ++++--
> >>   tools/libxc/xc_nomigrate.c       |  3 ++-
> >>   tools/libxc/xc_sr_common.h       |  2 +-
> >>   tools/libxc/xc_sr_save.c         |  5 +++--
> >>   tools/libxl/libxl.c              |  2 ++
> >>   tools/libxl/libxl_dom_save.c     | 11 ++++++++---
> >>   tools/libxl/libxl_internal.h     |  1 +
> >>   tools/libxl/libxl_save_callout.c |  2 +-
> >>   tools/libxl/libxl_save_helper.c  |  3 ++-
> >>   10 files changed, 30 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/tools/libxc/include/xenguest.h 
> >> b/tools/libxc/include/xenguest.h
> >> index e95af54..6e24b6c 100644
> >> --- a/tools/libxc/include/xenguest.h
> >> +++ b/tools/libxc/include/xenguest.h
> >> @@ -30,7 +30,6 @@
> >>   #define XCFLAGS_HVM       (1 << 2)
> >>   #define XCFLAGS_STDVGA    (1 << 3)
> >>   #define XCFLAGS_CHECKPOINT_COMPRESS    (1 << 4)
> >> -#define XCFLAGS_CHECKPOINTED    (1 << 5)
> >>
> >>   #define X86_64_B_SIZE   64
> >>   #define X86_32_B_SIZE   32
> >> @@ -85,16 +84,20 @@ struct save_callbacks {
> >>    * @parm xch a handle to an open hypervisor interface
> >>    * @parm fd the file descriptor to save a domain to
> >>    * @parm dom the id of the domain
> >> + * @parm checkpointed_stream non-zero if the far end of the stream is 
> >> using
> >> + *       checkpointing
> >
> > Do (or will) specific non-zero values have any meaning to the libxc
> > layer? i.e. does it have any knowledge of COLO vs. Remus as the libxl
> > enum added in the last patch does?
> 
> Yes, libxc side should be aware of the type of checkpointed_stream (Remus
> or COLO).

In which case I'm afraid something somewhere needs to explicitly convert
from the LIBXL_ namespace to the libxc one (which would be better made
explicit with their own names not open coded numbers). See the handling
of libxl_tsc_mode or libxl_trigger for an example.

Alternatively you could add a comment to libxl_types.idl like
libxl_timer_mode and libxl_shutdown_reason have indicating that these
values must remain consistent with some underlying interface. I don't
much like that but it is tolerable I suppose.

> 
> I think it is better to document the non-zero values here?
> for example:
>       * @parm checkpointed_stream non-zero if the far end of the stream is 
> using
>       *                           checkpointing
>       *                           0 no checkpointed stream
>       *                           1 Remus
>       *                           2 COLO

I'd prefer named constants in the XC_ namespace.

Ian.


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