[PATCH 0/4] virtio-fs: add readdir support

10 views
Skip to first unread message

Fotis Xenakis

unread,
Jul 19, 2020, 5:58:25 PM7/19/20
to osv...@googlegroups.com, Fotis Xenakis
This set of patches adds readdir() support for virtio-fs: the first
patch contains minor fixes, while the second and third lay the ground
work for the fourth which actually implements the readdir functionality.

While working on this, I noticed that the readdir implementations for
other OSv filesystems (e.g. ROFS, devfs, pseudofs) don't set the d_off
field of the dirent struct they return (this patch _does_ set it). Is
there a reason behind this pattern? If so, I will be glad to align this
implementation with the rest.

As always, any feedback is more than welcome.

Fotis Xenakis (4):
virtio-fs: fix allocation failure condition
virtio-fs: expose FUSE reply length
virtio-fs: use FUSE_OPENDIR and FUSE_RELEASEDIR
virtio-fs: implement readdir

fs/virtiofs/virtiofs_dax.cc | 4 +-
fs/virtiofs/virtiofs_i.hh | 8 ++-
fs/virtiofs/virtiofs_vfsops.cc | 16 +++--
fs/virtiofs/virtiofs_vnops.cc | 103 ++++++++++++++++++++++++++++-----
4 files changed, 106 insertions(+), 25 deletions(-)

--
2.27.0

Fotis Xenakis

unread,
Jul 19, 2020, 5:59:52 PM7/19/20
to osv...@googlegroups.com, Fotis Xenakis
Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---
fs/virtiofs/virtiofs_vnops.cc | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc
index 51be3ee4..10571a8d 100644
--- a/fs/virtiofs/virtiofs_vnops.cc
+++ b/fs/virtiofs/virtiofs_vnops.cc
@@ -44,7 +44,7 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
}

