Re: [PATCH v7 08/18] tools/libxl: introduce libxl__domain_restore_device_model to load qemu state

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Feb 04, 2016 at 01:24:41PM +0800, Wen Congyang wrote:
> On 02/04/2016 03:40 AM, Wei Liu wrote:
> > On Fri, Jan 29, 2016 at 01:27:24PM +0800, Wen Congyang wrote:
> >> In normal migration, the qemu state is passed to qemu as a parameter.
> >> With COLO, secondary vm is running. So we will do the following steps
> >> at every checkpoint:
> >> 1. suspend both primary vm and secondary vm
> >> 2. sync the state
> >> 3. resume both primary vm and secondary vm
> >> Primary will send qemu's state in step2, and secondary's qemu should
> >> read it and restore the state before it is resumed. We can not pass
> >> the state to qemu as a parameter because secondary QEMU already started
> >> at this point, so we introduce libxl__domain_restore_device_model() to
> >> do it. This API MUST be called before resuming secondary vm.
> >>
> >> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> >> Cc: Anthony Perard <anthony.perard@citrix.com>
> >> ---
> >>  tools/libxl/libxl_dom_save.c | 20 ++++++++++++++++++++
> >>  tools/libxl/libxl_internal.h |  4 ++++
> >>  tools/libxl/libxl_qmp.c      | 10 ++++++++++
> >>  3 files changed, 34 insertions(+)
> >>
> >> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
> >> index cd2e7de..7383d2d 100644
> >> --- a/tools/libxl/libxl_dom_save.c
> >> +++ b/tools/libxl/libxl_dom_save.c
> >> @@ -518,6 +518,26 @@ int libxl__restore_emulator_xenstore_data(libxl__domain_create_state *dcs,
> >>      return rc;
> >>  }
> >>  
> >> +int libxl__domain_restore_device_model(libxl__gc *gc, uint32_t domid,
> >> +                                       const char *restore_file)
> >> +{
> >> +    int rc;
> >> +
> >> +    switch (libxl__device_model_version_running(gc, domid)) {
> >> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> >> +        /* Will never be supported. */
> >> +        rc = ERROR_INVAL;
> >> +        break;
> > 
> > I'm not entirely sure if this statement would be true. The function name
> > is generic enough to indicate this case should be supported.
> > 
> > However, this function is not used anywhere in this series, so I don't
> > know whether my comment makes sense.
> > 
> > One way of moving forward is to stick this patch to COLO series itself.
> > Let's skip this in this prerequisite series.
> 
> OK, I will put it in the COLO series itself.
> This API is used for COLO, and COLO requries the newest qemu with block replication.
> The block replication is still in the way. The tranditional qemu doesn't support
> block replication and it is hard to backport it.
> 

OK. I'm asking you to support qemu-trad in COLO -- definitely not.

What I was getting at was: this function has a very generic name, which
suggests it will be used to consolidate some code inside libxl. I was
confused because it didn't handle the qemu-trad path (after all,
restoring device model is something you can do with qemu-trad).

As discussed, this patch will be moved to COLO series, let's discuss
that when you post this again.

Wei.