Dave Hansen [[email protected]] wrote:
| 
| We have set up sane defaults for how filesystems should
| be checkpointed.  However, as usual in the VFS, there
| are specialized places that will always need an ability
| to override these defaults.
| 
| 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 /dev/null patch in a second, all
| that we have to do to make something like /dev/null
| supported is add a single "generic" f_op entry.
| 
| Signed-off-by: Dave Hansen <[email protected]>
| ---
| 
|  linux-2.6.git-dave/checkpoint/ckpt_file.c     |   77 
++++++++++++++++----------
|  linux-2.6.git-dave/include/linux/checkpoint.h |    6 ++
|  linux-2.6.git-dave/include/linux/fs.h         |    4 +
|  3 files changed, 60 insertions(+), 27 deletions(-)
| 
| diff -puN checkpoint/ckpt_file.c~f_op-for-checkpointability 
checkpoint/ckpt_file.c
| --- linux-2.6.git/checkpoint/ckpt_file.c~f_op-for-checkpointability   
2009-02-27 12:07:39.000000000 -0800
| +++ linux-2.6.git-dave/checkpoint/ckpt_file.c 2009-02-27 12:07:39.000000000 
-0800
| @@ -104,56 +104,79 @@ int cr_explain_file(struct file *file, c
|       return 0;
|  }
| 
| -int cr_file_supported(struct file *file)
| +typedef int (do_checkpoint_t)(struct file *, struct cr_ctx *,
| +                           struct cr_hdr_fd *);
| +
| +int generic_file_checkpoint(struct file *file, struct cr_ctx *ctx,
| +                      struct cr_hdr_fd *hh)
| +{
| +     /*
| +      * A NULL hh means to make a trial run not
| +      * actually writing data.  Just determine
| +      * if the file is checkpointable.
| +      */
| +     if (!hh)
| +             return 0;
| +
| +     hh->f_flags = file->f_flags;
| +     hh->f_mode = file->f_mode;
| +     hh->f_pos = file->f_pos;
| +     hh->f_version = file->f_version;
| +     /* FIX: need also file->uid, file->gid, file->f_owner, etc */
| +
| +     return 0;
| +}
| +
| +do_checkpoint_t *cr_file_get_func(struct file *file)
|  {

Do we really need this helper ? IOW do callers need this function pointer
itself ? Or can we have a more generic helper that callers can use both
check if checkpoint is possible (pass NULL in ctx and hh) and to actually
checkpoint. Something like:

        int cr_file_checkpoint(file, ctx, hh)
        {
                int rc = -1;

                if (!cr_fs_checkpointable(fstype))
                        return rc;
                
                if (!cr_file_checkpointable(file))
                        return rc;

                if (special_file(file))
                        return rc;

                op = file->f_op->checkpoint;
                if (!op)
                        op = generic_file_checkpoint;

                return (*op)(file, ctx, hh);
        }



|       struct inode *inode = file->f_dentry->d_inode;
|       struct file_system_type *fs_type = inode->i_sb->s_type;
| 
| -     if (fs_is_cr_able(fs_type))
| -             return 0;
| +     if (file->f_op->checkpoint)
| +             return file->f_op->checkpoint;
| +
| +     if (!fs_is_cr_able(fs_type))
| +             return NULL;
| 
|       if (special_file(inode->i_mode))
| -             return 0;
| +             return NULL;
| 
| -     return 1;
| +     return generic_file_checkpoint;
| +}
| +
| +int cr_file_supported(struct file *file)
| +{
| +     do_checkpoint_t *func = cr_file_get_func(file);
| +
| +     if (func)
| +             return !func(file, NULL, NULL);
| +
| +     return 0;
|  }
| 
|  /* cr_write_fd_data - dump the state of a given file pointer */
|  static int cr_write_fd_data(struct cr_ctx *ctx, struct file *file, int 
parent)
|  {
| +     do_checkpoint_t *ckpt_func;
|       struct cr_hdr h;
|       struct cr_hdr_fd *hh = cr_hbuf_get(ctx, sizeof(*hh));
| -     struct dentry *dent = file->f_dentry;
| -     struct inode *inode = dent->d_inode;
| -     enum fd_type fd_type;
|       int ret;
| 
|       h.type = CR_HDR_FD_DATA;
|       h.len = sizeof(*hh);
|       h.parent = parent;
| 
| -     hh->f_flags = file->f_flags;
| -     hh->f_mode = file->f_mode;
| -     hh->f_pos = file->f_pos;
| -     hh->f_version = file->f_version;
| -     /* FIX: need also file->uid, file->gid, file->f_owner, etc */
| -
| -     switch (inode->i_mode & S_IFMT) {
| -     case S_IFREG:
| -             fd_type = CR_FD_FILE;
| -             break;
| -     case S_IFDIR:
| -             fd_type = CR_FD_DIR;
| -             break;
| -     default:
| -             cr_hbuf_put(ctx, sizeof(*hh));
| -             return -EBADF;
| -     }
| +     ckpt_func = cr_file_get_func(file);
| +     ret = -EBADF;
| +     if (!ckpt_func)
| +             goto out;
| 
| -     /* FIX: check if the file/dir/link is unlinked */
| -     hh->fd_type = fd_type;
| +     ret = ckpt_func(file, ctx, hh);
| +     if (ret)
| +             goto out;

So we can combine these two steps into just one ?

        ret = -EBADF;
        hh->fd_type = fd_type;
        if (cr_file_checkpoint(file, ctx, hh))
                goto out;

| 
|       ret = cr_write_obj(ctx, &h, hh);
| +out:
|       cr_hbuf_put(ctx, sizeof(*hh));
|       if (ret < 0)
|               return ret;

Sukadev
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel

Reply via email to