Sukadev Bhattiprolu wrote:
> A minor comment and a nit.
>
> Oren Laadan [orenl@xxxxxxxxxxxxxxx] wrote:
> | From d832bfba9a50789fbfadf8486fbdfbd8b498a9ea Mon Sep 17 00:00:00 2001
> | From: Dave Hansen <dave@xxxxxxxxxxxxxxxxxx>
> | Date: Fri, 27 Mar 2009 12:50:47 -0700
> | Subject: [PATCH 10/29] actually use f_op in checkpoint code
> |
> |
> | 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.
[...]
> | /* 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);
>
> Minor: not bisect safe for checkpoint - fwiw, with previous patch we could
> checkpoint a process with open file, but with this change, we can't ? How
> about merge patches 10 and 11 ?
It will will remain not-bisect-safe for anyone who is using other than
ext2/3/4 file systems.
I think it's a good way of separating the concept and it's implementation.
[...]
Oren.
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
