* [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo
2019-12-04 19:08 [PATCH v2 0/5] [RFC] virtiofsd, vhost-user-fs: Add support for notification queue Vivek Goyal
@ 2019-12-04 19:08 ` Vivek Goyal
2019-12-04 19:42 ` Dr. David Alan Gilbert
2019-12-04 19:08 ` [PATCH v2 2/5] virtiofsd: Release file locks using F_UNLCK Vivek Goyal
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Vivek Goyal @ 2019-12-04 19:08 UTC (permalink / raw)
To: virtio-fs, qemu-devel; +Cc: mszeredi, dgilbert, stefanha
There are some unused fields in "struct fv_QueueInfo". Get rid of these fields.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/fuse_virtio.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
index 31c8542b6c..2a9cd60a01 100644
--- a/contrib/virtiofsd/fuse_virtio.c
+++ b/contrib/virtiofsd/fuse_virtio.c
@@ -50,12 +50,6 @@ struct fv_QueueInfo {
int qidx;
int kick_fd;
int kill_fd; /* For killing the thread */
-
- /* The element for the command currently being processed */
- VuVirtqElement *qe;
- /* If any of the qe vec elements (towards vmm) are unmappable */
- unsigned int elem_bad_in;
- bool reply_sent;
};
/* A FUSE request */
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo
2019-12-04 19:08 ` [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo Vivek Goyal
@ 2019-12-04 19:42 ` Dr. David Alan Gilbert
2019-12-06 15:55 ` [Virtio-fs] " Dr. David Alan Gilbert
0 siblings, 1 reply; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2019-12-04 19:42 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs, mszeredi, qemu-devel, stefanha
* Vivek Goyal (vgoyal@redhat.com) wrote:
> There are some unused fields in "struct fv_QueueInfo". Get rid of these fields.
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
> contrib/virtiofsd/fuse_virtio.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
> index 31c8542b6c..2a9cd60a01 100644
> --- a/contrib/virtiofsd/fuse_virtio.c
> +++ b/contrib/virtiofsd/fuse_virtio.c
> @@ -50,12 +50,6 @@ struct fv_QueueInfo {
> int qidx;
> int kick_fd;
> int kill_fd; /* For killing the thread */
> -
> - /* The element for the command currently being processed */
> - VuVirtqElement *qe;
> - /* If any of the qe vec elements (towards vmm) are unmappable */
> - unsigned int elem_bad_in;
> - bool reply_sent;
Yep, those last two got moved into FVRequest as part of the thread pool
stuff.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> };
>
> /* A FUSE request */
> --
> 2.20.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Virtio-fs] [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo
2019-12-04 19:42 ` Dr. David Alan Gilbert
@ 2019-12-06 15:55 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2019-12-06 15:55 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs, mszeredi, qemu-devel
* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Vivek Goyal (vgoyal@redhat.com) wrote:
> > There are some unused fields in "struct fv_QueueInfo". Get rid of these fields.
> >
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> > contrib/virtiofsd/fuse_virtio.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
> > index 31c8542b6c..2a9cd60a01 100644
> > --- a/contrib/virtiofsd/fuse_virtio.c
> > +++ b/contrib/virtiofsd/fuse_virtio.c
> > @@ -50,12 +50,6 @@ struct fv_QueueInfo {
> > int qidx;
> > int kick_fd;
> > int kill_fd; /* For killing the thread */
> > -
> > - /* The element for the command currently being processed */
> > - VuVirtqElement *qe;
> > - /* If any of the qe vec elements (towards vmm) are unmappable */
> > - unsigned int elem_bad_in;
> > - bool reply_sent;
>
> Yep, those last two got moved into FVRequest as part of the thread pool
> stuff.
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
I've merged that 1/5 into my world.
> > };
> >
> > /* A FUSE request */
> > --
> > 2.20.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/5] virtiofsd: Release file locks using F_UNLCK
2019-12-04 19:08 [PATCH v2 0/5] [RFC] virtiofsd, vhost-user-fs: Add support for notification queue Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo Vivek Goyal
@ 2019-12-04 19:08 ` Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 3/5] virtiofd: Create a notification queue Vivek Goyal
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2019-12-04 19:08 UTC (permalink / raw)
To: virtio-fs, qemu-devel; +Cc: mszeredi, dgilbert, stefanha
We are emulating posix locks for guest using open file description locks
in virtiofsd. When any of the fd is closed in guest, we find associated
OFD lock fd (if there is one) and close it to release all the locks.
Assumption here is that there is no other thread using lo_inode_plock
structure or plock->fd, hence it is safe to do so.
But now we are about to introduce blocking variant of locks (SETLKW),
and that means we might be waiting to a lock to be available and
using plock->fd. And that means there are still users of plock structure.
So release locks using fcntl(SETLK, F_UNLCK) instead and plock will
be freed later.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
index bc214df0c7..6aa56882e8 100644
--- a/contrib/virtiofsd/passthrough_ll.c
+++ b/contrib/virtiofsd/passthrough_ll.c
@@ -936,6 +936,14 @@ static void put_shared(struct lo_data *lo, struct lo_inode *inode)
}
}
+static void posix_locks_value_destroy(gpointer data)
+{
+ struct lo_inode_plock *plock = data;
+
+ close(plock->fd);
+ free(plock);
+}
+
/* Increments nlookup and caller must release refcount using
* lo_inode_put(&parent).
*/
@@ -994,7 +1002,9 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
inode->key.ino = e->attr.st_ino;
inode->key.dev = e->attr.st_dev;
pthread_mutex_init(&inode->plock_mutex, NULL);
- inode->posix_locks = g_hash_table_new(g_direct_hash, g_direct_equal);
+ inode->posix_locks = g_hash_table_new_full(g_direct_hash,
+ g_direct_equal, NULL,
+ posix_locks_value_destroy);
get_shared(lo, inode);
@@ -1436,9 +1446,6 @@ static void unref_inode(struct lo_data *lo, struct lo_inode *inode, uint64_t n)
if (!inode->nlookup) {
lo_map_remove(&lo->ino_map, inode->fuse_ino);
g_hash_table_remove(lo->inodes, &inode->key);
- if (g_hash_table_size(inode->posix_locks)) {
- fuse_log(FUSE_LOG_WARNING, "Hash table is not empty\n");
- }
g_hash_table_destroy(inode->posix_locks);
pthread_mutex_destroy(&inode->plock_mutex);
@@ -1868,6 +1875,7 @@ static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo,
plock->fd = fd;
g_hash_table_insert(inode->posix_locks,
GUINT_TO_POINTER(plock->lock_owner), plock);
+ fuse_log(FUSE_LOG_DEBUG, "lookup_create_plock_ctx(): Inserted element in posix_locks hash table with value pointer %p\n", plock);
return plock;
}
@@ -2046,6 +2054,7 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
(void) ino;
struct lo_inode *inode;
struct lo_inode_plock *plock;
+ struct flock flock;
inode = lo_inode(req, ino);
if (!inode) {
@@ -2058,14 +2067,16 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
plock = g_hash_table_lookup(inode->posix_locks,
GUINT_TO_POINTER(fi->lock_owner));
if (plock) {
- g_hash_table_remove(inode->posix_locks,
- GUINT_TO_POINTER(fi->lock_owner));
/*
- * We had used open() for locks and had only one fd. So
- * closing this fd should release all OFD locks.
+ * An fd is being closed. For posix locks, this means
+ * drop all the associated locks.
*/
- close(plock->fd);
- free(plock);
+ memset(&flock, 0, sizeof(struct flock));
+ flock.l_type = F_UNLCK;
+ flock.l_whence = SEEK_SET;
+ /* Unlock whole file */
+ flock.l_start = flock.l_len = 0;
+ fcntl(plock->fd, F_OFD_SETLK, &flock);
}
pthread_mutex_unlock(&inode->plock_mutex);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/5] virtiofd: Create a notification queue
2019-12-04 19:08 [PATCH v2 0/5] [RFC] virtiofsd, vhost-user-fs: Add support for notification queue Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 1/5] virtiofsd: Get rid of unused fields in fv_QueueInfo Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 2/5] virtiofsd: Release file locks using F_UNLCK Vivek Goyal
@ 2019-12-04 19:08 ` Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 4/5] virtiofsd: Specify size of notification buffer using config space Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 5/5] virtiofsd: Implement blocking posix locks Vivek Goyal
4 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2019-12-04 19:08 UTC (permalink / raw)
To: virtio-fs, qemu-devel; +Cc: mszeredi, dgilbert, stefanha
Add a notification queue which will be used to send async notifications
for file lock availability.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/fuse_i.h | 1 +
contrib/virtiofsd/fuse_virtio.c | 74 +++++++++++++++-------
hw/virtio/vhost-user-fs-pci.c | 2 +-
hw/virtio/vhost-user-fs.c | 37 +++++++++--
include/hw/virtio/vhost-user-fs.h | 1 +
include/standard-headers/linux/virtio_fs.h | 3 +
6 files changed, 87 insertions(+), 31 deletions(-)
diff --git a/contrib/virtiofsd/fuse_i.h b/contrib/virtiofsd/fuse_i.h
index 966b1a3baa..4eeae0bfeb 100644
--- a/contrib/virtiofsd/fuse_i.h
+++ b/contrib/virtiofsd/fuse_i.h
@@ -74,6 +74,7 @@ struct fuse_session {
char *vu_socket_lock;
struct fv_VuDev *virtio_dev;
int thread_pool_size;
+ bool notify_enabled;
};
struct fuse_chan {
diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
index 2a9cd60a01..b1eebcf054 100644
--- a/contrib/virtiofsd/fuse_virtio.c
+++ b/contrib/virtiofsd/fuse_virtio.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu/iov.h"
#include "qapi/error.h"
+#include "standard-headers/linux/virtio_fs.h"
#include "fuse_i.h"
#include "fuse_kernel.h"
#include "fuse_misc.h"
@@ -92,23 +93,31 @@ struct fv_VuDev {
*/
size_t nqueues;
struct fv_QueueInfo **qi;
-};
-
-/* From spec */
-struct virtio_fs_config {
- char tag[36];
- uint32_t num_queues;
+ /* True if notification queue is being used */
+ bool notify_enabled;
};
/* Callback from libvhost-user */
static uint64_t fv_get_features(VuDev *dev)
{
- return 1ULL << VIRTIO_F_VERSION_1;
+ uint64_t features;
+
+ features = 1ull << VIRTIO_F_VERSION_1 |
+ 1ull << VIRTIO_FS_F_NOTIFICATION;
+
+ return features;
}
/* Callback from libvhost-user */
static void fv_set_features(VuDev *dev, uint64_t features)
{
+ struct fv_VuDev *vud = container_of(dev, struct fv_VuDev, dev);
+ struct fuse_session *se = vud->se;
+
+ if ((1ull << VIRTIO_FS_F_NOTIFICATION) & features) {
+ vud->notify_enabled = true;
+ se->notify_enabled = true;
+ }
}
/*
@@ -765,6 +774,9 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
{
struct fv_VuDev *vud = container_of(dev, struct fv_VuDev, dev);
struct fv_QueueInfo *ourqi;
+ void * (*thread_func) (void *) = fv_queue_thread;
+ int valid_queues = 2; /* One hiprio queue and one request queue */
+ bool notification_q = false;
fuse_log(FUSE_LOG_INFO, "%s: qidx=%d started=%d\n", __func__, qidx,
started);
@@ -776,10 +788,18 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
* well-behaved client in mind and may not protect against all types of
* races yet.
*/
- if (qidx > 1) {
- fuse_log(FUSE_LOG_ERR,
- "%s: multiple request queues not yet implemented, please only "
- "configure 1 request queue\n",
+ if (vud->notify_enabled) {
+ valid_queues++;
+ /*
+ * If notification queue is enabled, then qidx 1 is notificaiton queue.
+ */
+ if (qidx == 1)
+ notification_q = true;
+ }
+
+ if (qidx >= valid_queues) {
+ fuse_log(FUSE_LOG_ERR, "%s: multiple request queues not yet"
+ "implemented, please only configure 1 request queue\n",
__func__);
exit(EXIT_FAILURE);
}
@@ -803,13 +823,19 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
assert(vud->qi[qidx]->kick_fd == -1);
}
ourqi = vud->qi[qidx];
+ pthread_mutex_init(&ourqi->vq_lock, NULL);
+ /*
+ * For notification queue, we don't have to start a thread yet.
+ */
+ if (notification_q)
+ return;
+
ourqi->kick_fd = dev->vq[qidx].kick_fd;
ourqi->kill_fd = eventfd(0, EFD_CLOEXEC | EFD_SEMAPHORE);
assert(ourqi->kill_fd != -1);
- pthread_mutex_init(&ourqi->vq_lock, NULL);
- if (pthread_create(&ourqi->thread, NULL, fv_queue_thread, ourqi)) {
+ if (pthread_create(&ourqi->thread, NULL, thread_func, ourqi)) {
fuse_log(FUSE_LOG_ERR, "%s: Failed to create thread for queue %d\n",
__func__, qidx);
assert(0);
@@ -819,17 +845,19 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
assert(qidx < vud->nqueues);
ourqi = vud->qi[qidx];
- /* Kill the thread */
- if (eventfd_write(ourqi->kill_fd, 1)) {
- fuse_log(FUSE_LOG_ERR, "Eventfd_read for queue: %m\n");
- }
- ret = pthread_join(ourqi->thread, NULL);
- if (ret) {
- fuse_log(FUSE_LOG_ERR, "%s: Failed to join thread idx %d err %d\n",
- __func__, qidx, ret);
+ if (!notification_q) {
+ /* Kill the thread */
+ if (eventfd_write(ourqi->kill_fd, 1)) {
+ fuse_log(FUSE_LOG_ERR, "Eventfd_read for queue: %m\n");
+ }
+ ret = pthread_join(ourqi->thread, NULL);
+ if (ret) {
+ fuse_log(FUSE_LOG_ERR, "%s: Failed to join thread idx %d err"
+ " %d\n", __func__, qidx, ret);
+ }
+ close(ourqi->kill_fd);
}
pthread_mutex_destroy(&ourqi->vq_lock);
- close(ourqi->kill_fd);
ourqi->kick_fd = -1;
free(vud->qi[qidx]);
vud->qi[qidx] = NULL;
@@ -1034,7 +1062,7 @@ int virtio_session_mount(struct fuse_session *se)
se->virtio_dev = calloc(sizeof(struct fv_VuDev), 1);
se->virtio_dev->se = se;
pthread_rwlock_init(&se->virtio_dev->vu_dispatch_rwlock, NULL);
- vu_init(&se->virtio_dev->dev, 2, se->vu_socketfd, fv_panic, fv_set_watch,
+ vu_init(&se->virtio_dev->dev, 3, se->vu_socketfd, fv_panic, fv_set_watch,
fv_remove_watch, &fv_iface);
return 0;
diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
index 0f3c3c8711..95f9fe5c5c 100644
--- a/hw/virtio/vhost-user-fs-pci.c
+++ b/hw/virtio/vhost-user-fs-pci.c
@@ -44,7 +44,7 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
uint64_t totalsize;
if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
- vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
+ vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
}
qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index 455e97beea..5555fe9dbe 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -24,6 +24,10 @@
#include "exec/address-spaces.h"
#include "trace.h"
+static const int user_feature_bits[] = {
+ VIRTIO_FS_F_NOTIFICATION,
+};
+
uint64_t vhost_user_fs_slave_map(struct vhost_dev *dev, VhostUserFSSlaveMsg *sm,
int fd)
{
@@ -378,12 +382,23 @@ static void vuf_set_status(VirtIODevice *vdev, uint8_t status)
}
}
-static uint64_t vuf_get_features(VirtIODevice *vdev,
- uint64_t requested_features,
- Error **errp)
+static uint64_t vuf_get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp)
{
- /* No feature bits used yet */
- return requested_features;
+ VHostUserFS *fs = VHOST_USER_FS(vdev);
+
+ virtio_add_feature(&features, VIRTIO_FS_F_NOTIFICATION);
+
+ return vhost_get_features(&fs->vhost_dev, user_feature_bits, features);
+}
+
+static void vuf_set_features(VirtIODevice *vdev, uint64_t features)
+{
+ VHostUserFS *fs = VHOST_USER_FS(vdev);
+
+ if (virtio_has_feature(features, VIRTIO_FS_F_NOTIFICATION)) {
+ fs->notify_enabled = true;
+ }
}
static void vuf_handle_output(VirtIODevice *vdev, VirtQueue *vq)
@@ -515,13 +530,20 @@ static void vuf_device_realize(DeviceState *dev, Error **errp)
/* Hiprio queue */
virtio_add_queue(vdev, fs->conf.queue_size, vuf_handle_output);
+ /* Notification queue. Feature negotiation happens later. So at this
+ * point of time we don't know if driver will use notification queue
+ * or not.
+ */
+ virtio_add_queue(vdev, fs->conf.queue_size, vuf_handle_output);
+
/* Request queues */
for (i = 0; i < fs->conf.num_request_queues; i++) {
virtio_add_queue(vdev, fs->conf.queue_size, vuf_handle_output);
}
- /* 1 high prio queue, plus the number configured */
- fs->vhost_dev.nvqs = 1 + fs->conf.num_request_queues;
+ /* 1 high prio queue, 1 notification queue plus the number configured */
+ fs->vhost_dev.nvqs = 2 + fs->conf.num_request_queues;
+
fs->vhost_dev.vqs = g_new0(struct vhost_virtqueue, fs->vhost_dev.nvqs);
ret = vhost_dev_init(&fs->vhost_dev, &fs->vhost_user,
VHOST_BACKEND_TYPE_USER, 0);
@@ -584,6 +606,7 @@ static void vuf_class_init(ObjectClass *klass, void *data)
vdc->realize = vuf_device_realize;
vdc->unrealize = vuf_device_unrealize;
vdc->get_features = vuf_get_features;
+ vdc->set_features = vuf_set_features;
vdc->get_config = vuf_get_config;
vdc->set_status = vuf_set_status;
vdc->guest_notifier_mask = vuf_guest_notifier_mask;
diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
index 4e7be1f312..bd47e0da98 100644
--- a/include/hw/virtio/vhost-user-fs.h
+++ b/include/hw/virtio/vhost-user-fs.h
@@ -64,6 +64,7 @@ typedef struct {
/* Metadata version table */
size_t mdvt_size;
MemoryRegion mdvt;
+ bool notify_enabled;
} VHostUserFS;
/* Callbacks from the vhost-user code for slave commands */
diff --git a/include/standard-headers/linux/virtio_fs.h b/include/standard-headers/linux/virtio_fs.h
index 310210b7b6..9ee95f584f 100644
--- a/include/standard-headers/linux/virtio_fs.h
+++ b/include/standard-headers/linux/virtio_fs.h
@@ -8,6 +8,9 @@
#include "standard-headers/linux/virtio_config.h"
#include "standard-headers/linux/virtio_types.h"
+/* Feature bits */
+#define VIRTIO_FS_F_NOTIFICATION 0 /* Notification queue supported */
+
struct virtio_fs_config {
/* Filesystem name (UTF-8, not NUL-terminated, padded with NULs) */
uint8_t tag[36];
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/5] virtiofsd: Specify size of notification buffer using config space
2019-12-04 19:08 [PATCH v2 0/5] [RFC] virtiofsd, vhost-user-fs: Add support for notification queue Vivek Goyal
` (2 preceding siblings ...)
2019-12-04 19:08 ` [PATCH v2 3/5] virtiofd: Create a notification queue Vivek Goyal
@ 2019-12-04 19:08 ` Vivek Goyal
2019-12-04 19:08 ` [PATCH v2 5/5] virtiofsd: Implement blocking posix locks Vivek Goyal
4 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2019-12-04 19:08 UTC (permalink / raw)
To: virtio-fs, qemu-devel; +Cc: mszeredi, dgilbert, stefanha
Daemon specifies size of notification buffer needed and that should be done
using config space.
Only ->notify_buf_size value of config space comes from daemon. Rest of
it is filled by qemu device emulation code.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/fuse_virtio.c | 31 ++++++++++++++++++++++
hw/virtio/vhost-user-fs.c | 26 ++++++++++++++++++
include/hw/virtio/vhost-user-fs.h | 2 ++
include/standard-headers/linux/virtio_fs.h | 2 ++
4 files changed, 61 insertions(+)
diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
index b1eebcf054..94cf9b3791 100644
--- a/contrib/virtiofsd/fuse_virtio.c
+++ b/contrib/virtiofsd/fuse_virtio.c
@@ -869,6 +869,35 @@ static bool fv_queue_order(VuDev *dev, int qidx)
return false;
}
+static uint64_t fv_get_protocol_features(VuDev *dev)
+{
+ return 1ull << VHOST_USER_PROTOCOL_F_CONFIG;
+}
+
+static int fv_get_config(VuDev *dev, uint8_t *config, uint32_t len)
+{
+ struct virtio_fs_config fscfg = {};
+ unsigned notify_size, roundto = 64;
+ union fuse_notify_union {
+ struct fuse_notify_poll_wakeup_out wakeup_out;
+ struct fuse_notify_inval_inode_out inode_out;
+ struct fuse_notify_inval_entry_out entry_out;
+ struct fuse_notify_delete_out delete_out;
+ struct fuse_notify_store_out store_out;
+ struct fuse_notify_retrieve_out retrieve_out;
+ };
+
+ notify_size = sizeof(struct fuse_out_header) +
+ sizeof(union fuse_notify_union);
+ notify_size = ((notify_size + roundto)/roundto) * roundto;
+
+ fscfg.notify_buf_size = notify_size;
+ memcpy(config, &fscfg, len);
+ fuse_log(FUSE_LOG_DEBUG, "%s:Setting notify_buf_size=%d\n", __func__,
+ fscfg.notify_buf_size);
+ return 0;
+}
+
static const VuDevIface fv_iface = {
.get_features = fv_get_features,
.set_features = fv_set_features,
@@ -877,6 +906,8 @@ static const VuDevIface fv_iface = {
.queue_set_started = fv_queue_set_started,
.queue_is_processed_in_order = fv_queue_order,
+ .get_protocol_features = fv_get_protocol_features,
+ .get_config = fv_get_config,
};
/*
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index 5555fe9dbe..5a6d244b98 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -277,16 +277,40 @@ uint64_t vhost_user_fs_slave_io(struct vhost_dev *dev, VhostUserFSSlaveMsg *sm,
return (uint64_t)done;
}
+static int vhost_user_fs_handle_config_change(struct vhost_dev *dev)
+{
+ return 0;
+}
+
+const VhostDevConfigOps fs_ops = {
+ .vhost_dev_config_notifier = vhost_user_fs_handle_config_change,
+};
static void vuf_get_config(VirtIODevice *vdev, uint8_t *config)
{
VHostUserFS *fs = VHOST_USER_FS(vdev);
struct virtio_fs_config fscfg = {};
+ int ret;
+
+ /*
+ * As of now we only get notification buffer size from device. And that's
+ * needed only if notification queue is enabled.
+ */
+ if (fs->notify_enabled) {
+ ret = vhost_dev_get_config(&fs->vhost_dev, (uint8_t *)&fs->fscfg,
+ sizeof(struct virtio_fs_config));
+ if (ret < 0) {
+ error_report("vhost-user-fs: get device config space failed."
+ " ret=%d\n", ret);
+ return;
+ }
+ }
memcpy((char *)fscfg.tag, fs->conf.tag,
MIN(strlen(fs->conf.tag) + 1, sizeof(fscfg.tag)));
virtio_stl_p(vdev, &fscfg.num_request_queues, fs->conf.num_request_queues);
+ virtio_stl_p(vdev, &fscfg.notify_buf_size, fs->fscfg.notify_buf_size);
memcpy(config, &fscfg, sizeof(fscfg));
}
@@ -545,6 +569,8 @@ static void vuf_device_realize(DeviceState *dev, Error **errp)
fs->vhost_dev.nvqs = 2 + fs->conf.num_request_queues;
fs->vhost_dev.vqs = g_new0(struct vhost_virtqueue, fs->vhost_dev.nvqs);
+
+ vhost_dev_set_config_notifier(&fs->vhost_dev, &fs_ops);
ret = vhost_dev_init(&fs->vhost_dev, &fs->vhost_user,
VHOST_BACKEND_TYPE_USER, 0);
if (ret < 0) {
diff --git a/include/hw/virtio/vhost-user-fs.h b/include/hw/virtio/vhost-user-fs.h
index bd47e0da98..f667cc4b5a 100644
--- a/include/hw/virtio/vhost-user-fs.h
+++ b/include/hw/virtio/vhost-user-fs.h
@@ -14,6 +14,7 @@
#ifndef _QEMU_VHOST_USER_FS_H
#define _QEMU_VHOST_USER_FS_H
+#include "standard-headers/linux/virtio_fs.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-user.h"
@@ -58,6 +59,7 @@ typedef struct {
struct vhost_virtqueue *vhost_vqs;
struct vhost_dev vhost_dev;
VhostUserState vhost_user;
+ struct virtio_fs_config fscfg;
/*< public >*/
MemoryRegion cache;
diff --git a/include/standard-headers/linux/virtio_fs.h b/include/standard-headers/linux/virtio_fs.h
index 9ee95f584f..719216a262 100644
--- a/include/standard-headers/linux/virtio_fs.h
+++ b/include/standard-headers/linux/virtio_fs.h
@@ -17,6 +17,8 @@ struct virtio_fs_config {
/* Number of request queues */
uint32_t num_request_queues;
+ /* Size of notification buffer */
+ uint32_t notify_buf_size;
} QEMU_PACKED;
#define VIRTIO_FS_PCI_CACHE_BAR 2
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 5/5] virtiofsd: Implement blocking posix locks
2019-12-04 19:08 [PATCH v2 0/5] [RFC] virtiofsd, vhost-user-fs: Add support for notification queue Vivek Goyal
` (3 preceding siblings ...)
2019-12-04 19:08 ` [PATCH v2 4/5] virtiofsd: Specify size of notification buffer using config space Vivek Goyal
@ 2019-12-04 19:08 ` Vivek Goyal
4 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2019-12-04 19:08 UTC (permalink / raw)
To: virtio-fs, qemu-devel; +Cc: mszeredi, dgilbert, stefanha
As of now we don't support fcntl(F_SETLKW) and if we see one, we return
-EOPNOTSUPP.
Change that by accepting these requests and returning a reply immediately
asking caller to wait. Once lock is available, send a notification to
the waiter indicating lock is available.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/fuse_kernel.h | 7 +++
contrib/virtiofsd/fuse_lowlevel.c | 23 ++++++-
contrib/virtiofsd/fuse_lowlevel.h | 25 ++++++++
contrib/virtiofsd/fuse_virtio.c | 97 ++++++++++++++++++++++++++++--
contrib/virtiofsd/passthrough_ll.c | 49 ++++++++++++---
5 files changed, 185 insertions(+), 16 deletions(-)
diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
index 2bdc8b1c88..432eb14d14 100644
--- a/contrib/virtiofsd/fuse_kernel.h
+++ b/contrib/virtiofsd/fuse_kernel.h
@@ -444,6 +444,7 @@ enum fuse_notify_code {
FUSE_NOTIFY_STORE = 4,
FUSE_NOTIFY_RETRIEVE = 5,
FUSE_NOTIFY_DELETE = 6,
+ FUSE_NOTIFY_LOCK = 7,
FUSE_NOTIFY_CODE_MAX,
};
@@ -836,6 +837,12 @@ struct fuse_notify_retrieve_in {
uint64_t dummy4;
};
+struct fuse_notify_lock_out {
+ uint64_t unique;
+ int32_t error;
+ int32_t padding;
+};
+
/* Device ioctls: */
#define FUSE_DEV_IOC_CLONE _IOR(229, 0, uint32_t)
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index d4a42d9804..3d9c289510 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -183,7 +183,8 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
{
struct fuse_out_header out;
- if (error <= -1000 || error > 0) {
+ /* error = 1 has been used to signal client to wait for notificaiton */
+ if (error <= -1000 || error > 1) {
fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error);
error = -ERANGE;
}
@@ -291,6 +292,12 @@ int fuse_reply_err(fuse_req_t req, int err)
return send_reply(req, -err, NULL, 0);
}
+int fuse_reply_wait(fuse_req_t req)
+{
+ /* TODO: This is a hack. Fix it */
+ return send_reply(req, 1, NULL, 0);
+}
+
void fuse_reply_none(fuse_req_t req)
{
fuse_free_req(req);
@@ -2207,6 +2214,20 @@ static int send_notify_iov(struct fuse_session *se, int notify_code,
return fuse_send_msg(se, NULL, iov, count);
}
+int fuse_lowlevel_notify_lock(struct fuse_session *se, uint64_t unique,
+ int32_t error)
+{
+ struct fuse_notify_lock_out outarg = {0};
+ struct iovec iov[2];
+
+ outarg.unique = unique;
+ outarg.error = -error;
+
+ iov[1].iov_base = &outarg;
+ iov[1].iov_len = sizeof(outarg);
+ return send_notify_iov(se, FUSE_NOTIFY_LOCK, iov, 2);
+}
+
int fuse_lowlevel_notify_poll(struct fuse_pollhandle *ph)
{
if (ph != NULL) {
diff --git a/contrib/virtiofsd/fuse_lowlevel.h b/contrib/virtiofsd/fuse_lowlevel.h
index e664d2d12d..4126b4f967 100644
--- a/contrib/virtiofsd/fuse_lowlevel.h
+++ b/contrib/virtiofsd/fuse_lowlevel.h
@@ -1251,6 +1251,22 @@ struct fuse_lowlevel_ops {
*/
int fuse_reply_err(fuse_req_t req, int err);
+/**
+ * Ask caller to wait for lock.
+ *
+ * Possible requests:
+ * setlkw
+ *
+ * If caller sends a blocking lock request (setlkw), then reply to caller
+ * that wait for lock to be available. Once lock is available caller will
+ * receive a notification with request's unique id. Notification will
+ * carry info whether lock was successfully obtained or not.
+ *
+ * @param req request handle
+ * @return zero for success, -errno for failure to send reply
+ */
+int fuse_reply_wait(fuse_req_t req);
+
/**
* Don't send reply
*
@@ -1704,6 +1720,15 @@ int fuse_lowlevel_notify_delete(struct fuse_session *se,
int fuse_lowlevel_notify_store(struct fuse_session *se, fuse_ino_t ino,
off_t offset, struct fuse_bufvec *bufv,
enum fuse_buf_copy_flags flags);
+/**
+ * Notify event related to previous lock request
+ *
+ * @param se the session object
+ * @param unique the unique id of the request which requested setlkw
+ * @param error zero for success, -errno for the failure
+ */
+int fuse_lowlevel_notify_lock(struct fuse_session *se, uint64_t unique,
+ int32_t error);
/* ----------------------------------------------------------- *
* Utility functions *
diff --git a/contrib/virtiofsd/fuse_virtio.c b/contrib/virtiofsd/fuse_virtio.c
index 94cf9b3791..129dd329f6 100644
--- a/contrib/virtiofsd/fuse_virtio.c
+++ b/contrib/virtiofsd/fuse_virtio.c
@@ -208,6 +208,83 @@ static void copy_iov(struct iovec *src_iov, int src_count,
}
}
+static int virtio_send_notify_msg(struct fuse_session *se, struct iovec *iov,
+ int count)
+{
+ struct fv_QueueInfo *qi;
+ VuDev *dev = &se->virtio_dev->dev;
+ VuVirtq *q;
+ FVRequest *req;
+ VuVirtqElement *elem;
+ unsigned int in_num;
+ struct fuse_out_header *out = iov[0].iov_base;
+ size_t in_len, tosend_len = iov_size(iov, count);
+ struct iovec *in_sg;
+ int ret = 0;
+
+ /* Notifications have unique == 0 */
+ assert (!out->unique);
+
+ if (!se->notify_enabled)
+ return -EOPNOTSUPP;
+
+ /* If notifications are enabled, queue index 1 is notification queue */
+ qi = se->virtio_dev->qi[1];
+ q = vu_get_queue(dev, qi->qidx);
+
+ pthread_rwlock_rdlock(&qi->virtio_dev->vu_dispatch_rwlock);
+ pthread_mutex_lock(&qi->vq_lock);
+ /* Pop an element from queue */
+ req = vu_queue_pop(dev, q, sizeof(FVRequest), NULL, NULL);
+ if (!req) {
+ /* TODO: Implement some sort of ring buffer and queue notifications
+ * on that and send these later when notification queue has space
+ * available.
+ */
+ ret = -ENOSPC;
+ }
+ pthread_mutex_unlock(&qi->vq_lock);
+ pthread_rwlock_unlock(&qi->virtio_dev->vu_dispatch_rwlock);
+
+ if (ret)
+ return ret;
+
+ out->len = tosend_len;
+ elem = &req->elem;
+ in_num = elem->in_num;
+ in_sg = elem->in_sg;
+ in_len = iov_size(in_sg, in_num);
+ fuse_log(FUSE_LOG_DEBUG, "%s: elem %d: with %d in desc of length %zd\n",
+ __func__, elem->index, in_num, in_len);
+
+ if (in_len < sizeof(struct fuse_out_header)) {
+ fuse_log(FUSE_LOG_ERR, "%s: elem %d too short for out_header\n",
+ __func__, elem->index);
+ ret = -E2BIG;
+ goto out;
+ }
+
+ if (in_len < tosend_len) {
+ fuse_log(FUSE_LOG_ERR, "%s: elem %d too small for data len"
+ " %zd\n", __func__, elem->index, tosend_len);
+ ret = -E2BIG;
+ goto out;
+ }
+
+ /* First copy the header data from iov->in_sg */
+ copy_iov(iov, count, in_sg, in_num, tosend_len);
+
+ pthread_rwlock_rdlock(&qi->virtio_dev->vu_dispatch_rwlock);
+ pthread_mutex_lock(&qi->vq_lock);
+ vu_queue_push(dev, q, elem, tosend_len);
+ vu_queue_notify(dev, q);
+ pthread_mutex_unlock(&qi->vq_lock);
+ pthread_rwlock_unlock(&qi->virtio_dev->vu_dispatch_rwlock);
+out:
+ free(req);
+ return ret;
+}
+
/*
* Called back by ll whenever it wants to send a reply/message back
* The 1st element of the iov starts with the fuse_out_header
@@ -216,11 +293,11 @@ static void copy_iov(struct iovec *src_iov, int src_count,
int virtio_send_msg(struct fuse_session *se, struct fuse_chan *ch,
struct iovec *iov, int count)
{
- FVRequest *req = container_of(ch, FVRequest, ch);
- struct fv_QueueInfo *qi = ch->qi;
+ FVRequest *req;
+ struct fv_QueueInfo *qi;
VuDev *dev = &se->virtio_dev->dev;
- VuVirtq *q = vu_get_queue(dev, qi->qidx);
- VuVirtqElement *elem = &req->elem;
+ VuVirtq *q;
+ VuVirtqElement *elem;
int ret = 0;
assert(count >= 1);
@@ -231,8 +308,15 @@ int virtio_send_msg(struct fuse_session *se, struct fuse_chan *ch,
size_t tosend_len = iov_size(iov, count);
- /* unique == 0 is notification, which we don't support */
- assert(out->unique);
+ /* unique == 0 is notification */
+ if (!out->unique)
+ return virtio_send_notify_msg(se, iov, count);
+
+ assert(ch);
+ req = container_of(ch, FVRequest, ch);
+ elem = &req->elem;
+ qi = ch->qi;
+ q = vu_get_queue(dev, qi->qidx);
assert(!req->reply_sent);
/* The 'in' part of the elem is to qemu */
@@ -885,6 +969,7 @@ static int fv_get_config(VuDev *dev, uint8_t *config, uint32_t len)
struct fuse_notify_delete_out delete_out;
struct fuse_notify_store_out store_out;
struct fuse_notify_retrieve_out retrieve_out;
+ struct fuse_notify_lock_out lock_out;
};
notify_size = sizeof(struct fuse_out_header) +
diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
index 6aa56882e8..308fc76530 100644
--- a/contrib/virtiofsd/passthrough_ll.c
+++ b/contrib/virtiofsd/passthrough_ll.c
@@ -1926,7 +1926,10 @@ static void lo_setlk(fuse_req_t req, fuse_ino_t ino,
struct lo_data *lo = lo_data(req);
struct lo_inode *inode;
struct lo_inode_plock *plock;
- int ret, saverr = 0;
+ int ret, saverr = 0, ofd;
+ uint64_t unique;
+ struct fuse_session *se = req->se;
+ bool async_lock = false;
fuse_log(FUSE_LOG_DEBUG, "lo_setlk(ino=%" PRIu64 ", flags=%d)"
" cmd=%d pid=%d owner=0x%lx sleep=%d l_whence=%d"
@@ -1934,11 +1937,6 @@ static void lo_setlk(fuse_req_t req, fuse_ino_t ino,
lock->l_type, lock->l_pid, fi->lock_owner, sleep,
lock->l_whence, lock->l_start, lock->l_len);
- if (sleep) {
- fuse_reply_err(req, EOPNOTSUPP);
- return;
- }
-
inode = lo_inode(req, ino);
if (!inode) {
fuse_reply_err(req, EBADF);
@@ -1951,21 +1949,54 @@ static void lo_setlk(fuse_req_t req, fuse_ino_t ino,
if (!plock) {
saverr = ret;
+ pthread_mutex_unlock(&inode->plock_mutex);
goto out;
}
+ /*
+ * plock is now released when inode is going away. We already have
+ * a reference on inode, so it is guaranteed that plock->fd is
+ * still around even after dropping inode->plock_mutex lock
+ */
+ ofd = plock->fd;
+ pthread_mutex_unlock(&inode->plock_mutex);
+
+ /*
+ * If this lock request can block, request caller to wait for
+ * notification. Do not access req after this. Once lock is
+ * available, send a notification instead.
+ */
+ if (sleep && lock->l_type != F_UNLCK) {
+ /*
+ * If notification queue is not enabled, can't support async
+ * locks.
+ */
+ if (!se->notify_enabled) {
+ saverr = EOPNOTSUPP;
+ goto out;
+ }
+ async_lock = true;
+ unique = req->unique;
+ fuse_reply_wait(req);
+ }
/* TODO: Is it alright to modify flock? */
lock->l_pid = 0;
- ret = fcntl(plock->fd, F_OFD_SETLK, lock);
+ if (async_lock)
+ ret = fcntl(ofd, F_OFD_SETLKW, lock);
+ else
+ ret = fcntl(ofd, F_OFD_SETLK, lock);
if (ret == -1) {
saverr = errno;
}
out:
- pthread_mutex_unlock(&inode->plock_mutex);
lo_inode_put(lo, &inode);
- fuse_reply_err(req, saverr);
+ if (!async_lock)
+ fuse_reply_err(req, saverr);
+ else {
+ fuse_lowlevel_notify_lock(se, unique, saverr);
+ }
}
static void lo_fsyncdir(fuse_req_t req, fuse_ino_t ino, int datasync,
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread