On Tue, Apr 21, 2020 at 11:04 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > On Tue, Apr 21, 2020 at 08:59:24PM +0200, Miklos Szeredi wrote: > > On Tue, Apr 21, 2020 at 8:41 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > > > 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); > > > > > > Fix it by passing O_TRUNC to underlying filesystem. > > > > > > Or clear ATTR_OPEN in ovl_setattr() > > > > Need to think about side effects of passing O_TRUNC down to underlying > > fs. Clearing ATTR_OPEN seems obviously safe, so as a quick fix I'd > > rather go with that for now. > > Found another interesting problem while I cleared ATTR_OPEN. VFS also > sets ATTR_FILE and attr->ia_file has ovl file pointer. ovl_setattr() does not > look at this attribute and passes it to underlying layer as it is. Fuse > thinks it got a valid file object and passes file handle to server and > server complains -EBADF. > > ext4/xfs don't seem to look at ATTR_FILE, so it did not create problems > so far. > > For now, I will simply reset ATTR_FILE, indicating to lower layers > don't use attr->ia_file. Right. The solution to that would be to replace attr->ia_file with the real file, but that can wait until we see any use case for that. Thanks, Miklos