From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v4 --for 4.6 COLOPre 15/25] tools/libxl: check QEMU state before resume dm Date: Wed, 15 Jul 2015 14:48:53 +0100 Message-ID: <1436968133.32371.76.camel@citrix.com> References: <1436946351-21118-1-git-send-email-yanghy@cn.fujitsu.com> <1436946351-21118-16-git-send-email-yanghy@cn.fujitsu.com> <1436964497.32371.44.camel@citrix.com> <1436964852.32371.49.camel@citrix.com> <20150715130056.GG29717@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150715130056.GG29717@zion.uk.xensource.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: Wei Liu Cc: wency@cn.fujitsu.com, andrew.cooper3@citrix.com, yunhong.jiang@intel.com, eddie.dong@intel.com, xen-devel@lists.xen.org, guijianfeng@cn.fujitsu.com, rshriram@cs.ubc.ca, Yang Hongyang , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Wed, 2015-07-15 at 14:00 +0100, Wei Liu wrote: > On Wed, Jul 15, 2015 at 01:54:12PM +0100, Ian Campbell wrote: > > On Wed, 2015-07-15 at 13:48 +0100, Ian Campbell wrote: > > > > switch (libxl__device_model_version_running(gc, domid)) { > > > > case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: { > > > > - libxl__qemu_traditional_cmd(gc, domid, "continue"); > > > > - libxl__wait_for_device_model_deprecated(gc, domid, "running", NULL, NULL, NULL); > > > > + uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid); > > > > + > > > > + path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state"); > > > > + state = libxl__xs_read(gc, XBT_NULL, path); > > > > + if (state != NULL && !strcmp(state, "paused")) { > > > > + libxl__qemu_traditional_cmd(gc, domid, "continue"); > > > > > > Please can you explain the apparent discrepancy between the use of > > > dm_domid and domid here? > > > > I see from the next patch that this pattern came from the existing > > libxl_domain_unpause, which hopes to use this helper in the future. > > > > Looking at git annotate: > > 83cc69fa (Ian Jackson 2012-06-28 18:43:28 +0100 1045) if (type == LIBXL_DOMAIN_TYPE_HVM) { > > 1fc3aeb3 ( Wei Liu 2015-04-09 19:49:25 +0100 1046) uint32_t dm_domid = libxl_get_stubdom_id(ctx, domid); > > 1fc3aeb3 ( Wei Liu 2015-04-09 19:49:25 +0100 1047) > > 1fc3aeb3 ( Wei Liu 2015-04-09 19:49:25 +0100 1048) path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state"); > > bdf07e8e (Ian Jackson 2011-12-12 17:48:42 +0000 1049) state = libxl__xs_read(gc, XBT_NULL, path); > > d1c7c3ef (Keir Fraser 2009-11-30 10:53:39 +0000 1050) if (state != NULL && !strcmp(state, "paused")) { > > 0cb90b31 (Shriram Rajagopalan 2012-02-09 18:07:48 +0000 1051) libxl__qemu_traditional_cmd(gc, domid, "continue"); > > 47cb2273 (Ian Jackson 2013-10-14 17:26:01 +0100 1052) libxl__wait_for_device_model_deprecated(gc, domid, "running", > > 3b6eaa3e (Ian Campbell 2011-05-24 15:57:24 +0100 1053) NULL, NULL, NULL); > > d1c7c3ef (Keir Fraser 2009-11-30 10:53:39 +0000 1054) } > > > > It seems this came from Wei in 1fc3aeb3aa26 "libxl: use new QEMU > > xenstore protocol". I suspect it was a mistake. Wei? > > > > No, it's not. > > libxl__qemu_traditional_cmd accepts domid and then it calls > libxl_get_stubdom_id to extract dm_domid. How... exciting. Some sort of helper to get the DM state would help to hide this sort of wrinkle. Anyway, I shall go see if this means I can ack the COLO patches which moved this code. Ian.