[CRIU] [PATCH 1/2] dump: Rollback in case if -R option passed

[Andrew Vagin]

On Thu, Oct 31, 2013 at 10:04:53PM +0400, Cyrill Gorcunov wrote:
> 
> It's been found that if -R (leave task running after checkpoint)
> option passed we don't unlock network, nether we clean service
> files (such as link remaps).
> 
> After a long discussion we choose the following path: if -R option
> is passed, it means a user is quite confident in what he is doing
> and consistency of the resources (file system) is achieved by
> a user himself with help of post-dump script. Also a user knows
> that the network will be unlocked and accept such case.
> 
> So here we check of -R being passed in command line and once
> checkpoint complete we unlock the network and cleanup remap
> links if they were created durig checkpoint.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-dump.c           | 27 +++++++++++++++++++++++----
>  files-reg.c         | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/files-reg.h |  3 +++
>  3 files changed, 73 insertions(+), 4 deletions(-)
> 

> diff --git a/cr-dump.c b/cr-dump.c
> index 1106e31..39e4555 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -1767,18 +1767,37 @@ err:
>  	}
>  
>  	/*
> -	 * If we've failed to do anything -- unlock all TCP sockets
> -	 * so that the connections can go on. But if we succeeded --
> -	 * don't, just close them silently.
> +	 * Dump is complete at this stage. To choose what
> +	 * to do next we need to consider the following
> +	 * scenarios
> +	 *
> +	 *  - error happened during checkpoint: just clean up
> +	 *    everything and continue execution of the dumpee;
> +	 *
> +	 *  - dump successed but post-dump script returned
> +	 *    some ret code: same as in previous scenario --
> +	 *    just clean up everything and continue execution,
> +	 *    we will return script ret code back to criu caller
> +	 *    and it's up to a caller what to do with running instance
> +	 *    of the dumpee -- either kill it, or continue running;
> +	 *
> +	 *  - dump successed but -R option passed, pointing that
> +	 *    we're asked to continue execution of the dumpee. It's
> +	 *    assumed that a user will use post-dump script to keep
> +	 *    consistency of the FS and other resources, we simply
> +	 *    start rollback procedure and cleanup everyhting.
>  	 */
> -	if (ret || post_dump_ret)
> +	if (ret || post_dump_ret || opts.final_state == TASK_ALIVE) {
>  		network_unlock();
> +		delete_link_remaps();

delete_link_remaps must be in another patch.

> +	}
>  	pstree_switch_state(root_item,
>  			    (ret || post_dump_ret) ?
>  			    TASK_ALIVE : opts.final_state);
>  	timing_stop(TIME_FROZEN);
>  	free_pstree(root_item);
>  	free_file_locks();
> +	free_link_remaps();
>  
>  	close_service_fd(CR_PROC_FD_OFF);
>  
> diff --git a/files-reg.c b/files-reg.c
> index 41050ef..4aa6c34 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -41,6 +41,15 @@ static LIST_HEAD(ghost_files);
>  static mutex_t *ghost_file_mutex;
>  
>  /*
> + * To rollback link remaps.
> + */
> +struct link_remap_rlb {
> +	struct list_head	list;
> +	char			*path;
> +};
> +static LIST_HEAD(link_remaps);
> +
> +/*
>   * This constant is selected without any calculations. Just do not
>   * want to pick up too big files with us in the image.
>   */
> @@ -310,11 +319,31 @@ dump_entry:
>  			&rpe, PB_REMAP_FPATH);
>  }
>  
> +static void __rollback_link_remaps(int do_unlink)
> +{
> +	struct link_remap_rlb *rlb, *tmp;
> +
> +	if (!opts.link_remap_ok)
> +		return;
> +
> +	list_for_each_entry_safe(rlb, tmp, &link_remaps, list) {
> +		list_del(&rlb->list);
> +		if (do_unlink)
> +			unlinkat(mntns_root, rlb->path, 0);
> +		xfree(rlb->path);
> +		xfree(rlb);
> +	}
> +}
> +
> +void delete_link_remaps(void) { __rollback_link_remaps(1); }
> +void free_link_remaps(void) { __rollback_link_remaps(0); }
> +
>  static int create_link_remap(char *path, int len, int lfd, u32 *idp)
>  {
>  	char link_name[PATH_MAX], *tmp;
>  	RegFileEntry rfe = REG_FILE_ENTRY__INIT;
>  	FownEntry fwn = FOWN_ENTRY__INIT;
> +	struct link_remap_rlb *rlb;
>  
>  	if (!opts.link_remap_ok) {
>  		pr_err("Can't create link remap for %s. "
> @@ -353,6 +382,24 @@ static int create_link_remap(char *path, int len, int lfd, u32 *idp)
>  		return -1;
>  	}
>  
> +	/*
> +	 * Remember the name to delete it if needed on error or
> +	 * rollback action. Note we don't expect that there will
> +	 * be a HUGE number of link remaps, so in a sake of speed
> +	 * we keep all data in memory.
> +	 */
> +	rlb = xmalloc(sizeof(*rlb));
> +	if (rlb)
> +		rlb->path = strdup(link_name);
> +
> +	if (!rlb || !rlb->path) {
> +		pr_perror("Can't register rollback for %s", path);
> +		xfree(rlb ? rlb->path : NULL);
> +		xfree(rlb);
> +		return -1;
> +	}
> +	list_add(&rlb->list, &link_remaps);
> +
>  	return pb_write_one(fdset_fd(glob_fdset, CR_FD_REG_FILES), &rfe, PB_REG_FILE);
>  }
>  
> diff --git a/include/files-reg.h b/include/files-reg.h
> index a930641..d3bd4f0 100644
> --- a/include/files-reg.h
> +++ b/include/files-reg.h
> @@ -38,4 +38,7 @@ extern void remap_put(struct file_remap *remap);
>  extern struct collect_image_info reg_file_cinfo;
>  extern struct collect_image_info remap_cinfo;
>  
> +extern void delete_link_remaps(void);
> +extern void free_link_remaps(void);
> +
>  #endif /* __CR_FILES_REG_H__ */