Re: [C/R v20][PATCH 37/96] c/r: introduce new 'file_operations': ->checkpoint, ->collect()

[Nick Piggin <npiggin@suse.de>]

On Mon, Mar 22, 2010 at 03:16:35AM -0700, Matt Helsley wrote:
> On Mon, Mar 22, 2010 at 05:34:28PM +1100, Nick Piggin wrote:
> > On Thu, Mar 18, 2010 at 08:59:46PM -0400, Oren Laadan wrote:
> > Hmm, what does generic_file_checkpoint mean? A NULL checkpoint op means
> > that checkpointing is allowed, and no action is required? Shouldn't it
> > be an opt-in operation, where NULL means not allowed?
> 
> generic_file_checkpoint is for files that have a seek operation and can be
> backed up or restored with a simple copy.
> 
> A NULL checkpoint op means "not allowed" as you thought it should. What
> gave you the impression it was otherwise? Here's the relevant snippet
> from checkpoint/files.c:

Right I didn't check that far. It's just a bit strange to make it look
like filling in an aop function but it is actually still NULL.


> 
> /* checkpoint callback for file pointer */
> int checkpoint_file(struct ckpt_ctx *ctx, void *ptr)
> {
>         struct file *file = (struct file *) ptr;
>         int ret;
> 
>         if (!file->f_op || !file->f_op->checkpoint) {
>                 ckpt_err(ctx, -EBADF, "%(T)%(P)%(V)f_op lacks checkpoint\n",
>                                file, file->f_op);
>                 return -EBADF;
>         }
> 
> > Either way, I don't know if you need to have this #define, provided you
> > have sufficient documentation.
> 
> We need it (or a suitable replacement) to avoid adding #ifdef around
> assignments to the operation in every filesystem. It's used if
> CONFIG_CHECKPOINT is not defined.

If !CONFIG_CHECKPOINT, ->checkpoint should not exist and neither
should it's callers.