[Devel] Re: [RFC v14-rc2][PATCH 10/29] actually use f_op in checkpoint code

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at cs.columbia.edu):
> From: Dave Hansen <dave at linux.vnet.ibm.com>
> 
> Right now, we assume all normal files and directories
> can be checkpointed.  However, as usual in the VFS, there
> are specialized places that will always need an ability
> to override these defaults.  We could do this completely
> in the checkpoint code, but that would bitrot quickly.
> 
> This adds a new 'file_operations' function for
> checkpointing a file.  I did this under the assumption
> that we should have a dirt-simple way to make something
> (un)checkpointable that fits in with current code.
> 
> As you can see in the ext[234] and /proc patches, all
> that we have to do to make something simple be
> supported is add a single "generic" f_op entry.
> 
> Signed-off-by: Dave Hansen <dave at linux.vnet.ibm.com>

Oooh, I see - for some reason I was convinced you'd put this patch
further back in the stack.

Acked-by: Serge Hallyn <serue at us.ibm.com>

(of course that is assymetric with restart)

> ---
>  checkpoint/ckpt_file.c |   31 +++++++++++++++----------------
>  include/linux/fs.h     |   11 +++++++++++
>  2 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/checkpoint/ckpt_file.c b/checkpoint/ckpt_file.c
> index 9c344c7..0fe68bf 100644
> --- a/checkpoint/ckpt_file.c
> +++ b/checkpoint/ckpt_file.c
> @@ -91,6 +91,11 @@ static int cr_write_file_generic(struct cr_ctx *ctx, struct file *file,
> 
>  	hh->fd_type = CR_FD_GENERIC;
> 
> +	/*
> +	 * FIXME: when we'll add support for unlinked files/dirs, we'll
> +	 * need to distinguish between unlinked filed and unlinked dirs.
> +	 */
> +
>  	ret = cr_write_obj(ctx, &h, hh);
>  	if (ret < 0)
>  		return ret;
> @@ -98,12 +103,16 @@ static int cr_write_file_generic(struct cr_ctx *ctx, struct file *file,
>  	return cr_write_fname(ctx, &file->f_path, &ctx->fs_mnt);
>  }
> 
> +int generic_file_checkpoint(struct cr_ctx *ctx, struct file *file,
> +			    struct cr_hdr_file *hh)
> +{
> +	return cr_write_file_generic(ctx, file, hh);
> +}
> +
>  /* cr_write_file - dump the state of a given file pointer */
>  static int cr_write_file(struct cr_ctx *ctx, struct file *file)
>  {
>  	struct cr_hdr_file *hh;
> -	struct dentry *dent = file->f_dentry;
> -	struct inode *inode = dent->d_inode;
>  	int ret;
> 
>  	hh = cr_hbuf_get(ctx, sizeof(*hh));
> @@ -116,21 +125,11 @@ static int cr_write_file(struct cr_ctx *ctx, struct file *file)
>  	hh->f_version = file->f_version;
>  	/* FIX: need also file->uid, file->gid, file->f_owner, etc */
> 
> -	/*
> -	 * FIXME: when we'll add support for unlinked files/dirs, we'll
> -	 * need to distinguish between unlinked filed and unlinked dirs.
> -	 */
> -	switch (inode->i_mode & S_IFMT) {
> -	case S_IFREG:
> -	case S_IFDIR:
> -		ret = cr_write_file_generic(ctx, file, hh);
> -		break;
> -	default:
> -		ret = -EBADF;
> -		break;
> -	}
> -	cr_hbuf_put(ctx, sizeof(*hh));
> +	ret = -EBADF;
> +	if (file->f_op->checkpoint)
> +		ret = file->f_op->checkpoint(ctx, file, hh);
> 
> +	cr_hbuf_put(ctx, sizeof(*hh));
>  	return ret;
>  }
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3bf5057..835ee9e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1296,6 +1296,14 @@ int generic_osync_inode(struct inode *, struct address_space *, int);
>  typedef int (*filldir_t)(void *, const char *, int, loff_t, u64, unsigned);
>  struct block_device_operations;
> 
> +#ifdef CONFIG_CHECKPOINT
> +struct cr_ctx;
> +struct cr_hdr_file;
> +int generic_file_checkpoint(struct cr_ctx *, struct file *, struct cr_hdr_file *);
> +#else
> +#define generic_file_checkpoint NULL
> +#endif
> +
>  /* These macros are for out of kernel modules to test that
>   * the kernel supports the unlocked_ioctl and compat_ioctl
>   * fields in struct file_operations. */
> @@ -1334,6 +1342,9 @@ struct file_operations {
>  	ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int);
>  	ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
>  	int (*setlease)(struct file *, long, struct file_lock **);
> +#ifdef CONFIG_CHECKPOINT
> +	int (*checkpoint)(struct cr_ctx *, struct file *file, struct cr_hdr_file *);
> +#endif
>  };
> 
>  struct inode_operations {
> -- 
> 1.5.4.3
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers