From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yang Hongyang Subject: Re: [PATCH v3 COLOPre 12/26] tools/libxl: Update libxl_domain_unpause() to support qemu-xen Date: Wed, 1 Jul 2015 21:38:44 +0800 Message-ID: <5593ED64.40806@cn.fujitsu.com> References: <1435213552-10556-1-git-send-email-yanghy@cn.fujitsu.com> <1435213552-10556-13-git-send-email-yanghy@cn.fujitsu.com> <1435658447.21469.63.camel@citrix.com> <55934C05.5000806@cn.fujitsu.com> <1435747137.21469.249.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1435747137.21469.249.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.liu2@citrix.com, 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, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 07/01/2015 06:38 PM, Ian Campbell wrote: > On Wed, 2015-07-01 at 10:10 +0800, Yang Hongyang wrote: >> >> On 06/30/2015 06:00 PM, Ian Campbell wrote: >>> On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: >>>> Currently, libxl__domain_unpause() only supports >>>> qemu-xen-traditional. Update it to support qemu-xen. >>>> We use libxl__domain_resume_device_model to unpause guest dm. >>> >>> There appears to be at least two significant semantic changes to the >>> libxl__domain_resume_device_model function (namely it now deals >>> internally with stubdom and checks the current status before doing >>> anything). What is the impact on the existing callers of those changes? >> >> This change was suggested by Wei on last round(v2), he thought it is an >> enhancement to the existing callers. > > I don't think that contradicts what I've said. If multiple improvements > are needed to make libxl__domain_resume_device_model suitable for your > uses then there should be several patches. This will also give you an > opportunity to explain in each commit message why there is no impact (or > a positive impact) on the existing callers. Yes, for this part, I agree with you, will separate it in the next version! thank you! > >> >>> >>> I think those changes should be two separate preparatory patches. >>> >>> >>>> >>>> Signed-off-by: Yang Hongyang >>>> --- >>>> tools/libxl/libxl.c | 15 +++++---------- >>>> tools/libxl/libxl_dom_suspend.c | 15 ++++++++++++--- >>>> tools/libxl/libxl_internal.h | 1 + >>>> 3 files changed, 18 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >>>> index 2ca59ea..59e2dfe 100644 >>>> --- a/tools/libxl/libxl.c >>>> +++ b/tools/libxl/libxl.c >>>> @@ -938,8 +938,6 @@ out: >>>> int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid) >>>> { >>>> GC_INIT(ctx); >>>> - char *path; >>>> - char *state; >>>> int ret, rc = 0; >>>> >>>> libxl_domain_type type = libxl__domain_type(gc, domid); >>>> @@ -949,14 +947,11 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid) >>>> } >>>> >>>> if (type == LIBXL_DOMAIN_TYPE_HVM) { >>>> - 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"); >>>> - libxl__wait_for_device_model_deprecated(gc, domid, "running", >>>> - NULL, NULL, NULL); >>>> + rc = libxl__domain_resume_device_model(gc, domid); >>>> + if (rc < 0) { >>>> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to unpause device model " >>>> + "for domain %u:%d", domid, rc); >>>> + goto out; >>>> } >>>> } >>>> ret = xc_domain_unpause(ctx->xch, domid); >>>> diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c >>>> index 1c486c4..3edbd2e 100644 >>>> --- a/tools/libxl/libxl_dom_suspend.c >>>> +++ b/tools/libxl/libxl_dom_suspend.c >>>> @@ -376,13 +376,22 @@ static void domain_suspend_callback_common_done(libxl__egc *egc, >>>> >>>> /*======================= Domain resume ========================*/ >>>> >>>> -static int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid) >>>> +int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid) >>>> { >>>> + char *path; >>>> + char *state; >>>> >>>> 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"); >>>> + libxl__wait_for_device_model_deprecated(gc, domid, "running", >>>> + NULL, NULL, NULL); >>>> + } >>>> break; >>>> } >>>> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN: >>>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h >>>> index 0adc9b4..6960280 100644 >>>> --- a/tools/libxl/libxl_internal.h >>>> +++ b/tools/libxl/libxl_internal.h >>>> @@ -3326,6 +3326,7 @@ static inline bool libxl__save_helper_inuse(const libxl__save_helper_state *shs) >>>> /* Each time the dm needs to be saved, we must call suspend and then save */ >>>> _hidden int libxl__domain_suspend_device_model(libxl__gc *gc, >>>> libxl__domain_suspend_state *dsps); >>>> +_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid); >>>> _hidden void libxl__domain_save_device_model(libxl__egc *egc, >>>> libxl__domain_save_state *dss, >>>> libxl__save_device_model_cb *callback); >>> >>> >>> . >>> >> > > > . > -- Thanks, Yang.