From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Helsley Subject: Re: [RFC][PATCH 04/36] btrfs: Add checkpoint file operations Date: Mon, 4 May 2009 18:06:09 -0700 Message-ID: <20090505010609.GR11734@us.ibm.com> References: <63ff4e88102e10e86bffefd5e4445ab433ed51c0.1241462097.git.matthltc@us.ibm.com> <20090504214146.GA31338@us.ibm.com> <49FF69B4.3010700@cs.columbia.edu> <20090504222513.GA32323@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20090504222513.GA32323-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: "Serge E. Hallyn" Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Dave Hansen List-Id: containers.vger.kernel.org On Mon, May 04, 2009 at 05:25:13PM -0500, Serge E. Hallyn wrote: > Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org): > > > > Serge E. Hallyn wrote: > > > Quoting Matt Helsley (matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org): > > > > > > ... > > > > > >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > > >> index a7acfe6..f16be9d 100644 > > >> --- a/fs/btrfs/super.c > > >> +++ b/fs/btrfs/super.c > > >> @@ -686,6 +686,7 @@ static const struct file_operations btrfs_ctl_fops = { > > >> .unlocked_ioctl = btrfs_control_ioctl, > > >> .compat_ioctl = btrfs_control_ioctl, > > >> .owner = THIS_MODULE, > > >> + .checkpoint = generic_file_checkpoint, > > >> }; > > > > > > Do we really want this one? > > > > We need to checkpoint the open file regardless of whether the underlying > > filesystem has snapshot capabilities. > > But maybe we need to refuse a checkpoint if a task is talking to the > underlying btrfs filesystem? Good catch. The ctl file is a device node which, as best I can tell, is empty. So the only obvious concern are the unlocked_ioctl()s. I expect these ioctls() are "special" so generic_file_checkpoint() is probably insufficient here. v2 of the btrfs checkpoint op patch, which removes this hunk, is below. A non-generic handler which blocks or prevents ioctl() during checkpoint would seem to be required. Definitely a topic worthy of discussion on linux-fsdevel if /when we add a .checkpoint op for this. As best I can tell we don't have anything to handle the generic ioctl() case either. Am I wrong? Cheers, -Matt Helsley --- Add the checkpoint operation for btrfs files and directories. Signed-off-by: Matt Helsley Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org --- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 1 + fs/btrfs/super.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 482f8db..23d5c72 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1334,4 +1334,5 @@ struct file_operations btrfs_file_operations = { #ifdef CONFIG_COMPAT .compat_ioctl = btrfs_ioctl, #endif + .checkpoint = generic_file_checkpoint, }; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 65219f6..583d57e 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5158,6 +5158,7 @@ static struct file_operations btrfs_dir_file_operations = { #endif .release = btrfs_release_file, .fsync = btrfs_sync_file, + .checkpoint = generic_file_checkpoint, }; static struct extent_io_ops btrfs_extent_io_ops = {