From mboxrd@z Thu Jan 1 00:00:00 1970 From: Szeredi Miklos Subject: Re: [PATCH 01/34] VFS: Make clone_mnt() and copy_tree() return error codes Date: Fri, 1 Oct 2010 11:12:48 +0200 Message-ID: References: <1284675145-4391-1-git-send-email-vaurora@redhat.com> <1284675145-4391-2-git-send-email-vaurora@redhat.com> <20100930214123.GA490@shell> <20100930214418.GB490@shell> <20101001003342.GI9247@ram-laptop> <20101001015857.GA5003@ram-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: mszeredi2@gmail.com, Valerie Aurora , viro@zeniv.linux.org.uk, hch@infradead.org, agruen@suse.de, npiggin@kernel.dk, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: Ram Pai Return-path: Received: from mail-qw0-f46.google.com ([209.85.216.46]:39164 "EHLO mail-qw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752499Ab0JAJMt convert rfc822-to-8bit (ORCPT ); Fri, 1 Oct 2010 05:12:49 -0400 In-Reply-To: <20101001015857.GA5003@ram-laptop> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Oct 1, 2010 at 3:58 AM, Ram Pai wrote: > > > > > @@ -1212,11 +1216,12 @@ struct vfsmount *copy_tree(struct vfs= mount *mnt, struct dentry *dentry, > > > > > =A0 =A0 =A0 =A0 struct path path; > > > > > > > > > > =A0 =A0 =A0 =A0 if (!(flag & CL_COPY_ALL) && IS_MNT_UNBINDABL= E(mnt)) > > > > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL; > > > > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return ERR_PTR(-EINVAL); > > > > > > Ram, do you remember how this worked? > > > > Oops. That should be a OR condition. there is one other check in th= at > > function that should also be a OR condition. > > I may be wrong here. Can't exactly recollect what CL_COPY_ALL flag me= ans. Al Viro > might remember? =A0If CL_COPY_ALL means, to clone everything irrespec= tive of any other > flags, then the above code seems right. CL_COPY_ALL means clone the mount despite MNT_UNBINDABLE. It is used for cloning the whole namespace and for collect_mounts(), both of which ignore MNT_UNBINDABLE. Of the two remaining callers of copy_tree() do_loopback already checks MNT_UNBINDABLE on the root of the tree to be copied. So that leaves the one in pnode.c. That one will be called when attaching a new mount or mount tree. If the root of that tree is unbindable then the propagation will fail with -ENOMEM which is wrong, it should simply skip the whole tree and not try to propagate. Calls which result in propagation are do_loopback, do_move_mount and do_add_mount. Of this do_loopback and do_move_mount already check for MNT_UNBINDABLE, do_add_mount doesn't check, but should probably just mask out MNT_UNBINDABLE. So in the end that check in copy_tree() should never actually trigger and can be turned into a WARN_ON Additionally the propagation code should perhaps be more defensive and skip MNT_UNBINDABLE source mounts. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html