From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkozac...@gmail.com>
Branch: master
virtio-fs: refactor driver / fs
Since in virtio-fs the filesystem is very tightly coupled with the
driver, this tries to make clear the dependence of the first on the
second, as well as simplify.
This includes:
- The definition of fuse_request is moved from the fs to the driver,
since it is part of the interface it provides. Also, it is enhanced
with methods, somewhat promoting it to a "proper" class.
- fuse_strategy, as a redirection to the driver is removed and instead
the dependence on the driver is made explicit.
- Last, virtio::fs::fs_req is removed and fuse_request is used in its
place, since it offered no value with fuse_request now defined in the
driver.
Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id:
<vi1pr03mb4383c1d8e60219c273114e4aa6...@vi1pr03mb4383.eurprd03.prod.outlook.com>
---
diff --git a/drivers/virtio-fs.cc b/drivers/virtio-fs.cc
--- a/drivers/virtio-fs.cc
+++ b/drivers/virtio-fs.cc
@@ -28,25 +28,23 @@
using namespace memory;
-void fuse_req_wait(fuse_request* req)
-{
- WITH_LOCK(req->req_mutex) {
- req->req_wait.wait(req->req_mutex);
- }
-}
+using fuse_request = virtio::fs::fuse_request;
namespace virtio {
-static int fuse_make_request(void* driver, fuse_request* req)
+// Wait for the request to be marked as completed.
+void fs::fuse_request::wait()
{
- auto fs_driver = static_cast<fs*>(driver);
- return fs_driver->make_request(req);
+ WITH_LOCK(req_mutex) {
+ req_wait.wait(req_mutex);
+ }
}
-static void fuse_req_done(fuse_request* req)
+// Mark the request as completed.
+void fs::fuse_request::done()
{
- WITH_LOCK(req->req_mutex) {
- req->req_wait.wake_one(req->req_mutex);
+ WITH_LOCK(req_mutex) {
+ req_wait.wake_one(req_mutex);
}
}
@@ -87,7 +85,7 @@ static struct devops fs_devops {
struct driver fs_driver = {
"virtio_fs",
&fs_devops,
- sizeof(struct fuse_strategy),
+ sizeof(fs*),
};
bool fs::ack_irq()
@@ -161,10 +159,7 @@ fs::fs(virtio_device& virtio_dev)
dev_name += std::to_string(_disk_idx++);
struct device* dev = device_create(&fs_driver, dev_name.c_str(), D_BLK);
// TODO Should it be really D_BLK?
- auto* strategy = static_cast<fuse_strategy*>(dev->private_data);
- strategy->drv = this;
- strategy->make_request = fuse_make_request;
-
+ dev->private_data = this;
debugf("virtio-fs: Add device instance %d as [%s]\n", _id,
dev_name.c_str());
}
@@ -201,13 +196,12 @@ void fs::req_done()
while (true) {
virtio_driver::wait_for_queue(queue, &vring::used_ring_not_empty);
- fs_req* req;
+ fuse_request* req;
u32 len;
- while ((req = static_cast<fs_req*>(queue->get_buf_elem(&len))) !=
+ while ((req = static_cast<fuse_request*>(queue->get_buf_elem(&len))) !=
nullptr) {
- fuse_req_done(req->fuse_req);
- delete req;
+ req->done();
queue->get_buf_finalize();
}
@@ -231,11 +225,7 @@ int fs::make_request(fuse_request* req)
fuse_req_enqueue_input(queue, req);
fuse_req_enqueue_output(queue, req);
- auto* fs_request = new (std::nothrow) fs_req(req);
- if (!fs_request) {
- return ENOMEM;
- }
- queue->add_buf_wait(fs_request);
+ queue->add_buf_wait(req);
queue->kick();
return 0;
diff --git a/drivers/virtio-fs.hh b/drivers/virtio-fs.hh
--- a/drivers/virtio-fs.hh
+++ b/drivers/virtio-fs.hh
@@ -12,7 +12,7 @@
#include <osv/waitqueue.hh>
#include "drivers/virtio.hh"
#include "drivers/virtio-device.hh"
-#include "fs/virtiofs/virtiofs_i.hh"
+#include "fs/virtiofs/fuse_kernel.h"
namespace virtio {
@@ -23,6 +23,24 @@ enum {
class fs : public virtio_driver {
public:
+ struct fuse_request {
+ struct fuse_in_header in_header;
+ struct fuse_out_header out_header;
+
+ void* input_args_data;
+ size_t input_args_size;
+
+ void* output_args_data;
+ size_t output_args_size;
+
+ void wait();
+ void done();
+
+ private:
+ mutex_t req_mutex;
+ waitqueue req_wait;
+ };
+
struct fs_config {
char tag[36];
u32 num_queues;
@@ -59,13 +77,6 @@ public:
static hw_driver* probe(hw_device* dev);
private:
- struct fs_req {
- fs_req(fuse_request* f) : fuse_req(f) {};
- ~fs_req() {};
-
- fuse_request* fuse_req;
- };
-
std::string _driver_name;
fs_config _config;
dax_window _dax;
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
@@ -11,30 +11,10 @@
#include "fuse_kernel.h"
#include <osv/mutex.h>
#include <osv/waitqueue.hh>
+#include "drivers/virtio-fs.hh"
-struct fuse_request {
- struct fuse_in_header in_header;
- struct fuse_out_header out_header;
-
- void* input_args_data;
- size_t input_args_size;
-
- void* output_args_data;
- size_t output_args_size;
-
- mutex_t req_mutex;
- waitqueue req_wait;
-};
-
-struct fuse_strategy {
- void* drv;
- int (*make_request)(void*, fuse_request*);
-};
-
-int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode,
+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);
-void fuse_req_wait(fuse_request* req);
-
#endif
diff --git a/fs/virtiofs/virtiofs_vfsops.cc b/fs/virtiofs/virtiofs_vfsops.cc
--- a/fs/virtiofs/virtiofs_vfsops.cc
+++ b/fs/virtiofs/virtiofs_vfsops.cc
@@ -14,9 +14,11 @@
#include "virtiofs_i.hh"
#include "drivers/virtio-fs.hh"
+using fuse_request = virtio::fs::fuse_request;
+
static std::atomic<uint64_t> fuse_unique_id(1);
-int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode,
+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)
{
@@ -36,9 +38,9 @@ int fuse_req_send_and_receive_reply(fuse_strategy* strategy,
uint32_t opcode,
req->output_args_data = output_args_data;
req->output_args_size = output_args_size;
- assert(strategy->drv);
- strategy->make_request(strategy->drv, req.get());
- fuse_req_wait(req.get());
+ assert(drv);
+ drv->make_request(req.get());
+ req->wait();
int error = -req->out_header.error;
@@ -88,8 +90,8 @@ static int virtiofs_mount(struct mount* mp, const char* dev,
int flags,
in_args->max_readahead = PAGE_SIZE;
in_args->flags |= FUSE_MAP_ALIGNMENT;
- auto* strategy = static_cast<fuse_strategy*>(device->private_data);
- error = fuse_req_send_and_receive_reply(strategy, FUSE_INIT, FUSE_ROOT_ID,
+ 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));
if (error) {
kprintf("[virtiofs] Failed to initialize fuse filesystem!\n");
@@ -101,7 +103,6 @@ static int virtiofs_mount(struct mount* mp, const char*
dev, int flags,
"minor: %d\n", out_args->major, out_args->minor);
if (out_args->flags & FUSE_MAP_ALIGNMENT) {
- auto* drv = static_cast<virtio::fs*>(strategy->drv);
drv->set_map_alignment(out_args->map_alignment);
}
@@ -114,7 +115,7 @@ static int virtiofs_mount(struct mount* mp, const char*
dev, int flags,
virtiofs_set_vnode(mp->m_root->d_vnode, root_node);
- mp->m_data = strategy;
+ mp->m_data = drv;
mp->m_dev = device;
return 0;
diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc
--- a/fs/virtiofs/virtiofs_vnops.cc
+++ b/fs/virtiofs/virtiofs_vnops.cc
@@ -28,7 +28,6 @@
#include "virtiofs.hh"
#include "virtiofs_i.hh"
-#include "drivers/virtio-fs.hh"
static constexpr uint32_t OPEN_FLAGS = O_RDONLY;
@@ -60,8 +59,8 @@ static int virtiofs_lookup(struct vnode* vnode, char* name,
struct vnode** vpp)
}
strcpy(in_args.get(), name);
- auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
- auto error = fuse_req_send_and_receive_reply(strategy, FUSE_LOOKUP,
+ auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
+ 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));
if (error) {
@@ -111,8 +110,8 @@ static int virtiofs_open(struct file* fp)
}
in_args->flags = OPEN_FLAGS;
- auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
- auto error = fuse_req_send_and_receive_reply(strategy, FUSE_OPEN,
+ auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
+ 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));
if (error) {
@@ -146,8 +145,8 @@ static int virtiofs_close(struct vnode* vnode, struct file*
fp)
in_args->fh = f_data->file_handle;
in_args->flags = OPEN_FLAGS; // need to be same as in FUSE_OPEN
- auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
- auto error = fuse_req_send_and_receive_reply(strategy, FUSE_RELEASE,
+ auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
+ auto error = fuse_req_send_and_receive_reply(drv, FUSE_RELEASE,
inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
if (error) {
kprintf("[virtiofs] inode %lld, close failed\n", inode->nodeid);
@@ -173,8 +172,8 @@ static int virtiofs_readlink(struct vnode* vnode, struct
uio* uio)
return ENOMEM;
}
- auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
- auto error = fuse_req_send_and_receive_reply(strategy, FUSE_READLINK,
+ auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
+ auto error = fuse_req_send_and_receive_reply(drv, FUSE_READLINK,
inode->nodeid, nullptr, 0, link_path.get(), PATH_MAX);
if (error) {
kprintf("[virtiofs] inode %lld, readlink failed\n", inode->nodeid);
@@ -188,10 +187,9 @@ static int virtiofs_readlink(struct vnode* vnode, struct
uio* uio)
// Read @read_amt bytes from @inode, using the DAX window.
static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle,
- u64 read_amt, fuse_strategy& strategy, struct uio& uio)
+ u64 read_amt, virtio::fs& drv, struct uio& uio)
{
- auto* drv = static_cast<virtio::fs*>(strategy.drv);
- auto* dax = drv->get_dax();
+ auto* dax = drv.get_dax();
// Enter the critical path: setup mapping -> read -> remove mapping
std::lock_guard<mutex> guard {dax->lock};
@@ -215,7 +213,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64
file_handle,
uint64_t moffset = 0;
in_args->moffset = moffset;
- auto map_align = drv->get_map_alignment();
+ auto map_align = drv.get_map_alignment();
if (map_align < 0) {
kprintf("[virtiofs] inode %lld, map alignment not set\n",
inode.nodeid);
return ENOTSUP;
@@ -239,7 +237,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64
file_handle,
virtiofs_debug("inode %lld, setting up mapping (foffset=%lld, len=%lld, "
"moffset=%lld)\n", inode.nodeid, in_args->foffset,
in_args->len, in_args->moffset);
- auto error = fuse_req_send_and_receive_reply(&strategy, FUSE_SETUPMAPPING,
+ auto error = fuse_req_send_and_receive_reply(&drv, FUSE_SETUPMAPPING,
inode.nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
if (error) {
kprintf("[virtiofs] inode %lld, mapping setup failed\n", inode.nodeid);
@@ -277,7 +275,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64
file_handle,
virtiofs_debug("inode %lld, removing mapping (moffset=%lld, len=%lld)\n",
inode.nodeid, r_one->moffset, r_one->len);
- error = fuse_req_send_and_receive_reply(&strategy, FUSE_REMOVEMAPPING,
+ error = fuse_req_send_and_receive_reply(&drv, FUSE_REMOVEMAPPING,
inode.nodeid, r_in_args.get(), r_in_args_size, nullptr, 0);
if (error) {
kprintf("[virtiofs] inode %lld, mapping removal failed\n",
@@ -290,7 +288,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64
file_handle,
// Read @read_amt bytes from @inode, using the fallback FUSE_READ mechanism.
static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle,
- u32 read_amt, u32 flags, fuse_strategy& strategy, struct uio& uio)
+ u32 read_amt, u32 flags, virtio::fs& drv, struct uio& uio)
{
std::unique_ptr<fuse_read_in> in_args {new (std::nothrow) fuse_read_in()};
std::unique_ptr<void, std::function<void(void*)>> buf {
@@ -306,7 +304,7 @@ 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(&strategy, FUSE_READ,
+ auto error = fuse_req_send_and_receive_reply(&drv, FUSE_READ,
inode.nodeid, in_args.get(), sizeof(*in_args), buf.get(), read_amt);
if (error) {
kprintf("[virtiofs] inode %lld, read failed\n", inode.nodeid);
@@ -345,24 +343,23 @@ static int virtiofs_read(struct vnode* vnode, struct
file* fp, struct uio* uio,
auto* inode = static_cast<virtiofs_inode*>(vnode->v_data);
auto* file_data = static_cast<virtiofs_file_data*>(fp->f_data);
- auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
+ auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
// Total read amount is what they requested, or what is left
auto read_amt = std::min<uint64_t>(uio->uio_resid,
inode->attr.size - uio->uio_offset);
- auto* drv = static_cast<virtio::fs*>(strategy->drv);
if (drv->get_dax()) {
// Try to read from DAX
if (!virtiofs_read_direct(*inode, file_data->file_handle, read_amt,
- *strategy, *uio)) {
+ *drv, *uio)) {
return 0;
}
}
// DAX unavailable or failed, use fallback
return virtiofs_read_fallback(*inode, file_data->file_handle, read_amt,
- ioflag, *strategy, *uio);
+ ioflag, *drv, *uio);
}
static int virtiofs_readdir(struct vnode* vnode, struct file* fp,
--
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/000000000000cd7aea05a6d643c6%40google.com.
