From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkozac...@gmail.com>
Branch: master
virtio-fs: minor code improvements in driver
These include:
- Checking memory allocations
- Using static_cast instead of reinterpret_cast where possible
- Formatting and consistency
Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
Message-Id:
<vi1pr03mb43837c675943630604030e92a6...@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,7 +28,7 @@
using namespace memory;
-void fuse_req_wait(struct fuse_request* req)
+void fuse_req_wait(fuse_request* req)
{
WITH_LOCK(req->req_mutex) {
req->req_wait.wait(req->req_mutex);
@@ -37,37 +37,37 @@ void fuse_req_wait(struct fuse_request* req)
namespace virtio {
-static int fuse_make_request(void *driver, struct fuse_request* req)
+static int fuse_make_request(void* driver, fuse_request* req)
{
- auto fs_driver = reinterpret_cast<fs*>(driver);
+ auto fs_driver = static_cast<fs*>(driver);
return fs_driver->make_request(req);
}
-static void fuse_req_done(struct fuse_request* req)
+static void fuse_req_done(fuse_request* req)
{
WITH_LOCK(req->req_mutex) {
req->req_wait.wake_one(req->req_mutex);
}
}
-static void fuse_req_enqueue_input(vring* queue, struct fuse_request* req)
+static void fuse_req_enqueue_input(vring* queue, fuse_request* req)
{
// Header goes first
queue->add_out_sg(&req->in_header, sizeof(struct fuse_in_header));
- //
+
// Add fuse in arguments as out sg
- if (req->input_args_size) {
+ if (req->input_args_size > 0) {
queue->add_out_sg(req->input_args_data, req->input_args_size);
}
}
-static void fuse_req_enqueue_output(vring* queue, struct fuse_request* req)
+static void fuse_req_enqueue_output(vring* queue, fuse_request* req)
{
// Header goes first
queue->add_in_sg(&req->out_header, sizeof(struct fuse_out_header));
- //
+
// Add fuse out arguments as in sg
- if (req->output_args_size) {
+ if (req->output_args_size > 0) {
queue->add_in_sg(req->output_args_data, req->output_args_size);
}
}
@@ -93,14 +93,13 @@ struct driver fs_driver = {
bool fs::ack_irq()
{
auto isr = _dev.read_and_ack_isr();
- auto queue = get_virt_queue(VQ_REQUEST);
+ auto* queue = get_virt_queue(VQ_REQUEST);
if (isr) {
queue->disable_interrupts();
return true;
- } else {
- return false;
}
+ return false;
}
fs::fs(virtio_device& virtio_dev)
@@ -113,7 +112,6 @@ fs::fs(virtio_device& virtio_dev)
// Steps 4, 5 & 6 - negotiate and confirm features
setup_features();
read_config();
-
if (_config.num_queues < 1) {
virtio_i("Expected at least one request queue -> baling out!\n");
return;
@@ -122,29 +120,32 @@ fs::fs(virtio_device& virtio_dev)
// Step 7 - generic init of virtqueues
probe_virt_queues();
- //register the single irq callback for the block
+ // register the single irq callback for the block
sched::thread* t = sched::thread::make([this] { this->req_done(); },
- sched::thread::attr().name("virtio-fs"));
+ sched::thread::attr().name("virtio-fs"));
t->start();
- auto queue = get_virt_queue(VQ_REQUEST);
+ auto* queue = get_virt_queue(VQ_REQUEST);
interrupt_factory int_factory;
- int_factory.register_msi_bindings = [queue, t](interrupt_manager &msi) {
- msi.easy_register( {{ VQ_REQUEST, [=] { queue->disable_interrupts();
}, t }});
+ int_factory.register_msi_bindings = [queue, t](interrupt_manager& msi) {
+ msi.easy_register({
+ {VQ_HIPRIO, nullptr, nullptr},
+ {VQ_REQUEST, [=] { queue->disable_interrupts(); }, t}
+ });
};
- int_factory.create_pci_interrupt = [this,t](pci::device &pci_dev) {
+ int_factory.create_pci_interrupt = [this, t](pci::device& pci_dev) {
return new pci_interrupt(
pci_dev,
[=] { return this->ack_irq(); },
[=] { t->wake(); });
};
#ifndef AARCH64_PORT_STUB
- int_factory.create_gsi_edge_interrupt = [this,t]() {
+ int_factory.create_gsi_edge_interrupt = [this, t]() {
return new gsi_edge_interrupt(
- _dev.get_irq(),
- [=] { if (this->ack_irq()) t->wake(); });
+ _dev.get_irq(),
+ [=] { if (this->ack_irq()) t->wake(); });
};
#endif
@@ -159,37 +160,40 @@ fs::fs(virtio_device& virtio_dev)
std::string dev_name("virtiofs");
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?
- struct fuse_strategy *strategy = reinterpret_cast<struct
fuse_strategy*>(dev->private_data);
+ 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;
- debugf("virtio-fs: Add device instance %d as [%s]\n", _id,
dev_name.c_str());
+ debugf("virtio-fs: Add device instance %d as [%s]\n", _id,
+ dev_name.c_str());
}
fs::~fs()
{
- //TODO: In theory maintain the list of free instances and gc it
+ // TODO: In theory maintain the list of free instances and gc it
// including the thread objects and their stack
}
void fs::read_config()
{
- virtio_conf_read(0, &(_config.tag[0]), sizeof(_config.tag));
- virtio_conf_read(offsetof(fs_config,num_queues), &(_config.num_queues),
sizeof(_config.num_queues));
- debugf("virtio-fs: Detected device with tag: [%s] and num_queues: %d\n",
_config.tag, _config.num_queues);
+ virtio_conf_read(0, &_config, sizeof(_config));
+ debugf("virtio-fs: Detected device with tag: [%s] and num_queues: %d\n",
+ _config.tag, _config.num_queues);
}
void fs::req_done()
{
auto* queue = get_virt_queue(VQ_REQUEST);
- fs_req* req;
- while (1) {
+ while (true) {
virtio_driver::wait_for_queue(queue, &vring::used_ring_not_empty);
+ fs_req* req;
u32 len;
- while((req = static_cast<fs_req*>(queue->get_buf_elem(&len))) !=
nullptr) {
+ while ((req = static_cast<fs_req*>(queue->get_buf_elem(&len))) !=
+ nullptr) {
+
fuse_req_done(req->fuse_req);
delete req;
queue->get_buf_finalize();
@@ -200,12 +204,13 @@ void fs::req_done()
}
}
-int fs::make_request(struct fuse_request* req)
+int fs::make_request(fuse_request* req)
{
// The lock is here for parallel requests protection
WITH_LOCK(_lock) {
-
- if (!req) return EIO;
+ if (!req) {
+ return EIO;
+ }
auto* queue = get_virt_queue(VQ_REQUEST);
@@ -214,7 +219,10 @@ int fs::make_request(struct fuse_request* req)
fuse_req_enqueue_input(queue, req);
fuse_req_enqueue_output(queue, req);
- auto* fs_request = new fs_req(req);
+ auto* fs_request = new (std::nothrow) fs_req(req);
+ if (!fs_request) {
+ return ENOMEM;
+ }
queue->add_buf_wait(fs_request);
queue->kick();
diff --git a/drivers/virtio-fs.hh b/drivers/virtio-fs.hh
--- a/drivers/virtio-fs.hh
+++ b/drivers/virtio-fs.hh
@@ -17,8 +17,8 @@
namespace virtio {
enum {
- VQ_HIPRIO,
- VQ_REQUEST
+ VQ_HIPRIO = 0,
+ VQ_REQUEST = 1
};
class fs : public virtio_driver {
@@ -34,26 +34,27 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();
- int make_request(struct fuse_request*);
+ int make_request(fuse_request*);
void req_done();
int64_t size();
bool ack_irq();
static hw_driver* probe(hw_device* dev);
+
private:
struct fs_req {
- fs_req(struct fuse_request* f) :fuse_req(f) {};
+ fs_req(fuse_request* f) : fuse_req(f) {};
~fs_req() {};
- struct fuse_request* fuse_req;
+ fuse_request* fuse_req;
};
std::string _driver_name;
fs_config _config;
- //maintains the virtio instance number for multiple drives
+ // maintains the virtio instance number for multiple drives
static int _instance;
int _id;
// This mutex protects parallel make_request invocations
--
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/00000000000014211005a46676de%40google.com.
