Commit 15fd2ea9 authored by Vivek Goyal's avatar Vivek Goyal Committed by Miklos Szeredi

ovl: clear ATTR_OPEN from attr->ia_valid

As of now during open(), we don't pass bunch of flags to underlying
filesystem. O_TRUNC is one of these. Normally this is not a problem as VFS
calls ->setattr() with zero size and underlying filesystem sets file size
to 0.

But when overlayfs is running on top of virtiofs, it has an optimization
where it does not send setattr request to server if dectects that
truncation is part of open(O_TRUNC). It assumes that server already zeroed
file size as part of open(O_TRUNC).

fuse_do_setattr() {
        if (attr->ia_valid & ATTR_OPEN) {
                /*
                 * No need to send request to userspace, since actual
                 * truncation has already been done by OPEN.  But still
                 * need to truncate page cache.
                 */
        }
}

IOW, fuse expects O_TRUNC to be passed to it as part of open flags.

But currently overlayfs does not pass O_TRUNC to underlying filesystem
hence fuse/virtiofs breaks. Setup overlayfs on top of virtiofs and
following does not zero the file size of a file is either upper only or has
already been copied up.

fd = open(foo.txt, O_TRUNC | O_WRONLY);

There are two ways to fix this. Either pass O_TRUNC to underlying
filesystem or clear ATTR_OPEN from attr->ia_valid so that fuse ends up
sending a SETATTR request to server. Miklos is concerned that O_TRUNC might
have side affects so it is better to clear ATTR_OPEN for now. Hence this
patch clears ATTR_OPEN from attr->ia_valid.

I found this problem while running unionmount-testsuite. With this patch,
unionmount-testsuite passes with overlayfs on top of virtiofs.
Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
Fixes: bccece1e ("ovl: allow remote upper")
Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
parent e67f0216
...@@ -59,12 +59,23 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr) ...@@ -59,12 +59,23 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
attr->ia_valid &= ~ATTR_MODE; attr->ia_valid &= ~ATTR_MODE;
/* /*
* We might have to translate ovl file into underlying file * We might have to translate ovl file into real file object
* object once some use cases are there. For now, simply don't * once use cases emerge. For now, simply don't let underlying
* let underlying filesystem rely on attr->ia_file * filesystem rely on attr->ia_file
*/ */
attr->ia_valid &= ~ATTR_FILE; attr->ia_valid &= ~ATTR_FILE;
/*
* If open(O_TRUNC) is done, VFS calls ->setattr with ATTR_OPEN
* set. Overlayfs does not pass O_TRUNC flag to underlying
* filesystem during open -> do not pass ATTR_OPEN. This
* disables optimization in fuse which assumes open(O_TRUNC)
* already set file size to 0. But we never passed O_TRUNC to
* fuse. So by clearing ATTR_OPEN, fuse will be forced to send
* setattr request to server.
*/
attr->ia_valid &= ~ATTR_OPEN;
inode_lock(upperdentry->d_inode); inode_lock(upperdentry->d_inode);
old_cred = ovl_override_creds(dentry->d_sb); old_cred = ovl_override_creds(dentry->d_sb);
err = notify_change(upperdentry, attr, NULL); err = notify_change(upperdentry, attr, NULL);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment