From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v7 08/18] tools/libxl: introduce libxl__domain_restore_device_model to load qemu state Date: Thu, 4 Feb 2016 09:41:54 +0000 Message-ID: <20160204094154.GI23178@citrix.com> References: <1454045254-3711-1-git-send-email-wency@cn.fujitsu.com> <1454045254-3711-9-git-send-email-wency@cn.fujitsu.com> <20160203194023.GX23178@citrix.com> <56B2E099.3030909@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <56B2E099.3030909@cn.fujitsu.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: Wen Congyang Cc: Lars Kurth , Changlong Xie , Wei Liu , Ian Campbell , Andrew Cooper , Jiang Yunhong , Ian Jackson , xen devel , Anthony Perard , Dong Eddie , Gui Jianfeng , Shriram Rajagopalan , Yang Hongyang List-Id: xen-devel@lists.xenproject.org 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 > >> Signed-off-by: Wen Congyang > >> Cc: Anthony Perard > >> --- > >> 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.