if (!S_ISDIR(inode->attr.mode)) {
- kprintf("[virtiofs] inode:%lld, ABORTED lookup of %s because not a "
+ kprintf("[virtiofs] inode %lld, ABORTED lookup of %s because not a "
"directory\n", inode->nodeid, name);
return ENOTDIR;
}
@@ -64,7 +64,7 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
inode->nodeid, in_args.get(), in_args_len, out_args.get(),
sizeof(*out_args));
if (error) {
- kprintf("[virtiofs] inode:%lld, lookup failed to find %s\n",
+ kprintf("[virtiofs] inode %lld, lookup failed to find %s\n",
inode->nodeid, name);
// TODO: Implement proper error handling by sending FUSE_FORGET
return error;
@@ -73,7 +73,7 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
struct vnode* vp;
// TODO OPT: Should we even use the cache? (consult spec on metadata)
if (vget(vnode->v_mount, out_args->nodeid, &vp) == 1) {
- virtiofs_debug("lookup found vp in cache!\n");
+ virtiofs_debug("lookup found vp in cache\n");
*vpp = vp;
return 0;
}
@@ -83,7 +83,7 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
return ENOMEM;
}
new_inode->nodeid = out_args->nodeid;
- virtiofs_debug("inode %lld, lookup found inode %lld for %s!\n",
+ virtiofs_debug("inode %lld, lookup found inode %lld for %s\n",
inode->nodeid, new_inode->nodeid, name);
memcpy(&new_inode->attr, &out_args->attr, sizeof(out_args->attr));

@@ -196,7 +196,7 @@ static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle,
std::unique_ptr<void, std::function<void(void*)>> buf {
memory::alloc_phys_contiguous_aligned(read_amt,
alignof(std::max_align_t)), memory::free_phys_contiguous_aligned };
- if (!in_args | !buf) {
+ if (!in_args || !buf) {
return ENOMEM;
}
in_args->fh = file_handle;
--
2.27.0

Fotis Xenakis

unread,
Jul 19, 2020, 6:00:54 PM7/19/20
to osv...@googlegroups.com, Fotis Xenakis
Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---
fs/virtiofs/virtiofs_dax.cc | 4 ++--
fs/virtiofs/virtiofs_i.hh | 8 +++++---
fs/virtiofs/virtiofs_vfsops.cc | 16 ++++++++++------
fs/virtiofs/virtiofs_vnops.cc | 11 ++++++-----
4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/fs/virtiofs/virtiofs_dax.cc b/fs/virtiofs/virtiofs_dax.cc
index 8e612eb5..38ccdc39 100644
--- a/fs/virtiofs/virtiofs_dax.cc
+++ b/fs/virtiofs/virtiofs_dax.cc
@@ -194,7 +194,7 @@ int dax_manager::map_ll(uint64_t nodeid, uint64_t file_handle, chunk nchunks,
"moffset=%lld)\n", nodeid, in_args->foffset, in_args->len,
in_args->moffset);
auto error = fuse_req_send_and_receive_reply(&_drv, FUSE_SETUPMAPPING,
- nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
+ nodeid, in_args.get(), sizeof(*in_args), nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, mapping setup failed\n", nodeid);
return error;
@@ -230,7 +230,7 @@ int dax_manager::unmap_ll(chunk nchunks, chunk mstart)
virtiofs_debug("inode %lld, removing mapping (moffset=%lld, len=%lld)\n",
nodeid, r_one->moffset, r_one->len);
auto error = fuse_req_send_and_receive_reply(&_drv, FUSE_REMOVEMAPPING,
- nodeid, in_args.get(), in_args_size, nullptr, 0);
+ nodeid, in_args.get(), in_args_size, nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, mapping removal failed\n", nodeid);
return error;
diff --git a/fs/virtiofs/virtiofs_i.hh b/fs/virtiofs/virtiofs_i.hh
index e879546d..1d1b04bc 100644
--- a/fs/virtiofs/virtiofs_i.hh
+++ b/fs/virtiofs/virtiofs_i.hh
@@ -8,10 +8,12 @@
#ifndef VIRTIOFS_IO_H
#define VIRTIOFS_IO_H

+#include <utility>
+
#include "drivers/virtio-fs.hh"

-int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
- uint64_t nodeid, void* input_args_data, size_t input_args_size,
- void* output_args_data, size_t output_args_size);
+std::pair<size_t, int> fuse_req_send_and_receive_reply(virtio::fs* drv,
+ uint32_t opcode, uint64_t nodeid, void* input_args_data,
+ size_t input_args_size, void* output_args_data, size_t output_args_size);

#endif
diff --git a/fs/virtiofs/virtiofs_vfsops.cc b/fs/virtiofs/virtiofs_vfsops.cc
index be4a32e6..64855b73 100644
--- a/fs/virtiofs/virtiofs_vfsops.cc
+++ b/fs/virtiofs/virtiofs_vfsops.cc
@@ -10,6 +10,7 @@
#include <mutex>
#include <new>
#include <sys/types.h>
+#include <utility>

#include <api/assert.h>
#include <osv/debug.h>
@@ -32,14 +33,14 @@ static struct {
mutex lock;
} dax_managers;

-int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
- uint64_t nodeid, void* input_args_data, size_t input_args_size,
- void* output_args_data, size_t output_args_size)
+std::pair<size_t, int> fuse_req_send_and_receive_reply(virtio::fs* drv,
+ uint32_t opcode, uint64_t nodeid, void* input_args_data,
+ size_t input_args_size, void* output_args_data, size_t output_args_size)
{
std::unique_ptr<fuse_request> req {
new (std::nothrow) fuse_request(sched::thread::current())};
if (!req) {
- return ENOMEM;
+ return std::make_pair(0, ENOMEM);
}
req->in_header.len = sizeof(req->in_header) + input_args_size;
req->in_header.opcode = opcode;
@@ -57,9 +58,11 @@ int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
drv->make_request(*req);
req->wait();

+ // return the length of the response's payload
+ size_t len = req->out_header.len - sizeof(fuse_out_header);
int error = -req->out_header.error;

- return error;
+ return std::make_pair(len, error);
}

void virtiofs_set_vnode(struct vnode* vnode, struct virtiofs_inode* inode)
@@ -107,7 +110,8 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags,

auto* drv = static_cast<virtio::fs*>(device->private_data);
error = fuse_req_send_and_receive_reply(drv, FUSE_INIT, FUSE_ROOT_ID,
- in_args.get(), sizeof(*in_args), out_args.get(), sizeof(*out_args));
+ in_args.get(), sizeof(*in_args), out_args.get(),
+ sizeof(*out_args)).second;
if (error) {
kprintf("[virtiofs] Failed to initialize fuse filesystem!\n");
return error;
diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc
index 10571a8d..1e3cf99e 100644
--- a/fs/virtiofs/virtiofs_vnops.cc
+++ b/fs/virtiofs/virtiofs_vnops.cc
@@ -62,7 +62,7 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
auto* drv = m_data->drv;
auto error = fuse_req_send_and_receive_reply(drv, FUSE_LOOKUP,
inode->nodeid, in_args.get(), in_args_len, out_args.get(),
- sizeof(*out_args));
+ sizeof(*out_args)).second;
if (error) {
kprintf("[virtiofs] inode %lld, lookup failed to find %s\n",
inode->nodeid, name);
@@ -114,7 +114,7 @@ static int virtiofs_open(struct file* fp)
auto* drv = m_data->drv;
auto error = fuse_req_send_and_receive_reply(drv, FUSE_OPEN,
inode->nodeid, in_args.get(), sizeof(*in_args), out_args.get(),
- sizeof(*out_args));
+ sizeof(*out_args)).second;
if (error) {
kprintf("[virtiofs] inode %lld, open failed\n", inode->nodeid);
return error;
@@ -149,7 +149,7 @@ static int virtiofs_close(struct vnode* vnode, struct file* fp)
auto* m_data = static_cast<virtiofs_mount_data*>(vnode->v_mount->m_data);
auto* drv = m_data->drv;
auto error = fuse_req_send_and_receive_reply(drv, FUSE_RELEASE,
- inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
+ inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, close failed\n", inode->nodeid);
return error;
@@ -177,7 +177,7 @@ static int virtiofs_readlink(struct vnode* vnode, struct uio* uio)
auto* m_data = static_cast<virtiofs_mount_data*>(vnode->v_mount->m_data);
auto* drv = m_data->drv;
auto error = fuse_req_send_and_receive_reply(drv, FUSE_READLINK,
- inode->nodeid, nullptr, 0, link_path.get(), PATH_MAX);
+ inode->nodeid, nullptr, 0, link_path.get(), PATH_MAX).second;
if (error) {
kprintf("[virtiofs] inode %lld, readlink failed\n", inode->nodeid);
return error;
@@ -207,7 +207,8 @@ static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle,
virtiofs_debug("inode %lld, reading %lld bytes at offset %lld\n",
inode.nodeid, read_amt, uio.uio_offset);
auto error = fuse_req_send_and_receive_reply(&drv, FUSE_READ,
- inode.nodeid, in_args.get(), sizeof(*in_args), buf.get(), read_amt);
+ inode.nodeid, in_args.get(), sizeof(*in_args), buf.get(),
+ read_amt).second;
if (error) {
kprintf("[virtiofs] inode %lld, read failed\n", inode.nodeid);
return error;
--
2.27.0

Fotis Xenakis

unread,
Jul 19, 2020, 6:01:51 PM7/19/20
to osv...@googlegroups.com, Fotis Xenakis
When the file being open()ed or close()d is a directory, use
FUSE_OPENDIR and FUSE_RELEASEDIR instead of FUSE_OPEN and FUSE_RELEASE
respectively.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---
fs/virtiofs/virtiofs_vnops.cc | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc
index 1e3cf99e..4bea964e 100644
--- a/fs/virtiofs/virtiofs_vnops.cc
+++ b/fs/virtiofs/virtiofs_vnops.cc
@@ -112,7 +112,8 @@ static int virtiofs_open(struct file* fp)

auto* m_data = static_cast<virtiofs_mount_data*>(vnode->v_mount->m_data);
auto* drv = m_data->drv;
- auto error = fuse_req_send_and_receive_reply(drv, FUSE_OPEN,
+ auto operation = S_ISDIR(inode->attr.mode) ? FUSE_OPENDIR : FUSE_OPEN;
+ auto error = fuse_req_send_and_receive_reply(drv, operation,
inode->nodeid, in_args.get(), sizeof(*in_args), out_args.get(),
sizeof(*out_args)).second;
if (error) {
@@ -148,7 +149,8 @@ static int virtiofs_close(struct vnode* vnode, struct file* fp)

auto* m_data = static_cast<virtiofs_mount_data*>(vnode->v_mount->m_data);
auto* drv = m_data->drv;
- auto error = fuse_req_send_and_receive_reply(drv, FUSE_RELEASE,
+ auto operation = S_ISDIR(inode->attr.mode) ? FUSE_RELEASEDIR : FUSE_RELEASE;
+ auto error = fuse_req_send_and_receive_reply(drv, operation,
inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, close failed\n", inode->nodeid);
--
2.27.0

Fotis Xenakis

unread,
Jul 19, 2020, 6:02:42 PM7/19/20
to osv...@googlegroups.com, Fotis Xenakis
Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---
fs/virtiofs/virtiofs_vnops.cc | 76 ++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc
index 4bea964e..66eb6fb3 100644
--- a/fs/virtiofs/virtiofs_vnops.cc
+++ b/fs/virtiofs/virtiofs_vnops.cc
@@ -13,6 +13,7 @@
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/types.h>
+#include <tuple>

#include <osv/contiguous_alloc.hh>
#include <osv/debug.h>
@@ -24,6 +25,7 @@
#include <osv/sched.hh>
#include <osv/vnode.h>

+#include "fuse_kernel.h"
#include "virtiofs.hh"
#include "virtiofs_dax.hh"
#include "virtiofs_i.hh"
@@ -264,11 +266,81 @@ static int virtiofs_read(struct vnode* vnode, struct file* fp, struct uio* uio,
ioflag, *drv, *uio);
}

+// Checks if @buf (with size @len) points to a valid fuse_dirent (with its name
+// not exceeding @name_max) and if so returns @buf. Otherwise, returns nullptr.
+static fuse_dirent* parse_fuse_dirent(void* buf, size_t len, size_t name_max)
+{
+ if (len < FUSE_NAME_OFFSET) {
+ return nullptr;
+ }
+
+ auto* fdir = static_cast<struct fuse_dirent*>(buf);
+ if (FUSE_DIRENT_SIZE(fdir) > len || fdir->namelen > name_max) {
+ return nullptr;
+ }
+
+ return fdir;
+}
+
static int virtiofs_readdir(struct vnode* vnode, struct file* fp,
struct dirent* dir)
{
- // TODO: Implement
- return EPERM;
+ auto* inode = static_cast<virtiofs_inode*>(vnode->v_data);
+ auto* file_data = static_cast<virtiofs_file_data*>(fp->f_data);
+ auto* m_data = static_cast<virtiofs_mount_data*>(vnode->v_mount->m_data);
+ auto* drv = m_data->drv;
+
+ // NOTE: The response consists of a buffer of size <= fuse_read_in.size.
+ // This contains multiple (whole) fuse_dirent structs, each padded to 64-bit
+ // boundary. We only parse one such struct at a time, for simplicity. The
+ // fuse_dirent.name field is _not_ null-terminated, but our dirent.d_name
+ // is, requiring caution.
+ std::unique_ptr<fuse_read_in> in_args {new (std::nothrow) fuse_read_in()};
+ constexpr size_t name_max = sizeof(dir->d_name) - 1; // account for '\0'
+ // The size of a (padded) fuse_dirent with a name_max-long name
+ size_t bufsize = FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET + name_max);
+ std::unique_ptr<void, std::function<void(void*)>> buf {
+ memory::alloc_phys_contiguous_aligned(bufsize,
+ alignof(std::max_align_t)), memory::free_phys_contiguous_aligned };
+ if (!in_args || !buf) {
+ return ENOMEM;
+ }
+ in_args->fh = file_data->file_handle;
+ in_args->offset = fp->f_offset;
+ in_args->size = bufsize;
+ in_args->flags = fp->f_flags;
+
+ size_t len;
+ int error;
+ std::tie(len, error) = fuse_req_send_and_receive_reply(drv, FUSE_READDIR,
+ inode->nodeid, in_args.get(), sizeof(*in_args), buf.get(), bufsize);
+ if (error) {
+ kprintf("[virtiofs] inode %lld, readdir failed\n", inode->nodeid);
+ return error;
+ }
+
+ if (len == 0) {
+ return ENOENT;
+ }
+ auto* fdir = parse_fuse_dirent(buf.get(), len, name_max);
+ if (!fdir) {
+ kprintf("[virtiofs] inode %lld, dirent parsing failed\n",
+ inode->nodeid);
+ return EIO;
+ }
+
+ dir->d_ino = fdir->ino;
+ dir->d_off = fdir->off;
+ dir->d_type = fdir->type;
+ // Copy fdir->name (not null-terminated) to dir->d_name (null-terminated)
+ memcpy(dir->d_name, fdir->name, fdir->namelen);
+ dir->d_name[fdir->namelen] = '\0';
+
+ fp->f_offset = fdir->off;
+
+ virtiofs_debug("inode %lld, read dir entry %s\n", inode->nodeid,
+ dir->d_name);
+ return 0;
}

static int virtiofs_getattr(struct vnode* vnode, struct vattr* attr)
--
2.27.0

Commit Bot

unread,
Jul 21, 2020, 8:47:46 PM7/21/20
to osv...@googlegroups.com, Fotis Xenakis
From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio-fs: fix allocation failure condition

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id: <AM0PR03MB62923313E3...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc

Commit Bot

unread,
Jul 21, 2020, 8:47:48 PM7/21/20
to osv...@googlegroups.com, Fotis Xenakis
From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio-fs: expose FUSE reply length

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id: <AM0PR03MB6292EFEA10...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/fs/virtiofs/virtiofs_dax.cc b/fs/virtiofs/virtiofs_dax.cc
--- a/fs/virtiofs/virtiofs_dax.cc
+++ b/fs/virtiofs/virtiofs_dax.cc
@@ -194,7 +194,7 @@ int dax_manager::map_ll(uint64_t nodeid, uint64_t file_handle, chunk nchunks,
"moffset=%lld)\n", nodeid, in_args->foffset, in_args->len,
in_args->moffset);
auto error = fuse_req_send_and_receive_reply(&_drv, FUSE_SETUPMAPPING,
- nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
+ nodeid, in_args.get(), sizeof(*in_args), nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, mapping setup failed\n", nodeid);
return error;
@@ -230,7 +230,7 @@ int dax_manager::unmap_ll(chunk nchunks, chunk mstart)
virtiofs_debug("inode %lld, removing mapping (moffset=%lld, len=%lld)\n",
nodeid, r_one->moffset, r_one->len);
auto error = fuse_req_send_and_receive_reply(&_drv, FUSE_REMOVEMAPPING,
- nodeid, in_args.get(), in_args_size, nullptr, 0);
+ nodeid, in_args.get(), in_args_size, nullptr, 0).second;
if (error) {
kprintf("[virtiofs] inode %lld, mapping removal failed\n", nodeid);
return error;
diff --git a/fs/virtiofs/virtiofs_i.hh b/fs/virtiofs/virtiofs_i.hh
--- a/fs/virtiofs/virtiofs_i.hh
+++ b/fs/virtiofs/virtiofs_i.hh
@@ -8,10 +8,12 @@
#ifndef VIRTIOFS_IO_H
#define VIRTIOFS_IO_H

+#include <utility>
+
#include "drivers/virtio-fs.hh"

-int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
- uint64_t nodeid, void* input_args_data, size_t input_args_size,
- void* output_args_data, size_t output_args_size);
+std::pair<size_t, int> fuse_req_send_and_receive_reply(virtio::fs* drv,
+ uint32_t opcode, uint64_t nodeid, void* input_args_data,
+ size_t input_args_size, void* output_args_data, size_t output_args_size);

#endif
diff --git a/fs/virtiofs/virtiofs_vfsops.cc b/fs/virtiofs/virtiofs_vfsops.cc

Commit Bot

unread,
Jul 21, 2020, 8:47:49 PM7/21/20
to osv...@googlegroups.com, Fotis Xenakis
From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio-fs: use FUSE_OPENDIR and FUSE_RELEASEDIR

When the file being open()ed or close()d is a directory, use
FUSE_OPENDIR and FUSE_RELEASEDIR instead of FUSE_OPEN and FUSE_RELEASE
respectively.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id: <AM0PR03MB62929C1216...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc

Commit Bot

unread,
Jul 21, 2020, 8:47:50 PM7/21/20
to osv...@googlegroups.com, Fotis Xenakis
From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio-fs: implement readdir

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id: <AM0PR03MB6292510F3F...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc

Waldek Kozaczuk

unread,
Jul 21, 2020, 8:57:58 PM7/21/20
to Fotis Xenakis, OSv Development
Fotis,

Thanks for these patches - it makes virtiofs almost fully functional.

Regarding the d_off issue in other filesystems, I would welcome your patch to fix it. What exact symptom is caused by this incorrect implementation of d_off?

Now according to this - https://man7.org/linux/man-pages/man3/readdir.3.html - d_off is not something that userspace should necessarily rely on and I am not sure how easy it will be to fix it for each other FS. 

Waldek


--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/AM0PR03MB62928C209CFA5D9AF2F1731BA67A0%40AM0PR03MB6292.eurprd03.prod.outlook.com.

Fotis Xenakis

unread,
Jul 22, 2020, 3:07:02 PM7/22/20
to OSv Development
It's nice to see it pick up more functionality (readdir sure was necessary). My next goal is booting from virtio-fs (probably taking an approach similar to ROFS), hoping that will come together within the next couple of weeks.

The omission of d_off is not much of an issue, just something that striked me while reading other readdir implementations. From the manpage: "The  value  returned in d_off is the same as would be returned by calling telldir(3) at the current position in the directory stream." Since our telldir implementation returns the value of file.f_offset (https://github.com/cloudius-systems/osv/blob/a16e5c9c0ef11217be374824587e05225fbfde61/fs/vfs/vfs_syscalls.cc#L508), I guess that making sure that's also returned in d_off is a safe choice. I shall get round to sending a patch for this soon!
Reply all
Reply to author
Forward
0 new messages