Re: [Xen-devel] [RFC PATCH COLO v5 02/29] Refactor domain_suspend_callback_common()

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 01, 2015 at 02:41:38PM +0800, Yang Hongyang wrote:
> From: Wen Congyang <wency@xxxxxxxxxxxxxx>
> 
> libxl__domain_suspend() is to save the guest. I think
> we should call it libxl__domain_save(), but I don't
> rename it.
> 

FWIW this is not public API so we have certain degree of liberty to
rename it if the new name is deemed more appropriate.

> Secondary vm is running in colo mode. So we will do
> the following things again and again:
> 1. suspend both primay vm and secondary vm
> 2. sync the state
> 3. resume both primary vm and secondary vm
> To suspend secondary vm, we need an independent API to
> suspend vm.
> 
[...]
>  
> +/*
> + * libxl__domain_suspend_state is for saving guest, not
> + * for suspending guest. We need to an independent API
> + * to suspend guest only.
> + */
> +struct libxl__domain_suspend_state2 {
> +    /* set by caller of libxl__domain_suspend2 */
> +    libxl__ao *ao;
> +
> +    uint32_t domid;
> +    libxl__ev_evtchn guest_evtchn;;
> +    int guest_evtchn_lockfd;
> +    int hvm;
> +    const char *dm_savefile;
> +    void (*callback_common_done)(libxl__egc*,
> +                                 libxl__domain_suspend_state2*, int ok);
> +    int save_dm;
> +    int guest_responded;
> +    libxl__xswait_state pvcontrol;
> +    libxl__ev_xswatch guest_watch;
> +    libxl__ev_time guest_timeout;
> +};
> +
>  struct libxl__domain_suspend_state {
>      /* set by caller of libxl__domain_suspend */
>      libxl__ao *ao;
> @@ -2827,22 +2851,14 @@ struct libxl__domain_suspend_state {
>      int debug;
>      const libxl_domain_remus_info *remus;
>      /* private */
> -    libxl__ev_evtchn guest_evtchn;
> -    int guest_evtchn_lockfd;
> +    libxl__domain_suspend_state2 dss2;
>      int hvm;
>      int xcflags;
> -    int guest_responded;
> -    libxl__xswait_state pvcontrol;
> -    libxl__ev_xswatch guest_watch;
> -    libxl__ev_time guest_timeout;
> -    const char *dm_savefile;
>      libxl__remus_devices_state rds;
>      libxl__ev_time checkpoint_timeout; /* used for Remus checkpoint */
>      int interval; /* checkpoint interval (for Remus) */
>      libxl__save_helper_state shs;
>      libxl__logdirty_switch logdirty;
> -    void (*callback_common_done)(libxl__egc*,
> -                                 struct libxl__domain_suspend_state*, int 
> ok);
>      /* private for libxl__domain_save_device_model */
>      libxl__save_device_model_cb *save_dm_callback;
>      libxl__datacopier_state save_dm_datacopier;

You moved dm_savefile to new struct but not these two fields, why?
Since your suspend2 function is just a wrapper around
domain_suspend_callback_common, it doesn't seem to need access to
dm_savefile.

Wei.

> @@ -3116,6 +3132,9 @@ struct libxl__domain_create_state {
>  
>  /*----- Domain suspend (save) functions -----*/
>  
> +/* calls dss2->callback_common_done when done */
> +_hidden void libxl__domain_suspend2(libxl__egc *egc,
> +                                    libxl__domain_suspend_state2 *dss2);
>  /* calls dss->callback when done */
>  _hidden void libxl__domain_suspend(libxl__egc *egc,
>                                     libxl__domain_suspend_state *dss);
> @@ -3155,7 +3174,7 @@ _hidden void libxl__xc_domain_restore_done(libxl__egc 
> *egc, void *dcs_void,
>  
>  /* 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 *dss);
> +                                           libxl__domain_suspend_state2 
> *dss2);
>  _hidden void libxl__domain_save_device_model(libxl__egc *egc,
>                                       libxl__domain_suspend_state *dss,
>                                       libxl__save_device_model_cb *callback);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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