[CRIU] OverlayFS Patch
Gabriel Guimaraes
gabriellimaguimaraes at gmail.com
Wed Aug 5 15:06:15 PDT 2015
Hmm, I'm using GDB to look directly at the value of p->fs_type within
fill_fdlink, when p refers to a file within a Docker container mounted with
Overlayfs and I get p->fs_type = 0xef53
2015-08-05 14:36 GMT-07:00 Filipe Brandenburger <filbranden at google.com>:
> More specifically, I'm looking at this code:
>
> /**
> * ovl_statfs
> * @sb: The overlayfs super block
> * @buf: The struct kstatfs to fill in with stats
> *
> * Get the filesystem statistics. As writes always target the upper layer
> * filesystem pass the statfs to the same filesystem.
> */
> static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
> {
> struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
> struct dentry *root_dentry = dentry->d_sb->s_root;
> struct path path;
> int err;
>
> ovl_path_upper(root_dentry, &path);
>
> err = vfs_statfs(&path, buf);
> if (!err) {
> buf->f_namelen = max(buf->f_namelen, ofs->lower_namelen);
> buf->f_type = OVERLAYFS_SUPER_MAGIC;
> }
>
> return err;
> }
>
>
> So, indeed, it's forwarding the call to the upper layer (since it's
> where writes happen) but before returning it's overwriting f_type with
> OVERLAYFS_SUPER_MAGIC which is the constant with the value I posted
> above...
>
> Cheers,
> Filipe
>
>
> On Wed, Aug 5, 2015 at 2:34 PM, Filipe Brandenburger
> <filbranden at google.com> wrote:
> > Can you try a directory instead of a file?
> >
> > Either a subdirectory or the root of the filesystem (where it's mounted.)
> >
> >
> > On Wed, Aug 5, 2015 at 2:31 PM, Gabriel Guimaraes
> > <gabriellimaguimaraes at gmail.com> wrote:
> >> Hi Filipe,
> >>
> >> I just checked the value of p->fs_type when referring to /tmp/file
> within a
> >> Docker container, using overlayFS. It reports
> >>
> >> p->fs_type = 0xef53 which I believe is the magic number of ext2/3/4
> >>
> >> 2015-08-05 14:25 GMT-07:00 Filipe Brandenburger <filbranden at google.com
> >:
> >>>
> >>> Hi Gabriel,
> >>>
> >>> Looking at the code, I believe it f_type in statfs() output will
> >>> return this value for overlayfs:
> >>>
> >>> fs/overlayfs/super.c:#define OVERLAYFS_SUPER_MAGIC 0x794c7630
> >>>
> >>> You can easily double check that with a test program running it
> >>> against a mounted overlayfs.
> >>>
> >>> Cheers,
> >>> Filipe
> >>>
> >>>
> >>> On Wed, Aug 5, 2015 at 2:19 PM, Gabriel Guimaraes
> >>> <gabriellimaguimaraes at gmail.com> wrote:
> >>> >
> >>> >
> >>> > 2015-08-05 13:36 GMT-07:00 Pavel Emelyanov <xemul at parallels.com>:
> >>> >>
> >>> >> On 08/05/2015 08:51 PM, Gabriel Guimaraes wrote:
> >>> >> > Hi Pavel,
> >>> >> >
> >>> >> > Thanks for the review.
> >>> >> >
> >>> >> > On Wed, Aug 5, 2015 at 4:00 AM, Pavel Emelyanov <
> xemul at parallels.com
> >>> >> > <mailto:xemul at parallels.com>> wrote:
> >>> >> >
> >>> >> > Gabriel,
> >>> >> >
> >>> >> > Please, find my comments inline.
> >>> >> >
> >>> >> > > diff --git a/crtools.c b/crtools.c
> >>> >> > > index 6af6080..521d0ff 100644
> >>> >> > > --- a/crtools.c
> >>> >> > > +++ b/crtools.c
> >>> >> > > @@ -235,6 +235,7 @@ int main(int argc, char *argv[], char
> >>> >> > *envp[])
> >>> >> > > { "enable-fs",
> >>> >> > required_argument,
> >>> >> > 0, 1065 },
> >>> >> > > { "enable-external-sharing", no_argument,
> >>> >> > 0, 1066 },
> >>> >> > > { "enable-external-masters", no_argument,
> >>> >> > 0, 1067 },
> >>> >> > > + { "overlayfs", no_argument,
> >>> >> > 0, 1068 },
> >>> >> >
> >>> >> > With AUFS workaround we ended up with auto-detection scheme --
> >>> >> > when
> >>> >> > meeting an AUFS mount the opts.aufs is set to true. Can we do
> the
> >>> >> > same with overlayfs?
> >>> >> >
> >>> >> >
> >>> >> > Technically we could always enable overlayfs by default, since we
> >>> >> > only
> >>> >> > do something if there
> >>> >> > is an overlayfs mounted entry in the mountinfo table. Should I
> enable
> >>> >> > it
> >>> >> > always?
> >>> >>
> >>> >> In the fill_fdlink() there's fd_parms structure with fs_type field
> >>> >> filled
> >>> >> from
> >>> >> statfs() syscall. If we check for this value being overlayfs magic,
> we
> >>> >> can
> >>> >> silently
> >>> >> imply the workaround is on and go calling fixup_overlayfs().
> >>> >>
> >>> > Per man statfs, I don't think Overlayfs has a magic number. OverlayFS
> >>> > just
> >>> > overlays two filesystems on top of each other. From statfs' point of
> >>> > view, a
> >>> > file's filesystem type is the original one, not OverlayFS.
> >>> >
> >>> > More information here:
> >>> > https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt
> >>> >>
> >>> >> > > +static struct mount_info *__lookup_overlayfs(struct
> mount_info
> >>> >> > *list, char *rpath,
> >>> >> > > + unsigned int
> >>> >> > st_dev,
> >>> >> > unsigned int st_ino,
> >>> >> > > + unsigned int
> >>> >> > mnt_id)
> >>> >> > > +{
> >>> >> > > + /*
> >>> >> > > + * Goes through all entries in the mountinfo table
> >>> >> > > + * looking for a mount point that contains the file
> >>> >> > specified
> >>> >> > > + * in rpath. Uses the device number st_dev and the
> inode
> >>> >> > number st_ino
> >>> >> > > + * to make sure the file is correct.
> >>> >> > > + */
> >>> >> > > + struct mount_info *mi_ret = NULL;
> >>> >> > > + struct mount_info *m;
> >>> >> > > +
> >>> >> > > + for (m = list; m != NULL; m = m->next) {
> >>> >> > > + /* Case 1: If mnt_id is correct, don't do
> >>> >> > anything
> >>> >> > */
> >>> >> > > + if (st_dev == m->s_dev && mnt_id == m->mnt_id)
> >>> >> > > + return NULL;
> >>> >> >
> >>> >> > Shouldn't this if () be under the next one, so that everything
> >>> >> > that
> >>> >> > happens
> >>> >> > here applies to overlayfs only?
> >>> >> >
> >>> >> >
> >>> >> > This if () is basically a check to see if the bug exists. We look
> for
> >>> >> > all instances of the device
> >>> >> > st_dev and see if any of them match our mnt_id. If that's not the
> >>> >> > case,
> >>> >> > the bug exists and we want
> >>> >> > to proceed to try to fix it.
> >>> >>
> >>> >> OK
> >>> >>
> >>> >> > I could have left this as a separate for loop to make this more
> >>> >> > clear,
> >>> >> > but this way we only use
> >>> >> > one for loop for both. I believe it's better if we separate it
> into
> >>> >> > two
> >>> >> > for loops since we prevent
> >>> >> > some unnecessary stating. I can also move this part to
> >>> >> > lookup_overlayfs,
> >>> >> > before dealing with
> >>> >> > namespaces. What do you think?
> >>> >>
> >>> >> Yup, makes sense to me.
> >>> >>
> >>> >
> >>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20150805/4c48724f/attachment.html>
More information about the CRIU
mailing list