[Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Venkateswararao Jujjuri (JV)]

VirtFS (fileserver base on 9P) performs many blocking system calls in the 
vCPU context. This effort is to move the blocking calls out of vCPU/IO 
thread context, into asynchronous threads.

Anthony's " Add hard build dependency on glib" patch and 
Kevin/Stefan's coroutine effort is a prerequisite.

This patch set contains:
 - Converting all 9pfs calls into coroutines. 
 - Each 9P operation will be modified for:
    - Remove post* functions. These are our call back functions which makes 
      the code very hard to read. Now with coroutines, we can achieve the same 
      state machine model with nice sequential code flow.
    - Move errno access near to the local_syscall()
    - Introduce asynchronous threading

This series has the basic infrastructure and few routines like 
mkdir,monod,unlink,readdir,xattr,lstat, etc converted. 
Currently we are working on converting and testing other 9P operations also 
into this model and those patches will follow shortly.

Removing callback functions made some of the patches little lengthy. 
Here is the git tree for the reviewer convenience. 

http://repo.or.cz/w/qemu/aliguori/jvrao.git/shortlog/refs/heads/9p-coroutines-round1 

-Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>

[Qemu-devel] [PATCH 01/25] [virtio-9p] Add infrastructure to support glib threads and coroutines.

[Venkateswararao Jujjuri (JV)]

This patch is originally made by Arun Bharadwaj for glib support.
Later Harsh Prateek Bora added coroutines support.

Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 Makefile.objs              |    2 +
 hw/9pfs/virtio-9p-coth.c   |   68 ++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h   |   45 +++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-device.c |   26 +++++++++++++++-
 hw/9pfs/virtio-9p.h        |    1 -
 5 files changed, 139 insertions(+), 3 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-coth.c
 create mode 100644 hw/9pfs/virtio-9p-coth.h

diff --git a/Makefile.objs b/Makefile.objs
index 3873f10..96f6a24 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -297,8 +297,10 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
+$(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
 
 
 ######################################################################
diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
new file mode 100644
index 0000000..edd3cde
--- /dev/null
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -0,0 +1,68 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
+ *  Venkateswararao Jujjuri(JV) <jvrao@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "fsdev/qemu-fsdev.h"
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include "virtio-9p-coth.h"
+
+/* v9fs glib thread pool */
+V9fsThPool v9fs_pool;
+
+void v9fs_qemu_submit_request(V9fsRequest *req)
+{
+    V9fsThPool *p = &v9fs_pool;
+
+    req->done = 0;
+    p->requests = g_list_append(p->requests, req);
+    g_thread_pool_push(v9fs_pool.pool, req, NULL);
+}
+
+void v9fs_qemu_process_req_done(void *arg)
+{
+    struct V9fsThPool *p = &v9fs_pool;
+    char byte;
+    ssize_t len;
+    GList *cur_req, *next_req;
+
+    do {
+        len = read(p->rfd, &byte, sizeof(byte));
+    } while (len == -1 && errno == EINTR);
+
+    for (cur_req = p->requests; cur_req != NULL; cur_req = next_req) {
+        V9fsRequest *req = cur_req->data;
+        next_req = g_list_next(cur_req);
+
+        if (!req->done) {
+            continue;
+        }
+
+        Coroutine *entry = req->coroutine;
+        qemu_coroutine_enter(entry, NULL);
+
+        p->requests = g_list_delete_link(p->requests, cur_req);
+    }
+}
+
+void v9fs_thread_routine(gpointer data, gpointer user_data)
+{
+    V9fsRequest *req = data;
+    char byte = 0;
+    ssize_t len;
+    req->func(req);
+    req->done = 1;
+    do {
+        len = write(v9fs_pool.wfd, &byte, sizeof(byte));
+    } while (len == -1 && errno == EINTR);
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
new file mode 100644
index 0000000..fdc44f6
--- /dev/null
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -0,0 +1,45 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
+ *  Venkateswararao Jujjuri(JV) <jvrao@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef _QEMU_VIRTIO_9P_COTH_H
+#define _QEMU_VIRTIO_9P_COTH_H
+
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include <glib.h>
+
+typedef struct V9fsRequest {
+    void (*func)(struct V9fsRequest *req);
+
+    /* Flag to indicate that request is satisfied, ready for post-processing */
+    int done;
+    Coroutine *coroutine;
+} V9fsRequest;
+
+typedef struct V9fsThPool {
+    GThreadPool *pool;
+    GList *requests;
+    int rfd;
+    int wfd;
+} V9fsThPool;
+
+/* v9fs glib thread pool */
+extern V9fsThPool v9fs_pool;
+
+extern void v9fs_thread_routine(gpointer data, gpointer user_data);
+extern void v9fs_qemu_process_req_done(void *arg);
+extern void v9fs_qemu_submit_request(V9fsRequest *req);
+
+
+#endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index a2b6acc..21fb310 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -18,6 +18,9 @@
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-xattr.h"
+#include "virtio-9p-coth.h"
+
+static int notifier_fds[2];
 
 static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
 {
@@ -49,14 +52,13 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     int i, len;
     struct stat stat;
     FsTypeEntry *fse;
-
+    V9fsThPool *p = &v9fs_pool;
 
     s = (V9fsState *)virtio_common_init("virtio-9p",
                                     VIRTIO_ID_9P,
                                     sizeof(struct virtio_9p_config)+
                                     MAX_TAG_LEN,
                                     sizeof(V9fsState));
-
     /* initialize pdu allocator */
     QLIST_INIT(&s->free_list);
     for (i = 0; i < (MAX_REQ - 1); i++) {
@@ -132,6 +134,26 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
                         s->tag_len;
     s->vdev.get_config = virtio_9p_get_config;
 
+    if (!g_thread_supported()) {
+        g_thread_init(NULL);
+    }
+
+    if (qemu_pipe(notifier_fds) == -1) {
+        fprintf(stderr, "failed to create fd's for virtio-9p\n");
+        exit(1);
+    }
+
+    p->pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL);
+    p->rfd = notifier_fds[0];
+    p->wfd = notifier_fds[1];
+
+    fcntl(p->rfd, F_SETFL, O_NONBLOCK);
+    fcntl(p->wfd, F_SETFL, O_NONBLOCK);
+
+    qemu_set_fd_handler(p->rfd, v9fs_qemu_process_req_done, NULL, NULL);
+
+    (void) v9fs_qemu_submit_request;
+
     return &s->vdev;
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 2bfbe62..699d3ab 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -510,5 +510,4 @@ static inline size_t do_pdu_unpack(void *dst, struct iovec *sg, int sg_count,
 }
 
 extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
-
 #endif
-- 
1.7.1

Re: [Qemu-devel] [PATCH 01/25] [virtio-9p] Add infrastructure to support glib threads and coroutines.

[Stefan Hajnoczi]

On Thu, May 12, 2011 at 9:57 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> +/* v9fs glib thread pool */
> +V9fsThPool v9fs_pool;

This should be static and an init function in this file could perform
initialization.  Right now the initialization is inlined in
virtio-9p-device.c.

> +void v9fs_qemu_submit_request(V9fsRequest *req)
> +{
> +    V9fsThPool *p = &v9fs_pool;
> +
> +    req->done = 0;
> +    p->requests = g_list_append(p->requests, req);
> +    g_thread_pool_push(v9fs_pool.pool, req, NULL);
> +}
> +
> +void v9fs_qemu_process_req_done(void *arg)
> +{
> +    struct V9fsThPool *p = &v9fs_pool;
> +    char byte;
> +    ssize_t len;
> +    GList *cur_req, *next_req;
> +
> +    do {
> +        len = read(p->rfd, &byte, sizeof(byte));
> +    } while (len == -1 && errno == EINTR);
> +
> +    for (cur_req = p->requests; cur_req != NULL; cur_req = next_req) {
> +        V9fsRequest *req = cur_req->data;
> +        next_req = g_list_next(cur_req);
> +
> +        if (!req->done) {
> +            continue;
> +        }

The requests list is only used for completion, why not enqueue only
completed requests and get rid of the done field.  Then this function
doesn't have to skip over in-flight requests.

> +        Coroutine *entry = req->coroutine;
> +        qemu_coroutine_enter(entry, NULL);
> +
> +        p->requests = g_list_delete_link(p->requests, cur_req);

Does this actually free the req?

> +static int notifier_fds[2];

Why global?

> +    if (!g_thread_supported()) {
> +        g_thread_init(NULL);
> +    }

The logic looks inverted.  But if GThread isn't supported where in big
trouble so we should probably exit?

Stefan

Re: [Qemu-devel] [PATCH 01/25] [virtio-9p] Add infrastructure to support glib threads and coroutines.

[Venkateswararao Jujjuri]

On 05/12/2011 10:48 PM, Stefan Hajnoczi wrote:
> On Thu, May 12, 2011 at 9:57 PM, Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com>  wrote:
>> +/* v9fs glib thread pool */
>> +V9fsThPool v9fs_pool;
> This should be static and an init function in this file could perform
> initialization.  Right now the initialization is inlined in
> virtio-9p-device.c.
>
>> +void v9fs_qemu_submit_request(V9fsRequest *req)
>> +{
>> +    V9fsThPool *p =&v9fs_pool;
>> +
>> +    req->done = 0;
>> +    p->requests = g_list_append(p->requests, req);
>> +    g_thread_pool_push(v9fs_pool.pool, req, NULL);
>> +}
>> +
>> +void v9fs_qemu_process_req_done(void *arg)
>> +{
>> +    struct V9fsThPool *p =&v9fs_pool;
>> +    char byte;
>> +    ssize_t len;
>> +    GList *cur_req, *next_req;
>> +
>> +    do {
>> +        len = read(p->rfd,&byte, sizeof(byte));
>> +    } while (len == -1&&  errno == EINTR);
>> +
>> +    for (cur_req = p->requests; cur_req != NULL; cur_req = next_req) {
>> +        V9fsRequest *req = cur_req->data;
>> +        next_req = g_list_next(cur_req);
>> +
>> +        if (!req->done) {
>> +            continue;
>> +        }
> The requests list is only used for completion, why not enqueue only
> completed requests and get rid of the done field.  Then this function
> doesn't have to skip over in-flight requests.
>
>> +        Coroutine *entry = req->coroutine;
>> +        qemu_coroutine_enter(entry, NULL);
>> +
>> +        p->requests = g_list_delete_link(p->requests, cur_req);
> Does this actually free the req?
>
>> +static int notifier_fds[2];
> Why global?
>
>> +    if (!g_thread_supported()) {
>> +        g_thread_init(NULL);
>> +    }
> The logic looks inverted.  But if GThread isn't supported where in big
> trouble so we should probably exit?
>
Name of the function is little weird .. it basically checking if the 
thread infrastructure
is initialized or not. One can call g_thread_init() unconditionally.. 
but this may be good
practice.

http://developer.gnome.org/glib/2.28/glib-Threads.html#g-thread-supported

We will address all your comments. Thanks for the review.

- JV

> Stefan

Re: [Qemu-devel] [PATCH 01/25] [virtio-9p] Add infrastructure to support glib threads and coroutines.

[Stefan Hajnoczi]

On Fri, May 13, 2011 at 4:47 PM, Venkateswararao Jujjuri
<jvrao@linux.vnet.ibm.com> wrote:
> On 05/12/2011 10:48 PM, Stefan Hajnoczi wrote:
>>
>> On Thu, May 12, 2011 at 9:57 PM, Venkateswararao Jujjuri (JV)
>> <jvrao@linux.vnet.ibm.com>  wrote:
>>>
>>> +/* v9fs glib thread pool */
>>> +V9fsThPool v9fs_pool;
>>
>> This should be static and an init function in this file could perform
>> initialization.  Right now the initialization is inlined in
>> virtio-9p-device.c.
>>
>>> +void v9fs_qemu_submit_request(V9fsRequest *req)
>>> +{
>>> +    V9fsThPool *p =&v9fs_pool;
>>> +
>>> +    req->done = 0;
>>> +    p->requests = g_list_append(p->requests, req);
>>> +    g_thread_pool_push(v9fs_pool.pool, req, NULL);
>>> +}
>>> +
>>> +void v9fs_qemu_process_req_done(void *arg)
>>> +{
>>> +    struct V9fsThPool *p =&v9fs_pool;
>>> +    char byte;
>>> +    ssize_t len;
>>> +    GList *cur_req, *next_req;
>>> +
>>> +    do {
>>> +        len = read(p->rfd,&byte, sizeof(byte));
>>> +    } while (len == -1&&  errno == EINTR);
>>> +
>>> +    for (cur_req = p->requests; cur_req != NULL; cur_req = next_req) {
>>> +        V9fsRequest *req = cur_req->data;
>>> +        next_req = g_list_next(cur_req);
>>> +
>>> +        if (!req->done) {
>>> +            continue;
>>> +        }
>>
>> The requests list is only used for completion, why not enqueue only
>> completed requests and get rid of the done field.  Then this function
>> doesn't have to skip over in-flight requests.
>>
>>> +        Coroutine *entry = req->coroutine;
>>> +        qemu_coroutine_enter(entry, NULL);
>>> +
>>> +        p->requests = g_list_delete_link(p->requests, cur_req);
>>
>> Does this actually free the req?
>>
>>> +static int notifier_fds[2];
>>
>> Why global?
>>
>>> +    if (!g_thread_supported()) {
>>> +        g_thread_init(NULL);
>>> +    }
>>
>> The logic looks inverted.  But if GThread isn't supported where in big
>> trouble so we should probably exit?
>>
> Name of the function is little weird .. it basically checking if the thread
> infrastructure
> is initialized or not. One can call g_thread_init() unconditionally.. but
> this may be good
> practice.
>
> http://developer.gnome.org/glib/2.28/glib-Threads.html#g-thread-supported

Thanks I was just checking it out and noticed they a function with a
clearer name since 2.20:
http://developer.gnome.org/glib/2.28/glib-Threads.html#g-thread-get-initialized

Stefan

[Qemu-devel] [PATCH 02/25] [virtio-9p] Change all pdu handlers to coroutines.

[Venkateswararao Jujjuri (JV)]

This patch changes the top level handlers to coroutines and sets the base.
It will be followed up with series of patches to convert all filesystem
calls to threaded coroutines pushing all blocking clals in VirtFS out
of vcpu threads.

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-coth.h |    7 ++
 hw/9pfs/virtio-9p.c      |  194 ++++++++++++++++++++++++++++++++++++----------
 hw/9pfs/virtio-9p.h      |    2 +-
 3 files changed, 161 insertions(+), 42 deletions(-)

diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index fdc44f6..2ec1401 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -17,6 +17,7 @@
 
 #include "qemu-thread.h"
 #include "qemu-coroutine.h"
+#include "virtio-9p.h"
 #include <glib.h>
 
 typedef struct V9fsRequest {
@@ -34,6 +35,12 @@ typedef struct V9fsThPool {
     int wfd;
 } V9fsThPool;
 
+typedef struct V9fsCoPdu {
+    V9fsPDU *pdu;
+    V9fsState *s;
+    Coroutine *coroutine;
+} V9fsCoPdu;
+
 /* v9fs glib thread pool */
 extern V9fsThPool v9fs_pool;
 
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index ec97b10..e308e9b 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -19,6 +19,7 @@
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-debug.h"
 #include "virtio-9p-xattr.h"
+#include "virtio-9p-coth.h"
 
 int debug_9p_pdu;
 
@@ -1192,8 +1193,11 @@ static void v9fs_fix_path(V9fsString *dst, V9fsString *src, int len)
     v9fs_string_free(&str);
 }
 
-static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_version(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     V9fsString version;
     size_t offset = 7;
 
@@ -1211,10 +1215,15 @@ static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
     complete_pdu(s, pdu, offset);
 
     v9fs_string_free(&version);
+    qemu_free(copdu);
+    return;
 }
 
-static void v9fs_attach(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_attach(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid, afid, n_uname;
     V9fsString uname, aname;
     V9fsFidState *fidp;
@@ -1247,6 +1256,7 @@ out:
     complete_pdu(s, pdu, err);
     v9fs_string_free(&uname);
     v9fs_string_free(&aname);
+    qemu_free(copdu);
 }
 
 static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int err)
@@ -1269,8 +1279,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_stat(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_stat(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsStatState *vs;
     ssize_t err = 0;
@@ -1297,6 +1310,7 @@ out:
     complete_pdu(s, vs->pdu, err);
     v9fs_stat_free(&vs->v9stat);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
@@ -1316,8 +1330,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_getattr(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsStatStateDotl *vs;
     ssize_t err = 0;
@@ -1348,6 +1365,7 @@ static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 /* From Linux kernel code */
@@ -1465,8 +1483,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_setattr(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_setattr(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsSetattrState *vs;
     int err = 0;
@@ -1493,6 +1514,7 @@ static void v9fs_setattr(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
@@ -1579,8 +1601,11 @@ out:
     v9fs_walk_complete(s, vs, err);
 }
 
-static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_walk(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid, newfid;
     V9fsWalkState *vs;
     int err = 0;
@@ -1658,6 +1683,7 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
     err = vs->offset;
 out:
     v9fs_walk_complete(s, vs, err);
+    qemu_free(copdu);
 }
 
 static int32_t get_iounit(V9fsState *s, V9fsString *name)
@@ -1751,8 +1777,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_open(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_open(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsOpenState *vs;
     ssize_t err = 0;
@@ -1783,6 +1812,7 @@ static void v9fs_open(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_post_lcreate(V9fsState *s, V9fsLcreateState *vs, int err)
@@ -1836,8 +1866,11 @@ out:
     v9fs_post_lcreate(s, vs, err);
 }
 
-static void v9fs_lcreate(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_lcreate(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t dfid, flags, mode;
     gid_t gid;
     V9fsLcreateState *vs;
@@ -1873,6 +1906,7 @@ out:
     complete_pdu(s, vs->pdu, err);
     v9fs_string_free(&vs->name);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU *pdu, int err)
@@ -1883,8 +1917,11 @@ static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU *pdu, int err)
     complete_pdu(s, pdu, err);
 }
 
-static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_fsync(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     size_t offset = 7;
     V9fsFidState *fidp;
@@ -1900,10 +1937,14 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
     }
     err = v9fs_do_fsync(s, fidp->fs.fd, datasync);
     v9fs_post_do_fsync(s, pdu, err);
+    qemu_free(copdu);
 }
 
-static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_clunk(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     size_t offset = 7;
     int err;
@@ -1919,6 +1960,7 @@ static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu)
     err = offset;
 out:
     complete_pdu(s, pdu, err);
+    qemu_free(copdu);
 }
 
 static void v9fs_read_post_readdir(V9fsState *, V9fsReadState *, ssize_t);
@@ -2068,8 +2110,11 @@ static void v9fs_xattr_read(V9fsState *s, V9fsReadState *vs)
     qemu_free(vs);
 }
 
-static void v9fs_read(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_read(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsReadState *vs;
     ssize_t err = 0;
@@ -2120,6 +2165,7 @@ static void v9fs_read(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 typedef struct V9fsReadDirState {
@@ -2207,8 +2253,11 @@ static void v9fs_readdir_post_setdir(V9fsState *s, V9fsReadDirState *vs)
     return;
 }
 
-static void v9fs_readdir(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_readdir(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsReadDirState *vs;
     ssize_t err = 0;
@@ -2240,7 +2289,7 @@ static void v9fs_readdir(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, pdu, err);
     qemu_free(vs);
-    return;
+    qemu_free(copdu);
 }
 
 static void v9fs_write_post_pwritev(V9fsState *s, V9fsWriteState *vs,
@@ -2319,8 +2368,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_write(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_write(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsWriteState *vs;
     ssize_t err;
@@ -2370,6 +2422,7 @@ static void v9fs_write(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_create_post_getiounit(V9fsState *s, V9fsCreateState *vs)
@@ -2553,8 +2606,11 @@ out:
     v9fs_post_create(s, vs, err);
 }
 
-static void v9fs_create(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_create(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsCreateState *vs;
     int err = 0;
@@ -2586,6 +2642,7 @@ out:
     v9fs_string_free(&vs->name);
     v9fs_string_free(&vs->extension);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_post_symlink(V9fsState *s, V9fsSymlinkState *vs, int err)
@@ -2615,8 +2672,11 @@ out:
     v9fs_post_symlink(s, vs, err);
 }
 
-static void v9fs_symlink(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_symlink(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t dfid;
     V9fsSymlinkState *vs;
     int err = 0;
@@ -2649,16 +2709,25 @@ out:
     v9fs_string_free(&vs->name);
     v9fs_string_free(&vs->symname);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
-static void v9fs_flush(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_flush(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     /* A nop call with no return */
     complete_pdu(s, pdu, 7);
+    qemu_free(copdu);
+    return;
 }
 
-static void v9fs_link(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_link(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t dfid, oldfid;
     V9fsFidState *dfidp, *oldfidp;
     V9fsString name, fullname;
@@ -2692,6 +2761,7 @@ static void v9fs_link(V9fsState *s, V9fsPDU *pdu)
 out:
     v9fs_string_free(&name);
     complete_pdu(s, pdu, err);
+    qemu_free(copdu);
 }
 
 static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
@@ -2710,8 +2780,11 @@ static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
     qemu_free(vs);
 }
 
-static void v9fs_remove(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_remove(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsRemoveState *vs;
     int err = 0;
@@ -2735,6 +2808,7 @@ static void v9fs_remove(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_wstat_post_truncate(V9fsState *s, V9fsWstatState *vs, int err)
@@ -2877,8 +2951,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_rename(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_rename(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsRenameState *vs;
     ssize_t err = 0;
@@ -2903,6 +2980,7 @@ static void v9fs_rename(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_wstat_post_utime(V9fsState *s, V9fsWstatState *vs, int err)
@@ -3002,8 +3080,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_wstat(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsWstatState *vs;
     int err = 0;
@@ -3040,6 +3121,7 @@ out:
     v9fs_stat_free(&vs->v9stat);
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err)
@@ -3087,8 +3169,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_statfs(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_statfs(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     V9fsStatfsState *vs;
     ssize_t err = 0;
 
@@ -3113,6 +3198,8 @@ static void v9fs_statfs(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
+    return;
 }
 
 static void v9fs_mknod_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
@@ -3149,8 +3236,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_mknod(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_mknod(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsMkState *vs;
     int err = 0;
@@ -3184,6 +3274,7 @@ out:
     v9fs_string_free(&vs->fullname);
     v9fs_string_free(&vs->name);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 /*
@@ -3195,8 +3286,11 @@ out:
  * So when a TLOCK request comes, always return success
  */
 
-static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_lock(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid, err = 0;
     V9fsLockState *vs;
 
@@ -3233,6 +3327,7 @@ out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs->flock);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 /*
@@ -3240,8 +3335,11 @@ out:
  * handling is done by client's VFS layer.
  */
 
-static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_getlock(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid, err = 0;
     V9fsGetlockState *vs;
 
@@ -3273,6 +3371,7 @@ out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs->glock);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_mkdir_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
@@ -3309,8 +3408,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_mkdir(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_mkdir(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsMkState *vs;
     int err = 0;
@@ -3342,6 +3444,7 @@ out:
     v9fs_string_free(&vs->fullname);
     v9fs_string_free(&vs->name);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_post_xattr_getvalue(V9fsState *s, V9fsXattrState *vs, int err)
@@ -3433,8 +3536,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_xattrwalk(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_xattrwalk(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     ssize_t err = 0;
     V9fsXattrState *vs;
     int32_t fid, newfid;
@@ -3485,10 +3591,14 @@ out:
     complete_pdu(s, vs->pdu, err);
     v9fs_string_free(&vs->name);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
-static void v9fs_xattrcreate(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_xattrcreate(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int flags;
     int32_t fid;
     ssize_t err = 0;
@@ -3524,6 +3634,7 @@ out:
     complete_pdu(s, vs->pdu, err);
     v9fs_string_free(&vs->name);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
 static void v9fs_readlink_post_readlink(V9fsState *s, V9fsReadLinkState *vs,
@@ -3541,8 +3652,11 @@ out:
     qemu_free(vs);
 }
 
-static void v9fs_readlink(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_readlink(void *opaque)
 {
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
     V9fsReadLinkState *vs;
     int err = 0;
@@ -3567,11 +3681,10 @@ static void v9fs_readlink(V9fsState *s, V9fsPDU *pdu)
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
+    qemu_free(copdu);
 }
 
-typedef void (pdu_handler_t)(V9fsState *s, V9fsPDU *pdu);
-
-static pdu_handler_t *pdu_handlers[] = {
+static CoroutineEntry *pdu_co_handlers[] = {
     [P9_TREADDIR] = v9fs_readdir,
     [P9_TSTATFS] = v9fs_statfs,
     [P9_TGETATTR] = v9fs_getattr,
@@ -3608,18 +3721,17 @@ static pdu_handler_t *pdu_handlers[] = {
 
 static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
 {
-    pdu_handler_t *handler;
-
+    V9fsCoPdu *copdu;
     if (debug_9p_pdu) {
         pprint_pdu(pdu);
     }
-
-    BUG_ON(pdu->id >= ARRAY_SIZE(pdu_handlers));
-
-    handler = pdu_handlers[pdu->id];
-    BUG_ON(handler == NULL);
-
-    handler(s, pdu);
+    BUG_ON(pdu->id >= ARRAY_SIZE(pdu_co_handlers));
+    BUG_ON(pdu_co_handlers[pdu->id] == NULL);
+    copdu = qemu_malloc(sizeof(V9fsCoPdu));
+    copdu->s = s;
+    copdu->pdu = pdu;
+    copdu->coroutine = qemu_coroutine_create(pdu_co_handlers[pdu->id]);
+    qemu_coroutine_enter(copdu->coroutine, copdu);
 }
 
 void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 699d3ab..5021da8 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -5,7 +5,7 @@
 #include <dirent.h>
 #include <sys/time.h>
 #include <utime.h>
-
+#include "hw/virtio.h"
 #include "fsdev/file-op-9p.h"
 
 /* The feature bitmap for virtio 9P */
-- 
1.7.1

Re: [Qemu-devel] [PATCH 02/25] [virtio-9p] Change all pdu handlers to coroutines.

[Stefan Hajnoczi]

On Thu, May 12, 2011 at 9:57 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> +typedef struct V9fsCoPdu {
> +    V9fsPDU *pdu;
> +    V9fsState *s;
> +    Coroutine *coroutine;
> +} V9fsCoPdu;

How about adding the V9fsState *s field to V9fsPDU?  Then you do not
need this new V9fsCoPdu struct and all the extracting fields/freeing
copdu.  You can use qemu_coroutine_self() if it is necessary to give
someone a reference to your coroutine.

Stefan

[Qemu-devel] [PATCH 03/25] [virtio-9p] Remove post functions for v9fs_readlink.

[Venkateswararao Jujjuri (JV)]

In the process of preparation for coroutine threads, remove all post functions
and make the function more readable.

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   25 +++++++------------------
 1 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index e308e9b..0b38868 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3637,21 +3637,6 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_readlink_post_readlink(V9fsState *s, V9fsReadLinkState *vs,
-                                                    int err)
-{
-    if (err < 0) {
-        err = -errno;
-        goto out;
-    }
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "s", &vs->target);
-    err = vs->offset;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->target);
-    qemu_free(vs);
-}
-
 static void v9fs_readlink(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
@@ -3667,7 +3652,6 @@ static void v9fs_readlink(void *opaque)
     vs->offset = 7;
 
     pdu_unmarshal(vs->pdu, vs->offset, "d", &fid);
-
     fidp = lookup_fid(s, fid);
     if (fidp == NULL) {
         err = -ENOENT;
@@ -3676,8 +3660,13 @@ static void v9fs_readlink(void *opaque)
 
     v9fs_string_init(&vs->target);
     err = v9fs_do_readlink(s, &fidp->path, &vs->target);
-    v9fs_readlink_post_readlink(s, vs, err);
-    return;
+    if (err < 0) {
+        err = -errno;
+        goto out;
+    }
+    vs->offset += pdu_marshal(vs->pdu, vs->offset, "s", &vs->target);
+    err = vs->offset;
+    v9fs_string_free(&vs->target);
 out:
     complete_pdu(s, vs->pdu, err);
     qemu_free(vs);
-- 
1.7.1

[Qemu-devel] [PATCH 04/25] [virtio-9p] clean up v9fs_readlink.

[Venkateswararao Jujjuri (JV)]

Rearrange the code so that we can avoid V9fsReadLinkState.

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   26 ++++++++++----------------
 hw/9pfs/virtio-9p.h |    7 -------
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 0b38868..c4d903a 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3640,36 +3640,30 @@ out:
 static void v9fs_readlink(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
+    size_t offset = 7;
+    V9fsString target;
     int32_t fid;
-    V9fsReadLinkState *vs;
     int err = 0;
     V9fsFidState *fidp;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    pdu_unmarshal(vs->pdu, vs->offset, "d", &fid);
-    fidp = lookup_fid(s, fid);
+    pdu_unmarshal(copdu->pdu, offset, "d", &fid);
+    fidp = lookup_fid(copdu->s, fid);
     if (fidp == NULL) {
         err = -ENOENT;
         goto out;
     }
 
-    v9fs_string_init(&vs->target);
-    err = v9fs_do_readlink(s, &fidp->path, &vs->target);
+    v9fs_string_init(&target);
+    err = v9fs_do_readlink(copdu->s, &fidp->path, &target);
     if (err < 0) {
         err = -errno;
         goto out;
     }
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "s", &vs->target);
-    err = vs->offset;
-    v9fs_string_free(&vs->target);
+    offset += pdu_marshal(copdu->pdu, offset, "s", &target);
+    err = offset;
+    v9fs_string_free(&target);
 out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
+    complete_pdu(copdu->s, copdu->pdu, err);
     qemu_free(copdu);
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 5021da8..bf8f64b 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -493,13 +493,6 @@ typedef struct V9fsGetlockState
     V9fsGetlock *glock;
 } V9fsGetlockState;
 
-typedef struct V9fsReadLinkState
-{
-    V9fsPDU *pdu;
-    size_t offset;
-    V9fsString target;
-} V9fsReadLinkState;
-
 size_t pdu_packunpack(void *addr, struct iovec *sg, int sg_count,
                       size_t offset, size_t size, int pack);
 
-- 
1.7.1

[Qemu-devel] [PATCH 05/25] [virtio-9p] Move errno into v9fs_do_readlink

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index c4d903a..a748c34 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -82,19 +82,21 @@ static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
     return s->ops->lstat(&s->ctx, path->data, stbuf);
 }
 
-static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
+static int v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
+        ssize_t *len)
 {
-    ssize_t len;
-
+    int err;
     buf->data = qemu_malloc(1024);
 
-    len = s->ops->readlink(&s->ctx, path->data, buf->data, 1024 - 1);
-    if (len > -1) {
-        buf->size = len;
-        buf->data[len] = 0;
+    *len = s->ops->readlink(&s->ctx, path->data, buf->data, 1024 - 1);
+    if (*len > -1) {
+        buf->size = *len;
+        buf->data[*len] = 0;
+        err = 0;
+    } else {
+        err = -errno;
     }
-
-    return len;
+    return err;
 }
 
 static int v9fs_do_close(V9fsState *s, int fd)
@@ -1055,13 +1057,11 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name,
     v9fs_string_null(&v9stat->extension);
 
     if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
-        err = v9fs_do_readlink(s, name, &v9stat->extension);
-        if (err == -1) {
-            err = -errno;
+        ssize_t symlink_len;
+        err = v9fs_do_readlink(s, name, &v9stat->extension, &symlink_len);
+        if (err < 0) {
             return err;
         }
-        v9stat->extension.data[err] = 0;
-        v9stat->extension.size = err;
     } else if (v9stat->mode & P9_STAT_MODE_DEVICE) {
         v9fs_string_sprintf(&v9stat->extension, "%c %u %u",
                 S_ISCHR(stbuf->st_mode) ? 'c' : 'b',
@@ -3645,6 +3645,7 @@ static void v9fs_readlink(void *opaque)
     int32_t fid;
     int err = 0;
     V9fsFidState *fidp;
+    ssize_t symlink_len;
 
     pdu_unmarshal(copdu->pdu, offset, "d", &fid);
     fidp = lookup_fid(copdu->s, fid);
@@ -3654,9 +3655,8 @@ static void v9fs_readlink(void *opaque)
     }
 
     v9fs_string_init(&target);
-    err = v9fs_do_readlink(copdu->s, &fidp->path, &target);
+    err = v9fs_do_readlink(copdu->s, &fidp->path, &target, &symlink_len);
     if (err < 0) {
-        err = -errno;
         goto out;
     }
     offset += pdu_marshal(copdu->pdu, offset, "s", &target);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 05/25] [virtio-9p] Move errno into v9fs_do_readlink

[Stefan Hajnoczi]

On Thu, May 12, 2011 at 01:57:27PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
> ---
>  hw/9pfs/virtio-9p.c |   32 ++++++++++++++++----------------
>  1 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index c4d903a..a748c34 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -82,19 +82,21 @@ static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
>      return s->ops->lstat(&s->ctx, path->data, stbuf);
>  }
>  
> -static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
> +static int v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
> +        ssize_t *len)

The len argument is redundant and not used by any callers, please just
return -errno and drop the len argument.  Callers rely on buf->size
instead.

Stefan

[Qemu-devel] [PATCH 06/25] [virtio-9p] coroutines for readlink

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 Makefile.objs            |    2 +-
 hw/9pfs/cofs.c           |   60 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    3 +-
 hw/9pfs/virtio-9p.c      |   22 ++--------------
 4 files changed, 66 insertions(+), 21 deletions(-)
 create mode 100644 hw/9pfs/cofs.c

diff --git a/Makefile.objs b/Makefile.objs
index 96f6a24..36005bb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
-9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
new file mode 100644
index 0000000..b972561
--- /dev/null
+++ b/hw/9pfs/cofs.c
@@ -0,0 +1,60 @@
+
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "fsdev/qemu-fsdev.h"
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include "virtio-9p-coth.h"
+
+typedef struct V9fsThReadlinkState {
+    V9fsState *s;
+    V9fsString *path;
+    V9fsString *buf;
+    ssize_t len;
+    int err;
+    V9fsRequest request;
+} V9fsThReadlinkState;
+
+static void v9fs_th_do_readlink(V9fsRequest *request)
+{
+    V9fsThReadlinkState *vsp = container_of(request, V9fsThReadlinkState,
+            request);
+    vsp->buf->data = qemu_malloc(PATH_MAX);
+
+    vsp->len = vsp->s->ops->readlink(&vsp->s->ctx, vsp->path->data,
+                                     vsp->buf->data, PATH_MAX - 1);
+    if (vsp->len > -1) {
+        vsp->buf->size = vsp->len;
+        vsp->buf->data[vsp->len] = 0;
+        vsp->err = 0;
+    } else {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
+        ssize_t *len)
+{
+    V9fsThReadlinkState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.buf = buf;
+    vs.request.func = v9fs_th_do_readlink;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    buf->size = vs.buf->size;
+    buf->data = vs.buf->data;
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 2ec1401..d0b856e 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -48,5 +48,6 @@ extern void v9fs_thread_routine(gpointer data, gpointer user_data);
 extern void v9fs_qemu_process_req_done(void *arg);
 extern void v9fs_qemu_submit_request(V9fsRequest *req);
 
-
+extern int v9fs_co_readlink(V9fsState *, V9fsString *,
+                            V9fsString *, ssize_t *);
 #endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index a748c34..7ef6ad8 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -82,23 +82,6 @@ static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
     return s->ops->lstat(&s->ctx, path->data, stbuf);
 }
 
-static int v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
-        ssize_t *len)
-{
-    int err;
-    buf->data = qemu_malloc(1024);
-
-    *len = s->ops->readlink(&s->ctx, path->data, buf->data, 1024 - 1);
-    if (*len > -1) {
-        buf->size = *len;
-        buf->data[*len] = 0;
-        err = 0;
-    } else {
-        err = -errno;
-    }
-    return err;
-}
-
 static int v9fs_do_close(V9fsState *s, int fd)
 {
     return s->ops->close(&s->ctx, fd);
@@ -1058,7 +1041,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name,
 
     if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
         ssize_t symlink_len;
-        err = v9fs_do_readlink(s, name, &v9stat->extension, &symlink_len);
+        err = v9fs_co_readlink(s, name, &v9stat->extension, &symlink_len);
         if (err < 0) {
             return err;
         }
@@ -3655,10 +3638,11 @@ static void v9fs_readlink(void *opaque)
     }
 
     v9fs_string_init(&target);
-    err = v9fs_do_readlink(copdu->s, &fidp->path, &target, &symlink_len);
+    err = v9fs_co_readlink(copdu->s, &fidp->path, &target, &symlink_len);
     if (err < 0) {
         goto out;
     }
+
     offset += pdu_marshal(copdu->pdu, offset, "s", &target);
     err = offset;
     v9fs_string_free(&target);
-- 
1.7.1

[Qemu-devel] [PATCH 07/25] [virtio-9p] Remove post functions for v9fs_mkdir.

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   52 ++++++++++++--------------------------------------
 1 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 7ef6ad8..af0143d 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3357,40 +3357,6 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_mkdir_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    stat_to_qid(&vs->stbuf, &vs->qid);
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q", &vs->qid);
-    err = vs->offset;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
-static void v9fs_mkdir_post_mkdir(V9fsState *s, V9fsMkState *vs, int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    err = v9fs_do_lstat(s, &vs->fullname, &vs->stbuf);
-    v9fs_mkdir_post_lstat(s, vs, err);
-    return;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
 static void v9fs_mkdir(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
@@ -3409,19 +3375,27 @@ static void v9fs_mkdir(void *opaque)
 
     v9fs_string_init(&vs->fullname);
     pdu_unmarshal(vs->pdu, vs->offset, "dsdd", &fid, &vs->name, &mode,
-        &gid);
+                  &gid);
 
     fidp = lookup_fid(s, fid);
     if (fidp == NULL) {
         err = -ENOENT;
         goto out;
     }
-
     v9fs_string_sprintf(&vs->fullname, "%s/%s", fidp->path.data, vs->name.data);
     err = v9fs_do_mkdir(s, vs->fullname.data, mode, fidp->uid, gid);
-    v9fs_mkdir_post_mkdir(s, vs, err);
-    return;
-
+    if (err == -1) {
+        err = -errno;
+        goto out;
+    }
+    err = v9fs_do_lstat(s, &vs->fullname, &vs->stbuf);
+    if (err == -1) {
+        err = -errno;
+        goto out;
+    }
+    stat_to_qid(&vs->stbuf, &vs->qid);
+    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q", &vs->qid);
+    err = vs->offset;
 out:
     complete_pdu(s, vs->pdu, err);
     v9fs_string_free(&vs->fullname);
-- 
1.7.1

[Qemu-devel] [PATCH 08/25] [virtio-9p] clean up v9fs_mkdir.

[Venkateswararao Jujjuri (JV)]

Rearrange the code so that we can avoid V9fsMkState and additional malloc()s.

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   39 +++++++++++++++++----------------------
 1 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index af0143d..883eced 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3360,47 +3360,42 @@ out:
 static void v9fs_mkdir(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
+    size_t offset = 7;
     int32_t fid;
-    V9fsMkState *vs;
-    int err = 0;
+    struct stat stbuf;
+    V9fsString name, fullname;
+    V9fsQID qid;
     V9fsFidState *fidp;
     gid_t gid;
     int mode;
+    int err = 0;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    v9fs_string_init(&vs->fullname);
-    pdu_unmarshal(vs->pdu, vs->offset, "dsdd", &fid, &vs->name, &mode,
-                  &gid);
+    v9fs_string_init(&fullname);
+    pdu_unmarshal(copdu->pdu, offset, "dsdd", &fid, &name, &mode, &gid);
 
-    fidp = lookup_fid(s, fid);
+    fidp = lookup_fid(copdu->s, fid);
     if (fidp == NULL) {
         err = -ENOENT;
         goto out;
     }
-    v9fs_string_sprintf(&vs->fullname, "%s/%s", fidp->path.data, vs->name.data);
-    err = v9fs_do_mkdir(s, vs->fullname.data, mode, fidp->uid, gid);
+    v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
+    err = v9fs_do_mkdir(copdu->s, fullname.data, mode, fidp->uid, gid);
     if (err == -1) {
         err = -errno;
         goto out;
     }
-    err = v9fs_do_lstat(s, &vs->fullname, &vs->stbuf);
+    err = v9fs_do_lstat(copdu->s, &fullname, &stbuf);
     if (err == -1) {
         err = -errno;
         goto out;
     }
-    stat_to_qid(&vs->stbuf, &vs->qid);
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q", &vs->qid);
-    err = vs->offset;
+    stat_to_qid(&stbuf, &qid);
+    offset += pdu_marshal(copdu->pdu, offset, "Q", &qid);
+    err = offset;
 out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
+    complete_pdu(copdu->s, copdu->pdu, err);
+    v9fs_string_free(&fullname);
+    v9fs_string_free(&name);
     qemu_free(copdu);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 09/25] hw/9pfs: Add yield support for readdir related coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

This include readdir, telldir, seekdir, rewinddir.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 Makefile.objs            |    2 +-
 hw/9pfs/codir.c          |  135 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    5 ++
 3 files changed, 141 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/codir.c

diff --git a/Makefile.objs b/Makefile.objs
index 36005bb..614bcaf 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
-9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
new file mode 100644
index 0000000..adca50c
--- /dev/null
+++ b/hw/9pfs/codir.c
@@ -0,0 +1,135 @@
+
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "fsdev/qemu-fsdev.h"
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include "virtio-9p-coth.h"
+
+typedef struct V9fsThReaddirState {
+    int err;
+    V9fsState *s;
+    V9fsFidState *fidp;
+    struct dirent *dent;
+    V9fsRequest request;
+} V9fsThReaddirState;
+
+static void v9fs_th_do_readdir(V9fsRequest *request)
+{
+    V9fsThReaddirState *vsp = container_of(request, V9fsThReaddirState,
+                                           request);
+    errno = 0;
+    vsp->dent = vsp->s->ops->readdir(&vsp->s->ctx, vsp->fidp->fs.dir);
+    if (!vsp->dent && errno) {
+        vsp->err = -errno;
+    } else {
+        vsp->err = 0;
+    }
+}
+
+int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp, struct dirent **dent)
+{
+    V9fsThReaddirState vs;
+    vs.s = s;
+    vs.fidp = fidp;
+    vs.request.func = v9fs_th_do_readdir;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    *dent = vs.dent;
+    return vs.err;
+}
+
+typedef struct V9fsThTelldirState {
+    off_t err;
+    V9fsState *s;
+    V9fsFidState *fidp;
+    V9fsRequest request;
+} V9fsThTelldirState;
+
+
+static void v9fs_th_do_telldir(V9fsRequest *request)
+{
+    V9fsThTelldirState *vsp = container_of(request, V9fsThTelldirState,
+                                           request);
+    errno = 0;
+    vsp->err = vsp->s->ops->telldir(&vsp->s->ctx, vsp->fidp->fs.dir);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+off_t v9fs_co_telldir(V9fsState *s, V9fsFidState *fidp)
+{
+    V9fsThTelldirState vs;
+    vs.s = s;
+    vs.fidp = fidp;
+    vs.request.func = v9fs_th_do_telldir;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
+
+typedef struct V9fsThSeekdirState {
+    off_t offset;
+    V9fsState *s;
+    V9fsFidState *fidp;
+    V9fsRequest request;
+} V9fsThSeekdirState;
+
+static void v9fs_th_do_seekdir(V9fsRequest *request)
+{
+    V9fsThSeekdirState *vsp = container_of(request, V9fsThSeekdirState,
+                                           request);
+    vsp->s->ops->seekdir(&vsp->s->ctx, vsp->fidp->fs.dir, vsp->offset);
+}
+
+void v9fs_co_seekdir(V9fsState *s, V9fsFidState *fidp, off_t offset)
+{
+    V9fsThSeekdirState vs;
+    vs.s = s;
+    vs.fidp = fidp;
+    vs.offset = offset;
+    vs.request.func = v9fs_th_do_seekdir;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return;
+}
+
+typedef struct V9fsThRewindirState {
+    V9fsState *s;
+    V9fsFidState *fidp;
+    V9fsRequest request;
+} V9fsThRewinddirState;
+
+static void v9fs_th_do_rewinddir(V9fsRequest *request)
+{
+    V9fsThRewinddirState *vsp = container_of(request, V9fsThRewinddirState,
+                                             request);
+    vsp->s->ops->rewinddir(&vsp->s->ctx, vsp->fidp->fs.dir);
+}
+
+void v9fs_co_rewinddir(V9fsState *s, V9fsFidState *fidp)
+{
+    V9fsThSeekdirState vs;
+    vs.s = s;
+    vs.fidp = fidp;
+    vs.request.func = v9fs_th_do_rewinddir;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index d0b856e..4d34098 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -50,4 +50,9 @@ extern void v9fs_qemu_submit_request(V9fsRequest *req);
 
 extern int v9fs_co_readlink(V9fsState *, V9fsString *,
                             V9fsString *, ssize_t *);
+extern int v9fs_co_readdir(V9fsState *, V9fsFidState *,
+                           struct dirent **);
+extern off_t v9fs_co_telldir(V9fsState *, V9fsFidState *);
+extern void v9fs_co_seekdir(V9fsState *, V9fsFidState *, off_t);
+extern void v9fs_co_rewinddir(V9fsState *, V9fsFidState *);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 10/25] hw/9pfs: Update v9fs_readdir to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |  170 +++++++++++++++++++++------------------------------
 1 files changed, 69 insertions(+), 101 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 883eced..148382d 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -112,11 +112,6 @@ static off_t v9fs_do_telldir(V9fsState *s, DIR *dir)
     return s->ops->telldir(&s->ctx, dir);
 }
 
-static struct dirent *v9fs_do_readdir(V9fsState *s, DIR *dir)
-{
-    return s->ops->readdir(&s->ctx, dir);
-}
-
 static void v9fs_do_seekdir(V9fsState *s, DIR *dir, off_t off)
 {
     return s->ops->seekdir(&s->ctx, dir, off);
@@ -1988,7 +1983,7 @@ static void v9fs_read_post_dir_lstat(V9fsState *s, V9fsReadState *vs,
     v9fs_stat_free(&vs->v9stat);
     v9fs_string_free(&vs->name);
     vs->dir_pos = vs->dent->d_off;
-    vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
+    v9fs_co_readdir(s, vs->fidp, &vs->dent);
     v9fs_read_post_readdir(s, vs, err);
     return;
 out:
@@ -2020,7 +2015,7 @@ static void v9fs_read_post_readdir(V9fsState *s, V9fsReadState *vs, ssize_t err)
 
 static void v9fs_read_post_telldir(V9fsState *s, V9fsReadState *vs, ssize_t err)
 {
-    vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
+    v9fs_co_readdir(s, vs->fidp, &vs->dent);
     v9fs_read_post_readdir(s, vs, err);
     return;
 }
@@ -2151,127 +2146,100 @@ out:
     qemu_free(copdu);
 }
 
-typedef struct V9fsReadDirState {
-    V9fsPDU *pdu;
-    V9fsFidState *fidp;
-    V9fsQID qid;
-    off_t saved_dir_pos;
-    struct dirent *dent;
-    int32_t count;
-    int32_t max_count;
-    size_t offset;
-    int64_t initial_offset;
-    V9fsString name;
-} V9fsReadDirState;
-
-static void v9fs_readdir_post_seekdir(V9fsState *s, V9fsReadDirState *vs)
-{
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->count);
-    vs->offset += vs->count;
-    complete_pdu(s, vs->pdu, vs->offset);
-    qemu_free(vs);
-    return;
-}
-
-/* Size of each dirent on the wire: size of qid (13) + size of offset (8)
+/*
+ * Size of each dirent on the wire: size of qid (13) + size of offset (8)
  * size of type (1) + size of name.size (2) + strlen(name.data)
  */
-#define V9_READDIR_DATA_SZ (24 + strlen(vs->name.data))
+#define V9_READDIR_DATA_SZ (24 + strlen(name.data))
 
-static void v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
+static int v9fs_do_readdir(V9fsState *s, V9fsPDU *pdu,
+                           V9fsFidState *fidp, int32_t max_count)
 {
-    int len;
     size_t size;
+    V9fsQID qid;
+    V9fsString name;
+    int len, err = 0;
+    int32_t count = 0;
+    off_t saved_dir_pos;
+    struct dirent *dent;
 
-    if (vs->dent) {
-        v9fs_string_init(&vs->name);
-        v9fs_string_sprintf(&vs->name, "%s", vs->dent->d_name);
-
-        if ((vs->count + V9_READDIR_DATA_SZ) > vs->max_count) {
+    /* save the directory position */
+    saved_dir_pos = v9fs_co_telldir(s, fidp);
+    if (saved_dir_pos < 0) {
+        return saved_dir_pos;
+    }
+    while (1) {
+        err = v9fs_co_readdir(s, fidp, &dent);
+        if (err || !dent) {
+            break;
+        }
+        v9fs_string_init(&name);
+        v9fs_string_sprintf(&name, "%s", dent->d_name);
+        if ((count + V9_READDIR_DATA_SZ) > max_count) {
             /* Ran out of buffer. Set dir back to old position and return */
-            v9fs_do_seekdir(s, vs->fidp->fs.dir, vs->saved_dir_pos);
-            v9fs_readdir_post_seekdir(s, vs);
-            return;
+            v9fs_co_seekdir(s, fidp, saved_dir_pos);
+            v9fs_string_free(&name);
+            return count;
         }
-
-        /* Fill up just the path field of qid because the client uses
+        /*
+         * Fill up just the path field of qid because the client uses
          * only that. To fill the entire qid structure we will have
          * to stat each dirent found, which is expensive
          */
-        size = MIN(sizeof(vs->dent->d_ino), sizeof(vs->qid.path));
-        memcpy(&vs->qid.path, &vs->dent->d_ino, size);
+        size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
+        memcpy(&qid.path, &dent->d_ino, size);
         /* Fill the other fields with dummy values */
-        vs->qid.type = 0;
-        vs->qid.version = 0;
-
-        len = pdu_marshal(vs->pdu, vs->offset+4+vs->count, "Qqbs",
-                              &vs->qid, vs->dent->d_off,
-                              vs->dent->d_type, &vs->name);
-        vs->count += len;
-        v9fs_string_free(&vs->name);
-        vs->saved_dir_pos = vs->dent->d_off;
-        vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
-        v9fs_readdir_post_readdir(s, vs);
-        return;
-    }
-
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->count);
-    vs->offset += vs->count;
-    complete_pdu(s, vs->pdu, vs->offset);
-    qemu_free(vs);
-    return;
-}
-
-static void v9fs_readdir_post_telldir(V9fsState *s, V9fsReadDirState *vs)
-{
-    vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
-    v9fs_readdir_post_readdir(s, vs);
-    return;
-}
+        qid.type = 0;
+        qid.version = 0;
 
-static void v9fs_readdir_post_setdir(V9fsState *s, V9fsReadDirState *vs)
-{
-    vs->saved_dir_pos = v9fs_do_telldir(s, vs->fidp->fs.dir);
-    v9fs_readdir_post_telldir(s, vs);
-    return;
+        /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
+        len = pdu_marshal(pdu, 11 + count, "Qqbs",
+                          &qid, dent->d_off,
+                          dent->d_type, &name);
+        count += len;
+        v9fs_string_free(&name);
+        saved_dir_pos = dent->d_off;
+    }
+    if (err < 0) {
+        return err;
+    }
+    return count;
 }
 
 static void v9fs_readdir(void *opaque)
 {
+    int32_t fid;
+    V9fsFidState *fidp;
+    ssize_t retval = 0;
+    size_t offset = 7;
+    int64_t initial_offset;
+    int32_t count, max_count;
     V9fsCoPdu *copdu = opaque;
     V9fsState *s = copdu->s;
     V9fsPDU *pdu = copdu->pdu;
-    int32_t fid;
-    V9fsReadDirState *vs;
-    ssize_t err = 0;
-    size_t offset = 7;
-
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-    vs->count = 0;
 
-    pdu_unmarshal(vs->pdu, offset, "dqd", &fid, &vs->initial_offset,
-                                                        &vs->max_count);
+    pdu_unmarshal(pdu, offset, "dqd", &fid, &initial_offset, &max_count);
 
-    vs->fidp = lookup_fid(s, fid);
-    if (vs->fidp == NULL || !(vs->fidp->fs.dir)) {
-        err = -EINVAL;
+    fidp = lookup_fid(s, fid);
+    if (fidp == NULL || !fidp->fs.dir) {
+        retval = -EINVAL;
         goto out;
     }
-
-    if (vs->initial_offset == 0) {
-        v9fs_do_rewinddir(s, vs->fidp->fs.dir);
+    if (initial_offset == 0) {
+        v9fs_co_rewinddir(s, fidp);
     } else {
-        v9fs_do_seekdir(s, vs->fidp->fs.dir, vs->initial_offset);
+        v9fs_co_seekdir(s, fidp, initial_offset);
     }
-
-    v9fs_readdir_post_setdir(s, vs);
-    return;
-
+    count = v9fs_do_readdir(s, pdu, fidp, max_count);
+    if (count < 0) {
+        retval = count;
+        goto out;
+    }
+    retval = offset;
+    retval += pdu_marshal(pdu, offset, "d", count);
+    retval += count;
 out:
-    complete_pdu(s, pdu, err);
-    qemu_free(vs);
+    complete_pdu(s, pdu, retval);
     qemu_free(copdu);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 11/25] hw/9pfs: Add yield support to statfs coroutine

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/cofs.c           |   31 +++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    1 +
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index b972561..27e243e 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -58,3 +58,34 @@ int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
     buf->data = vs.buf->data;
     return vs.err;
 }
+
+typedef struct V9fsThStatfsState {
+    int err;
+    V9fsState *s;
+    V9fsString *path;
+    struct statfs *stbuf;
+    V9fsRequest request;
+} V9fsThStatfsState;
+
+static void v9fs_th_do_statfs(V9fsRequest *request)
+{
+    V9fsThStatfsState *vsp = container_of(request, V9fsThStatfsState,
+                                          request);
+    vsp->err = vsp->s->ops->statfs(&vsp->s->ctx, vsp->path->data, vsp->stbuf);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
+{
+    V9fsThStatfsState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.stbuf = stbuf;
+    vs.request.func = v9fs_th_do_statfs;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 4d34098..57f30ff 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -55,4 +55,5 @@ extern int v9fs_co_readdir(V9fsState *, V9fsFidState *,
 extern off_t v9fs_co_telldir(V9fsState *, V9fsFidState *);
 extern void v9fs_co_seekdir(V9fsState *, V9fsFidState *, off_t);
 extern void v9fs_co_rewinddir(V9fsState *, V9fsFidState *);
+extern int v9fs_co_statfs(V9fsState *, V9fsString *, struct statfs *);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 12/25] hw/9pfs: Update v9fs_statfs to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   92 ++++++++++++++++++++++++--------------------------
 hw/9pfs/virtio-9p.h |   22 ------------
 2 files changed, 44 insertions(+), 70 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 148382d..86e9482 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3075,80 +3075,76 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err)
-{
+static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
+{
+    uint32_t f_type;
+    uint32_t f_bsize;
+    uint64_t f_blocks;
+    uint64_t f_bfree;
+    uint64_t f_bavail;
+    uint64_t f_files;
+    uint64_t f_ffree;
+    uint64_t fsid_val;
+    uint32_t f_namelen;
+    size_t offset = 7;
     int32_t bsize_factor;
 
-    if (err) {
-        err = -errno;
-        goto out;
-    }
-
     /*
      * compute bsize factor based on host file system block size
      * and client msize
      */
-    bsize_factor = (s->msize - P9_IOHDRSZ)/vs->stbuf.f_bsize;
+    bsize_factor = (s->msize - P9_IOHDRSZ)/stbuf->f_bsize;
     if (!bsize_factor) {
         bsize_factor = 1;
     }
-    vs->v9statfs.f_type = vs->stbuf.f_type;
-    vs->v9statfs.f_bsize = vs->stbuf.f_bsize;
-    vs->v9statfs.f_bsize *= bsize_factor;
+    f_type  = stbuf->f_type;
+    f_bsize = stbuf->f_bsize;
+    f_bsize *= bsize_factor;
     /*
      * f_bsize is adjusted(multiplied) by bsize factor, so we need to
      * adjust(divide) the number of blocks, free blocks and available
      * blocks by bsize factor
      */
-    vs->v9statfs.f_blocks = vs->stbuf.f_blocks/bsize_factor;
-    vs->v9statfs.f_bfree = vs->stbuf.f_bfree/bsize_factor;
-    vs->v9statfs.f_bavail = vs->stbuf.f_bavail/bsize_factor;
-    vs->v9statfs.f_files = vs->stbuf.f_files;
-    vs->v9statfs.f_ffree = vs->stbuf.f_ffree;
-    vs->v9statfs.fsid_val = (unsigned int) vs->stbuf.f_fsid.__val[0] |
-			(unsigned long long)vs->stbuf.f_fsid.__val[1] << 32;
-    vs->v9statfs.f_namelen = vs->stbuf.f_namelen;
-
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "ddqqqqqqd",
-         vs->v9statfs.f_type, vs->v9statfs.f_bsize, vs->v9statfs.f_blocks,
-         vs->v9statfs.f_bfree, vs->v9statfs.f_bavail, vs->v9statfs.f_files,
-         vs->v9statfs.f_ffree, vs->v9statfs.fsid_val,
-         vs->v9statfs.f_namelen);
+    f_blocks = stbuf->f_blocks/bsize_factor;
+    f_bfree  = stbuf->f_bfree/bsize_factor;
+    f_bavail = stbuf->f_bavail/bsize_factor;
+    f_files  = stbuf->f_files;
+    f_ffree  = stbuf->f_ffree;
+    fsid_val = (unsigned int) stbuf->f_fsid.__val[0] |
+               (unsigned long long)stbuf->f_fsid.__val[1] << 32;
+    f_namelen = stbuf->f_namelen;
 
-out:
-    complete_pdu(s, vs->pdu, vs->offset);
-    qemu_free(vs);
+    return pdu_marshal(pdu, offset, "ddqqqqqqd",
+                       f_type, f_bsize, f_blocks, f_bfree,
+                       f_bavail, f_files, f_ffree,
+                       fsid_val, f_namelen);
 }
 
 static void v9fs_statfs(void *opaque)
 {
+    int32_t fid;
+    ssize_t retval = 0;
+    size_t offset = 7;
+    V9fsFidState *fidp;
+    struct statfs stbuf;
     V9fsCoPdu *copdu = opaque;
     V9fsState *s = copdu->s;
     V9fsPDU *pdu = copdu->pdu;
-    V9fsStatfsState *vs;
-    ssize_t err = 0;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    memset(&vs->v9statfs, 0, sizeof(vs->v9statfs));
-
-    pdu_unmarshal(vs->pdu, vs->offset, "d", &vs->fid);
-
-    vs->fidp = lookup_fid(s, vs->fid);
-    if (vs->fidp == NULL) {
-        err = -ENOENT;
+    pdu_unmarshal(pdu, offset, "d", &fid);
+    fidp = lookup_fid(s, fid);
+    if (fidp == NULL) {
+        retval = -ENOENT;
         goto out;
     }
-
-    err = v9fs_do_statfs(s, &vs->fidp->path, &vs->stbuf);
-    v9fs_statfs_post_statfs(s, vs, err);
-    return;
-
+    retval = v9fs_co_statfs(s, &fidp->path, &stbuf);
+    if (retval < 0) {
+        goto out;
+    }
+    retval = offset;
+    retval += v9fs_fill_statfs(s, pdu, &stbuf);
 out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
+    complete_pdu(s, pdu, retval);
     qemu_free(copdu);
     return;
 }
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index bf8f64b..ee8cb79 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -396,28 +396,6 @@ struct virtio_9p_config
     uint8_t tag[0];
 } __attribute__((packed));
 
-typedef struct V9fsStatfs
-{
-    uint32_t f_type;
-    uint32_t f_bsize;
-    uint64_t f_blocks;
-    uint64_t f_bfree;
-    uint64_t f_bavail;
-    uint64_t f_files;
-    uint64_t f_ffree;
-    uint64_t fsid_val;
-    uint32_t f_namelen;
-} V9fsStatfs;
-
-typedef struct V9fsStatfsState {
-    V9fsPDU *pdu;
-    size_t offset;
-    int32_t fid;
-    V9fsStatfs v9statfs;
-    V9fsFidState *fidp;
-    struct statfs stbuf;
-} V9fsStatfsState;
-
 typedef struct V9fsMkState {
     V9fsPDU *pdu;
     size_t offset;
-- 
1.7.1

[Qemu-devel] [PATCH 13/25] hw/9pfs: Add yield support to lstat coroutine

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 Makefile.objs            |    2 +-
 hw/9pfs/cofile.c         |   50 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    1 +
 3 files changed, 52 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/cofile.c

diff --git a/Makefile.objs b/Makefile.objs
index 614bcaf..83d30bd 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
-9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
new file mode 100644
index 0000000..860b060
--- /dev/null
+++ b/hw/9pfs/cofile.c
@@ -0,0 +1,50 @@
+
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "fsdev/qemu-fsdev.h"
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include "virtio-9p-coth.h"
+
+
+typedef struct V9fsThLstatState {
+    int err;
+    V9fsState *s;
+    V9fsString *path;
+    struct stat *stbuf;
+    V9fsRequest request;
+} V9fsThLstatState;
+
+static void v9fs_th_do_lstat(V9fsRequest *request)
+{
+    V9fsThLstatState *vsp = container_of(request, V9fsThLstatState,
+                                         request);
+    vsp->err = vsp->s->ops->lstat(&vsp->s->ctx, vsp->path->data, vsp->stbuf);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
+{
+    V9fsThLstatState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.stbuf = stbuf;
+    vs.request.func = v9fs_th_do_lstat;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 57f30ff..aedeba9 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -56,4 +56,5 @@ extern off_t v9fs_co_telldir(V9fsState *, V9fsFidState *);
 extern void v9fs_co_seekdir(V9fsState *, V9fsFidState *, off_t);
 extern void v9fs_co_rewinddir(V9fsState *, V9fsFidState *);
 extern int v9fs_co_statfs(V9fsState *, V9fsString *, struct statfs *);
+extern int v9fs_co_lstat(V9fsState *, V9fsString *, struct stat *);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 14/25] hw/9pfs: Update v9fs_getattr to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   59 +++++++++++++++++---------------------------------
 hw/9pfs/virtio-9p.h |    8 -------
 2 files changed, 20 insertions(+), 47 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 86e9482..fa2bb1f 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1088,7 +1088,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name,
 
 
 static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
-                            V9fsStatDotl *v9lstat)
+                                V9fsStatDotl *v9lstat)
 {
     memset(v9lstat, 0, sizeof(*v9lstat));
 
@@ -1291,58 +1291,39 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
-                                                                int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl);
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl);
-    err = vs->offset;
-
-out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
-
 static void v9fs_getattr(void *opaque)
 {
-    V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
-    V9fsStatStateDotl *vs;
-    ssize_t err = 0;
+    size_t offset = 7;
+    ssize_t retval = 0;
+    struct stat stbuf;
     V9fsFidState *fidp;
     uint64_t request_mask;
+    V9fsStatDotl v9stat_dotl;
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    memset(&vs->v9stat_dotl, 0, sizeof(vs->v9stat_dotl));
-
-    pdu_unmarshal(vs->pdu, vs->offset, "dq", &fid, &request_mask);
+    pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
 
     fidp = lookup_fid(s, fid);
     if (fidp == NULL) {
-        err = -ENOENT;
+        retval = -ENOENT;
         goto out;
     }
-
-    /* Currently we only support BASIC fields in stat, so there is no
+    /*
+     * Currently we only support BASIC fields in stat, so there is no
      * need to look at request_mask.
      */
-    err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf);
-    v9fs_getattr_post_lstat(s, vs, err);
-    return;
-
+    retval = v9fs_co_lstat(s, &fidp->path, &stbuf);
+    if (retval < 0) {
+        goto out;
+    }
+    stat_to_v9stat_dotl(s, &stbuf, &v9stat_dotl);
+    retval = offset;
+    retval += pdu_marshal(pdu, offset, "A", &v9stat_dotl);
 out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
+    complete_pdu(s, pdu, retval);
     qemu_free(copdu);
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index ee8cb79..7e4bea0 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -276,14 +276,6 @@ typedef struct V9fsStatDotl {
     uint64_t st_data_version;
 } V9fsStatDotl;
 
-typedef struct V9fsStatStateDotl {
-    V9fsPDU *pdu;
-    size_t offset;
-    V9fsStatDotl v9stat_dotl;
-    struct stat stbuf;
-} V9fsStatStateDotl;
-
-
 typedef struct V9fsWalkState {
     V9fsPDU *pdu;
     size_t offset;
-- 
1.7.1

[Qemu-devel] [PATCH 15/25] hw/9pfs: Add yield support to setattr related coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

This include chmod, utimensat, chown and truncate.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/cofs.c           |  135 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    4 ++
 2 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 27e243e..02eb0ba 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -89,3 +89,138 @@ int v9fs_co_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
     qemu_coroutine_yield();
     return vs.err;
 }
+
+typedef struct V9fsThChmodState {
+    int err;
+    mode_t mode;
+    V9fsState *s;
+    V9fsString *path;
+    V9fsRequest request;
+} V9fsThChmodState;
+
+static void v9fs_th_do_chmod(V9fsRequest *request)
+{
+    FsCred cred;
+    V9fsThChmodState *vsp = container_of(request, V9fsThChmodState,
+                                         request);
+    cred_init(&cred);
+    cred.fc_mode = vsp->mode;
+    vsp->err = vsp->s->ops->chmod(&vsp->s->ctx, vsp->path->data, &cred);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_chmod(V9fsState *s, V9fsString *path, mode_t mode)
+{
+    V9fsThChmodState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.mode = mode;
+    vs.request.func = v9fs_th_do_chmod;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
+
+typedef struct V9fsThUtimeState {
+    int err;
+    V9fsState *s;
+    V9fsString *path;
+    struct timespec *times;
+    V9fsRequest request;
+} V9fsThUtimeState;
+
+static void v9fs_th_do_utimensat(V9fsRequest *request)
+{
+    V9fsThUtimeState *vsp = container_of(request, V9fsThUtimeState,
+                                         request);
+    vsp->err = vsp->s->ops->utimensat(&vsp->s->ctx,
+                                      vsp->path->data, vsp->times);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_utimensat(V9fsState *s, V9fsString *path,
+                      struct timespec times[2])
+{
+    V9fsThUtimeState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.times = times;
+    vs.request.func = v9fs_th_do_utimensat;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
+
+typedef struct V9fsThChownState {
+    int err;
+    uid_t uid;
+    gid_t gid;
+    V9fsState *s;
+    V9fsString *path;
+    V9fsRequest request;
+} V9fsThChownState;
+
+static void v9fs_th_do_chown(V9fsRequest *request)
+{
+    FsCred cred;
+    V9fsThChownState *vsp = container_of(request, V9fsThChownState,
+                                         request);
+    cred_init(&cred);
+    cred.fc_uid = vsp->uid;
+    cred.fc_gid = vsp->gid;
+    vsp->err = vsp->s->ops->chown(&vsp->s->ctx, vsp->path->data, &cred);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_chown(V9fsState *s, V9fsString *path, uid_t uid, gid_t gid)
+{
+    V9fsThChownState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.uid  = uid;
+    vs.gid  = gid;
+    vs.request.func = v9fs_th_do_chown;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
+
+typedef struct V9fsThTruncateState {
+    int err;
+    off_t size;
+    V9fsState *s;
+    V9fsString *path;
+    V9fsRequest request;
+} V9fsThTruncateState;
+
+static void v9fs_th_do_truncate(V9fsRequest *request)
+{
+    V9fsThTruncateState *vsp = container_of(request, V9fsThTruncateState,
+                                            request);
+    vsp->err = vsp->s->ops->truncate(&vsp->s->ctx, vsp->path->data, vsp->size);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_truncate(V9fsState *s, V9fsString *path, off_t size)
+{
+    V9fsThTruncateState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.size = size;
+    vs.request.func = v9fs_th_do_truncate;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index aedeba9..4e2bc23 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -57,4 +57,8 @@ extern void v9fs_co_seekdir(V9fsState *, V9fsFidState *, off_t);
 extern void v9fs_co_rewinddir(V9fsState *, V9fsFidState *);
 extern int v9fs_co_statfs(V9fsState *, V9fsString *, struct statfs *);
 extern int v9fs_co_lstat(V9fsState *, V9fsString *, struct stat *);
+extern int v9fs_co_chmod(V9fsState *, V9fsString *, mode_t);
+extern int v9fs_co_utimensat(V9fsState *, V9fsString *, struct timespec [2]);
+extern int v9fs_co_chown(V9fsState *, V9fsString *, uid_t, gid_t);
+extern int v9fs_co_truncate(V9fsState *, V9fsString *, off_t);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 16/25] hw/9pfs: Update v9fs_setattr to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |  165 +++++++++++++++++----------------------------------
 hw/9pfs/virtio-9p.h |    8 ---
 2 files changed, 55 insertions(+), 118 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index fa2bb1f..8723039 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1339,140 +1339,85 @@ out:
 #define ATTR_ATIME_SET  (1 << 7)
 #define ATTR_MTIME_SET  (1 << 8)
 
-static void v9fs_setattr_post_truncate(V9fsState *s, V9fsSetattrState *vs,
-                                                                  int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-    err = vs->offset;
-
-out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
-
-static void v9fs_setattr_post_chown(V9fsState *s, V9fsSetattrState *vs, int err)
+static void v9fs_setattr(void *opaque)
 {
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    if (vs->v9iattr.valid & (ATTR_SIZE)) {
-        err = v9fs_do_truncate(s, &vs->fidp->path, vs->v9iattr.size);
-    }
-    v9fs_setattr_post_truncate(s, vs, err);
-    return;
+    int err = 0;
+    int32_t fid;
+    V9fsFidState *fidp;
+    size_t offset = 7;
+    V9fsIattr v9iattr;
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
 
-out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
+    pdu_unmarshal(pdu, offset, "dI", &fid, &v9iattr);
 
-static void v9fs_setattr_post_utimensat(V9fsState *s, V9fsSetattrState *vs,
-                                                                   int err)
-{
-    if (err == -1) {
-        err = -errno;
+    fidp = lookup_fid(s, fid);
+    if (fidp == NULL) {
+        err = -EINVAL;
         goto out;
     }
-
-    /* If the only valid entry in iattr is ctime we can call
-     * chown(-1,-1) to update the ctime of the file
-     */
-    if ((vs->v9iattr.valid & (ATTR_UID | ATTR_GID)) ||
-            ((vs->v9iattr.valid & ATTR_CTIME)
-            && !((vs->v9iattr.valid & ATTR_MASK) & ~ATTR_CTIME))) {
-        if (!(vs->v9iattr.valid & ATTR_UID)) {
-            vs->v9iattr.uid = -1;
-        }
-        if (!(vs->v9iattr.valid & ATTR_GID)) {
-            vs->v9iattr.gid = -1;
+    if (v9iattr.valid & ATTR_MODE) {
+        err = v9fs_co_chmod(s, &fidp->path, v9iattr.mode);
+        if (err < 0) {
+            goto out;
         }
-        err = v9fs_do_chown(s, &vs->fidp->path, vs->v9iattr.uid,
-                                                vs->v9iattr.gid);
     }
-    v9fs_setattr_post_chown(s, vs, err);
-    return;
-
-out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
-
-static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs, int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    if (vs->v9iattr.valid & (ATTR_ATIME | ATTR_MTIME)) {
+    if (v9iattr.valid & (ATTR_ATIME | ATTR_MTIME)) {
         struct timespec times[2];
-        if (vs->v9iattr.valid & ATTR_ATIME) {
-            if (vs->v9iattr.valid & ATTR_ATIME_SET) {
-                times[0].tv_sec = vs->v9iattr.atime_sec;
-                times[0].tv_nsec = vs->v9iattr.atime_nsec;
+        if (v9iattr.valid & ATTR_ATIME) {
+            if (v9iattr.valid & ATTR_ATIME_SET) {
+                times[0].tv_sec = v9iattr.atime_sec;
+                times[0].tv_nsec = v9iattr.atime_nsec;
             } else {
                 times[0].tv_nsec = UTIME_NOW;
             }
         } else {
             times[0].tv_nsec = UTIME_OMIT;
         }
-
-        if (vs->v9iattr.valid & ATTR_MTIME) {
-            if (vs->v9iattr.valid & ATTR_MTIME_SET) {
-                times[1].tv_sec = vs->v9iattr.mtime_sec;
-                times[1].tv_nsec = vs->v9iattr.mtime_nsec;
+        if (v9iattr.valid & ATTR_MTIME) {
+            if (v9iattr.valid & ATTR_MTIME_SET) {
+                times[1].tv_sec = v9iattr.mtime_sec;
+                times[1].tv_nsec = v9iattr.mtime_nsec;
             } else {
                 times[1].tv_nsec = UTIME_NOW;
             }
         } else {
             times[1].tv_nsec = UTIME_OMIT;
         }
-        err = v9fs_do_utimensat(s, &vs->fidp->path, times);
+        err = v9fs_co_utimensat(s, &fidp->path, times);
+        if (err < 0) {
+            goto out;
+        }
     }
-    v9fs_setattr_post_utimensat(s, vs, err);
-    return;
-
-out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
-
-static void v9fs_setattr(void *opaque)
-{
-    V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
-    int32_t fid;
-    V9fsSetattrState *vs;
-    int err = 0;
-
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    pdu_unmarshal(pdu, vs->offset, "dI", &fid, &vs->v9iattr);
-
-    vs->fidp = lookup_fid(s, fid);
-    if (vs->fidp == NULL) {
-        err = -EINVAL;
-        goto out;
+    /*
+     * If the only valid entry in iattr is ctime we can call
+     * chown(-1,-1) to update the ctime of the file
+     */
+    if ((v9iattr.valid & (ATTR_UID | ATTR_GID)) ||
+        ((v9iattr.valid & ATTR_CTIME)
+         && !((v9iattr.valid & ATTR_MASK) & ~ATTR_CTIME))) {
+        if (!(v9iattr.valid & ATTR_UID)) {
+            v9iattr.uid = -1;
+        }
+        if (!(v9iattr.valid & ATTR_GID)) {
+            v9iattr.gid = -1;
+        }
+        err = v9fs_co_chown(s, &fidp->path, v9iattr.uid,
+                            v9iattr.gid);
+        if (err < 0) {
+            goto out;
+        }
     }
-
-    if (vs->v9iattr.valid & ATTR_MODE) {
-        err = v9fs_do_chmod(s, &vs->fidp->path, vs->v9iattr.mode);
+    if (v9iattr.valid & (ATTR_SIZE)) {
+        err = v9fs_co_truncate(s, &fidp->path, v9iattr.size);
+        if (err < 0) {
+            goto out;
+        }
     }
-
-    v9fs_setattr_post_chmod(s, vs, err);
-    return;
-
+    err = offset;
 out:
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
+    complete_pdu(s, pdu, err);
     qemu_free(copdu);
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 7e4bea0..dd4bcc8 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -372,14 +372,6 @@ typedef struct V9fsIattr
     int64_t mtime_nsec;
 } V9fsIattr;
 
-typedef struct V9fsSetattrState
-{
-    V9fsPDU *pdu;
-    size_t offset;
-    V9fsIattr v9iattr;
-    V9fsFidState *fidp;
-} V9fsSetattrState;
-
 struct virtio_9p_config
 {
     /* number of characters in tag */
-- 
1.7.1

[Qemu-devel] [PATCH 17/25] hw/9pfs: Add yield support to xattr related coroutine

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

This include llistxattr and lgetxattr.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 Makefile.objs            |    1 +
 hw/9pfs/coxattr.c        |   92 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    3 +
 3 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 hw/9pfs/coxattr.c

diff --git a/Makefile.objs b/Makefile.objs
index 83d30bd..98a345b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -298,6 +298,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
+9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/coxattr.c b/hw/9pfs/coxattr.c
new file mode 100644
index 0000000..fe8526e
--- /dev/null
+++ b/hw/9pfs/coxattr.c
@@ -0,0 +1,92 @@
+
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "fsdev/qemu-fsdev.h"
+#include "qemu-thread.h"
+#include "qemu-coroutine.h"
+#include "virtio-9p-coth.h"
+
+
+typedef struct V9fsThLlistXattrState {
+    int err;
+    void *value;
+    size_t size;
+    V9fsState *s;
+    V9fsString *path;
+    V9fsRequest request;
+} V9fsThLlistXattrState;
+
+static void v9fs_th_do_llistxattr(V9fsRequest *request)
+{
+    V9fsThLlistXattrState *vsp = container_of(request, V9fsThLlistXattrState,
+                                              request);
+    vsp->err = vsp->s->ops->llistxattr(&vsp->s->ctx, vsp->path->data,
+                                       vsp->value, vsp->size);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_llistxattr(V9fsState *s, V9fsString *path, void *value, size_t size)
+{
+    V9fsThLlistXattrState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.value = value;
+    vs.size = size;
+    vs.request.func = v9fs_th_do_llistxattr;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
+
+typedef struct V9fsThLgetXattrState {
+    int err;
+    void *value;
+    size_t size;
+    V9fsState *s;
+    V9fsString *path;
+    V9fsString *xattr_name;
+    V9fsRequest request;
+} V9fsThLgetXattrState;
+
+static void v9fs_th_do_lgetxattr(V9fsRequest *request)
+{
+    V9fsThLgetXattrState *vsp = container_of(request, V9fsThLgetXattrState,
+                                             request);
+    vsp->err = vsp->s->ops->lgetxattr(&vsp->s->ctx, vsp->path->data,
+                                      vsp->xattr_name->data,
+                                      vsp->value, vsp->size);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_lgetxattr(V9fsState *s, V9fsString *path,
+                      V9fsString *xattr_name,
+                      void *value, size_t size)
+{
+    V9fsThLgetXattrState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.value = value;
+    vs.size = size;
+    vs.xattr_name = xattr_name;
+    vs.request.func = v9fs_th_do_lgetxattr;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 4e2bc23..c8d37dd 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -61,4 +61,7 @@ extern int v9fs_co_chmod(V9fsState *, V9fsString *, mode_t);
 extern int v9fs_co_utimensat(V9fsState *, V9fsString *, struct timespec [2]);
 extern int v9fs_co_chown(V9fsState *, V9fsString *, uid_t, gid_t);
 extern int v9fs_co_truncate(V9fsState *, V9fsString *, off_t);
+extern int v9fs_co_llistxattr(V9fsState *, V9fsString *, void *, size_t);
+extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
+                             V9fsString *, void *, size_t);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 18/25] hw/9pfs: Update v9fs_xattrwalk to coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |  198 ++++++++++++++++-----------------------------------
 1 files changed, 63 insertions(+), 135 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8723039..b50ac3c 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -240,21 +240,6 @@ static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
     return s->ops->statfs(&s->ctx, path->data, stbuf);
 }
 
-static ssize_t v9fs_do_lgetxattr(V9fsState *s, V9fsString *path,
-                             V9fsString *xattr_name,
-                             void *value, size_t size)
-{
-    return s->ops->lgetxattr(&s->ctx, path->data,
-                             xattr_name->data, value, size);
-}
-
-static ssize_t v9fs_do_llistxattr(V9fsState *s, V9fsString *path,
-                              void *value, size_t size)
-{
-    return s->ops->llistxattr(&s->ctx, path->data,
-                              value, size);
-}
-
 static int v9fs_do_lsetxattr(V9fsState *s, V9fsString *path,
                              V9fsString *xattr_name,
                              void *value, size_t size, int flags)
@@ -3289,150 +3274,93 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_post_xattr_getvalue(V9fsState *s, V9fsXattrState *vs, int err)
-{
-
-    if (err < 0) {
-        err = -errno;
-        free_fid(s, vs->xattr_fidp->fid);
-        goto out;
-    }
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "q", vs->size);
-    err = vs->offset;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-    return;
-}
-
-static void v9fs_post_xattr_check(V9fsState *s, V9fsXattrState *vs, ssize_t err)
-{
-    if (err < 0) {
-        err = -errno;
-        free_fid(s, vs->xattr_fidp->fid);
-        goto out;
-    }
-    /*
-     * Read the xattr value
-     */
-    vs->xattr_fidp->fs.xattr.len = vs->size;
-    vs->xattr_fidp->fid_type = P9_FID_XATTR;
-    vs->xattr_fidp->fs.xattr.copied_len = -1;
-    if (vs->size) {
-        vs->xattr_fidp->fs.xattr.value = qemu_malloc(vs->size);
-        err = v9fs_do_lgetxattr(s, &vs->xattr_fidp->path,
-                                &vs->name, vs->xattr_fidp->fs.xattr.value,
-                                vs->xattr_fidp->fs.xattr.len);
-    }
-    v9fs_post_xattr_getvalue(s, vs, err);
-    return;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
-static void v9fs_post_lxattr_getvalue(V9fsState *s,
-                                      V9fsXattrState *vs, int err)
-{
-    if (err < 0) {
-        err = -errno;
-        free_fid(s, vs->xattr_fidp->fid);
-        goto out;
-    }
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "q", vs->size);
-    err = vs->offset;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-    return;
-}
-
-static void v9fs_post_lxattr_check(V9fsState *s,
-                                   V9fsXattrState *vs, ssize_t err)
-{
-    if (err < 0) {
-        err = -errno;
-        free_fid(s, vs->xattr_fidp->fid);
-        goto out;
-    }
-    /*
-     * Read the xattr value
-     */
-    vs->xattr_fidp->fs.xattr.len = vs->size;
-    vs->xattr_fidp->fid_type = P9_FID_XATTR;
-    vs->xattr_fidp->fs.xattr.copied_len = -1;
-    if (vs->size) {
-        vs->xattr_fidp->fs.xattr.value = qemu_malloc(vs->size);
-        err = v9fs_do_llistxattr(s, &vs->xattr_fidp->path,
-                                 vs->xattr_fidp->fs.xattr.value,
-                                 vs->xattr_fidp->fs.xattr.len);
-    }
-    v9fs_post_lxattr_getvalue(s, vs, err);
-    return;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
 static void v9fs_xattrwalk(void *opaque)
 {
+    int64_t size;
+    V9fsString name;
+    ssize_t err = 0;
+    size_t offset = 7;
+    int32_t fid, newfid;
+    V9fsFidState *file_fidp;
+    V9fsFidState *xattr_fidp;
     V9fsCoPdu *copdu = opaque;
     V9fsState *s = copdu->s;
     V9fsPDU *pdu = copdu->pdu;
-    ssize_t err = 0;
-    V9fsXattrState *vs;
-    int32_t fid, newfid;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    pdu_unmarshal(vs->pdu, vs->offset, "dds", &fid, &newfid, &vs->name);
-    vs->file_fidp = lookup_fid(s, fid);
-    if (vs->file_fidp == NULL) {
+    pdu_unmarshal(pdu, offset, "dds", &fid, &newfid, &name);
+    file_fidp = lookup_fid(s, fid);
+    if (file_fidp == NULL) {
         err = -ENOENT;
         goto out;
     }
-
-    vs->xattr_fidp = alloc_fid(s, newfid);
-    if (vs->xattr_fidp == NULL) {
+    xattr_fidp = alloc_fid(s, newfid);
+    if (xattr_fidp == NULL) {
         err = -EINVAL;
         goto out;
     }
-
-    v9fs_string_copy(&vs->xattr_fidp->path, &vs->file_fidp->path);
-    if (vs->name.data[0] == 0) {
+    v9fs_string_copy(&xattr_fidp->path, &file_fidp->path);
+    if (name.data[0] == 0) {
         /*
          * listxattr request. Get the size first
          */
-        vs->size = v9fs_do_llistxattr(s, &vs->xattr_fidp->path,
-                                      NULL, 0);
-        if (vs->size < 0) {
-            err = vs->size;
+        size = v9fs_co_llistxattr(s, &xattr_fidp->path, NULL, 0);
+        if (size < 0) {
+            err = size;
+            free_fid(s, xattr_fidp->fid);
+            goto out;
         }
-        v9fs_post_lxattr_check(s, vs, err);
-        return;
+        /*
+         * Read the xattr value
+         */
+        xattr_fidp->fs.xattr.len = size;
+        xattr_fidp->fid_type = P9_FID_XATTR;
+        xattr_fidp->fs.xattr.copied_len = -1;
+        if (size) {
+            xattr_fidp->fs.xattr.value = qemu_malloc(size);
+            err = v9fs_co_llistxattr(s, &xattr_fidp->path,
+                                     xattr_fidp->fs.xattr.value,
+                                     xattr_fidp->fs.xattr.len);
+            if (err < 0) {
+                free_fid(s, xattr_fidp->fid);
+                goto out;
+            }
+        }
+        offset += pdu_marshal(pdu, offset, "q", size);
+        err = offset;
     } else {
         /*
          * specific xattr fid. We check for xattr
          * presence also collect the xattr size
          */
-        vs->size = v9fs_do_lgetxattr(s, &vs->xattr_fidp->path,
-                                     &vs->name, NULL, 0);
-        if (vs->size < 0) {
-            err = vs->size;
+        size = v9fs_co_lgetxattr(s, &xattr_fidp->path,
+                                 &name, NULL, 0);
+        if (size < 0) {
+            err = size;
+            free_fid(s, xattr_fidp->fid);
+            goto out;
         }
-        v9fs_post_xattr_check(s, vs, err);
-        return;
+        /*
+         * Read the xattr value
+         */
+        xattr_fidp->fs.xattr.len = size;
+        xattr_fidp->fid_type = P9_FID_XATTR;
+        xattr_fidp->fs.xattr.copied_len = -1;
+        if (size) {
+            xattr_fidp->fs.xattr.value = qemu_malloc(size);
+            err = v9fs_co_lgetxattr(s, &xattr_fidp->path,
+                                    &name, xattr_fidp->fs.xattr.value,
+                                    xattr_fidp->fs.xattr.len);
+            if (err < 0) {
+                free_fid(s, xattr_fidp->fid);
+                goto out;
+            }
+        }
+        offset += pdu_marshal(pdu, offset, "q", size);
+        err = offset;
     }
 out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
+    complete_pdu(s, pdu, err);
+    v9fs_string_free(&name);
     qemu_free(copdu);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 19/25] hw/9pfs: Update v9fs_xattrcreate to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   55 +++++++++++++++++++++++++--------------------------
 hw/9pfs/virtio-9p.h |   11 ----------
 2 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index b50ac3c..764348d 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3366,44 +3366,43 @@ out:
 
 static void v9fs_xattrcreate(void *opaque)
 {
-    V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
     int flags;
     int32_t fid;
+    int64_t size;
     ssize_t err = 0;
-    V9fsXattrState *vs;
-
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
+    V9fsString name;
+    size_t offset = 7;
+    V9fsFidState *file_fidp;
+    V9fsFidState *xattr_fidp;
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
 
-    pdu_unmarshal(vs->pdu, vs->offset, "dsqd",
-                  &fid, &vs->name, &vs->size, &flags);
+    pdu_unmarshal(pdu, offset, "dsqd",
+                  &fid, &name, &size, &flags);
 
-    vs->file_fidp = lookup_fid(s, fid);
-    if (vs->file_fidp == NULL) {
+    file_fidp = lookup_fid(s, fid);
+    if (file_fidp == NULL) {
         err = -EINVAL;
         goto out;
     }
-
     /* Make the file fid point to xattr */
-    vs->xattr_fidp = vs->file_fidp;
-    vs->xattr_fidp->fid_type = P9_FID_XATTR;
-    vs->xattr_fidp->fs.xattr.copied_len = 0;
-    vs->xattr_fidp->fs.xattr.len = vs->size;
-    vs->xattr_fidp->fs.xattr.flags = flags;
-    v9fs_string_init(&vs->xattr_fidp->fs.xattr.name);
-    v9fs_string_copy(&vs->xattr_fidp->fs.xattr.name, &vs->name);
-    if (vs->size)
-        vs->xattr_fidp->fs.xattr.value = qemu_malloc(vs->size);
-    else
-        vs->xattr_fidp->fs.xattr.value = NULL;
-
+    xattr_fidp = file_fidp;
+    xattr_fidp->fid_type = P9_FID_XATTR;
+    xattr_fidp->fs.xattr.copied_len = 0;
+    xattr_fidp->fs.xattr.len = size;
+    xattr_fidp->fs.xattr.flags = flags;
+    v9fs_string_init(&xattr_fidp->fs.xattr.name);
+    v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name);
+    if (size) {
+        xattr_fidp->fs.xattr.value = qemu_malloc(size);
+    } else {
+        xattr_fidp->fs.xattr.value = NULL;
+    }
+    err = offset;
 out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
+    complete_pdu(s, pdu, err);
+    v9fs_string_free(&name);
     qemu_free(copdu);
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index dd4bcc8..596d35b 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -397,17 +397,6 @@ typedef struct V9fsRenameState {
     V9fsString name;
 } V9fsRenameState;
 
-typedef struct V9fsXattrState
-{
-    V9fsPDU *pdu;
-    size_t offset;
-    V9fsFidState *file_fidp;
-    V9fsFidState *xattr_fidp;
-    V9fsString name;
-    int64_t size;
-    int flags;
-    void *value;
-} V9fsXattrState;
 
 #define P9_LOCK_SUCCESS 0
 #define P9_LOCK_BLOCKED 1
-- 
1.7.1

[Qemu-devel] [PATCH 20/25] hw/9pfs: Add yield support to mknod coroutine

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/cofs.c           |   45 +++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    2 ++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 02eb0ba..06127f7 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -224,3 +224,48 @@ int v9fs_co_truncate(V9fsState *s, V9fsString *path, off_t size)
     qemu_coroutine_yield();
     return vs.err;
 }
+
+typedef struct V9fsThMknodState {
+    int err;
+    uid_t uid;
+    gid_t gid;
+    mode_t mode;
+    dev_t dev;
+    V9fsState *s;
+    V9fsString *path;
+    struct stat *stbuf;
+    V9fsRequest request;
+} V9fsThMknodState;
+
+static void v9fs_th_do_mknod(V9fsRequest *request)
+{
+    FsCred cred;
+    V9fsThMknodState *vsp = container_of(request, V9fsThMknodState,
+                                         request);
+    cred_init(&cred);
+    cred.fc_uid  = vsp->uid;
+    cred.fc_gid  = vsp->gid;
+    cred.fc_mode = vsp->mode;
+    cred.fc_rdev = vsp->dev;
+    vsp->err = vsp->s->ops->mknod(&vsp->s->ctx, vsp->path->data, &cred);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_mknod(V9fsState *s, V9fsString *path, uid_t uid,
+                  gid_t gid, dev_t dev, mode_t mode)
+{
+    V9fsThMknodState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.uid  = uid;
+    vs.gid  = gid;
+    vs.dev  = dev;
+    vs.mode = mode;
+    vs.request.func = v9fs_th_do_mknod;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index c8d37dd..bd410cb 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -64,4 +64,6 @@ extern int v9fs_co_truncate(V9fsState *, V9fsString *, off_t);
 extern int v9fs_co_llistxattr(V9fsState *, V9fsString *, void *, size_t);
 extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
                              V9fsString *, void *, size_t);
+extern int v9fs_co_mknod(V9fsState *, V9fsString *, uid_t,
+                         gid_t, dev_t, mode_t);
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 21/25] hw/9pfs: Update v9fs_mknod to use coroutines

[Venkateswararao Jujjuri (JV)]

From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   89 +++++++++++++++++---------------------------------
 1 files changed, 30 insertions(+), 59 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 764348d..4858f37 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3060,78 +3060,49 @@ out:
     return;
 }
 
-static void v9fs_mknod_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    stat_to_qid(&vs->stbuf, &vs->qid);
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q", &vs->qid);
-    err = vs->offset;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
-static void v9fs_mknod_post_mknod(V9fsState *s, V9fsMkState *vs, int err)
-{
-    if (err == -1) {
-        err = -errno;
-        goto out;
-    }
-
-    err = v9fs_do_lstat(s, &vs->fullname, &vs->stbuf);
-    v9fs_mknod_post_lstat(s, vs, err);
-    return;
-out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
-}
-
 static void v9fs_mknod(void *opaque)
 {
-    V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
+    int mode;
+    gid_t gid;
     int32_t fid;
-    V9fsMkState *vs;
+    V9fsQID qid;
     int err = 0;
-    V9fsFidState *fidp;
-    gid_t gid;
-    int mode;
     int major, minor;
+    size_t offset = 7;
+    V9fsString name;
+    struct stat stbuf;
+    V9fsString fullname;
+    V9fsFidState *fidp;
+    V9fsCoPdu *copdu = opaque;
+    V9fsState *s = copdu->s;
+    V9fsPDU *pdu = copdu->pdu;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    v9fs_string_init(&vs->fullname);
-    pdu_unmarshal(vs->pdu, vs->offset, "dsdddd", &fid, &vs->name, &mode,
-        &major, &minor, &gid);
+    v9fs_string_init(&fullname);
+    pdu_unmarshal(pdu, offset, "dsdddd", &fid, &name, &mode,
+                  &major, &minor, &gid);
 
     fidp = lookup_fid(s, fid);
     if (fidp == NULL) {
         err = -ENOENT;
         goto out;
     }
-
-    v9fs_string_sprintf(&vs->fullname, "%s/%s", fidp->path.data, vs->name.data);
-    err = v9fs_do_mknod(s, vs->fullname.data, mode, makedev(major, minor),
-        fidp->uid, gid);
-    v9fs_mknod_post_mknod(s, vs, err);
-    return;
-
+    v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
+    err = v9fs_co_mknod(s, &fullname, fidp->uid, gid,
+                        makedev(major, minor), mode);
+    if (err < 0) {
+        goto out;
+    }
+    err = v9fs_co_lstat(s, &fullname, &stbuf);
+    if (err < 0) {
+        goto out;
+    }
+    stat_to_qid(&stbuf, &qid);
+    err = offset;
+    err += pdu_marshal(pdu, offset, "Q", &qid);
 out:
-    complete_pdu(s, vs->pdu, err);
-    v9fs_string_free(&vs->fullname);
-    v9fs_string_free(&vs->name);
-    qemu_free(vs);
+    complete_pdu(s, pdu, err);
+    v9fs_string_free(&fullname);
+    v9fs_string_free(&name);
     qemu_free(copdu);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 22/25] [virtio-9p] coroutine and threading for mkdir

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/codir.c          |   34 ++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    1 +
 hw/9pfs/virtio-9p.c      |   28 ++++++----------------------
 3 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index adca50c..fcd204d 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -133,3 +133,37 @@ void v9fs_co_rewinddir(V9fsState *s, V9fsFidState *fidp)
     qemu_coroutine_yield();
     return;
 }
+
+typedef struct V9fsThMkDirState {
+    V9fsState *s;
+    FsCred cred;
+    char *name;
+    int err;
+    V9fsRequest request;
+} V9fsThMkDirState;
+
+static void v9fs_th_do_mkdir(V9fsRequest *request)
+{
+    V9fsThMkDirState *vsp = container_of(request, V9fsThMkDirState, request);
+
+    vsp->err = vsp->s->ops->mkdir(&vsp->s->ctx, vsp->name, &vsp->cred);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_mkdir(V9fsState *s, char *name, mode_t mode, uid_t uid, gid_t gid)
+{
+    V9fsThMkDirState vs;
+    vs.s = s;
+    vs.name = name;
+    cred_init(&vs.cred);
+    vs.cred.fc_mode = mode;
+    vs.cred.fc_uid = uid;
+    vs.cred.fc_gid = gid;
+    vs.request.func = v9fs_th_do_mkdir;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index bd410cb..3edab4e 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -66,4 +66,5 @@ extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
                              V9fsString *, void *, size_t);
 extern int v9fs_co_mknod(V9fsState *, V9fsString *, uid_t,
                          gid_t, dev_t, mode_t);
+extern int v9fs_co_mkdir(V9fsState *, char *, mode_t, uid_t, gid_t);
 #endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 4858f37..e5112fe 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -149,19 +149,6 @@ static int v9fs_do_mknod(V9fsState *s, char *name,
     return s->ops->mknod(&s->ctx, name, &cred);
 }
 
-static int v9fs_do_mkdir(V9fsState *s, char *name, mode_t mode,
-                uid_t uid, gid_t gid)
-{
-    FsCred cred;
-
-    cred_init(&cred);
-    cred.fc_uid = uid;
-    cred.fc_gid = gid;
-    cred.fc_mode = mode;
-
-    return s->ops->mkdir(&s->ctx, name, &cred);
-}
-
 static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
 {
     return s->ops->fstat(&s->ctx, fd, stbuf);
@@ -2354,8 +2341,7 @@ out:
 
 static void v9fs_create_post_mkdir(V9fsState *s, V9fsCreateState *vs, int err)
 {
-    if (err) {
-        err = -errno;
+    if (err < 0) {
         goto out;
     }
 
@@ -2404,7 +2390,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
     }
 
     if (vs->perm & P9_STAT_MODE_DIR) {
-        err = v9fs_do_mkdir(s, vs->fullname.data, vs->perm & 0777,
+        err = v9fs_co_mkdir(s, vs->fullname.data, vs->perm & 0777,
                 vs->fidp->uid, -1);
         v9fs_create_post_mkdir(s, vs, err);
     } else if (vs->perm & P9_STAT_MODE_SYMLINK) {
@@ -3225,14 +3211,12 @@ static void v9fs_mkdir(void *opaque)
         goto out;
     }
     v9fs_string_sprintf(&fullname, "%s/%s", fidp->path.data, name.data);
-    err = v9fs_do_mkdir(copdu->s, fullname.data, mode, fidp->uid, gid);
-    if (err == -1) {
-        err = -errno;
+    err = v9fs_co_mkdir(copdu->s, fullname.data, mode, fidp->uid, gid);
+    if (err < 0) {
         goto out;
     }
-    err = v9fs_do_lstat(copdu->s, &fullname, &stbuf);
-    if (err == -1) {
-        err = -errno;
+    err = v9fs_co_lstat(copdu->s, &fullname, &stbuf);
+    if (err < 0) {
         goto out;
     }
     stat_to_qid(&stbuf, &qid);
-- 
1.7.1

[Qemu-devel] [PATCH 23/25] [virtio-9p] Remove post functions for v9fs_remove

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   26 +++++++-------------------
 1 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index e5112fe..dcce864 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -2612,22 +2612,6 @@ out:
     qemu_free(copdu);
 }
 
-static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
-                                                                int err)
-{
-    if (err < 0) {
-        err = -errno;
-    } else {
-        err = vs->offset;
-    }
-
-    /* For TREMOVE we need to clunk the fid even on failed remove */
-    free_fid(s, vs->fidp->fid);
-
-    complete_pdu(s, vs->pdu, err);
-    qemu_free(vs);
-}
-
 static void v9fs_remove(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
@@ -2648,11 +2632,15 @@ static void v9fs_remove(void *opaque)
         err = -EINVAL;
         goto out;
     }
-
     err = v9fs_do_remove(s, &vs->fidp->path);
-    v9fs_remove_post_remove(s, vs, err);
-    return;
+    if (err < 0) {
+        err = -errno;
+    } else {
+        err = vs->offset;
+    }
 
+    /* For TREMOVE we need to clunk the fid even on failed remove */
+    free_fid(s, vs->fidp->fid);
 out:
     complete_pdu(s, pdu, err);
     qemu_free(vs);
-- 
1.7.1

[Qemu-devel] [PATCH 24/25] [virtio-9p] clean up v9fs_remove.

[Venkateswararao Jujjuri (JV)]

Rearrange the code so that we can avoid V9fsRemoveState
and additional malloc()s.

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p.c |   24 +++++++++---------------
 hw/9pfs/virtio-9p.h |    6 ------
 2 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index dcce864..114162c 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -2615,35 +2615,29 @@ out:
 static void v9fs_remove(void *opaque)
 {
     V9fsCoPdu *copdu = opaque;
-    V9fsState *s = copdu->s;
-    V9fsPDU *pdu = copdu->pdu;
     int32_t fid;
-    V9fsRemoveState *vs;
     int err = 0;
+    size_t offset = 7;
+    V9fsFidState *fidp;
 
-    vs = qemu_malloc(sizeof(*vs));
-    vs->pdu = pdu;
-    vs->offset = 7;
-
-    pdu_unmarshal(vs->pdu, vs->offset, "d", &fid);
+    pdu_unmarshal(copdu->pdu, offset, "d", &fid);
 
-    vs->fidp = lookup_fid(s, fid);
-    if (vs->fidp == NULL) {
+    fidp = lookup_fid(copdu->s, fid);
+    if (fidp == NULL) {
         err = -EINVAL;
         goto out;
     }
-    err = v9fs_do_remove(s, &vs->fidp->path);
+    err = v9fs_do_remove(copdu->s, &fidp->path);
     if (err < 0) {
         err = -errno;
     } else {
-        err = vs->offset;
+        err = offset;
     }
 
     /* For TREMOVE we need to clunk the fid even on failed remove */
-    free_fid(s, vs->fidp->fid);
+    free_fid(copdu->s, fidp->fid);
 out:
-    complete_pdu(s, pdu, err);
-    qemu_free(vs);
+    complete_pdu(copdu->s, copdu->pdu, err);
     qemu_free(copdu);
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 596d35b..04b2a19 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -331,12 +331,6 @@ typedef struct V9fsWriteState {
     int cnt;
 } V9fsWriteState;
 
-typedef struct V9fsRemoveState {
-    V9fsPDU *pdu;
-    size_t offset;
-    V9fsFidState *fidp;
-} V9fsRemoveState;
-
 typedef struct V9fsWstatState
 {
     V9fsPDU *pdu;
-- 
1.7.1

[Qemu-devel] [PATCH 25/25] [virtio-9p] coroutine and threading for remove/unlink

[Venkateswararao Jujjuri (JV)]

Signed-off-by: Venkateswararao Jujjuri "<jvrao@linux.vnet.ibm.com>
---
 hw/9pfs/cofs.c           |   28 ++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h |    1 +
 hw/9pfs/virtio-9p.c      |   11 ++---------
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 06127f7..68cd3ab 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -269,3 +269,31 @@ int v9fs_co_mknod(V9fsState *s, V9fsString *path, uid_t uid,
     qemu_coroutine_yield();
     return vs.err;
 }
+
+typedef struct V9fsThRemoveState {
+    V9fsState *s;
+    V9fsString *path;
+    int err;
+    V9fsRequest request;
+} V9fsThRemoveState;
+
+static void v9fs_th_do_remove(V9fsRequest *request)
+{
+    V9fsThRemoveState *vsp = container_of(request, V9fsThRemoveState, request);
+    vsp->err = vsp->s->ops->remove(&vsp->s->ctx, vsp->path->data);
+    if (vsp->err < 0) {
+        vsp->err = -errno;
+    }
+}
+
+int v9fs_co_remove(V9fsState *s, V9fsString *path)
+{
+    V9fsThRemoveState vs;
+    vs.s = s;
+    vs.path = path;
+    vs.request.func = v9fs_th_do_remove;
+    vs.request.coroutine = qemu_coroutine_self();
+    v9fs_qemu_submit_request(&vs.request);
+    qemu_coroutine_yield();
+    return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 3edab4e..b6428ec 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -67,4 +67,5 @@ extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
 extern int v9fs_co_mknod(V9fsState *, V9fsString *, uid_t,
                          gid_t, dev_t, mode_t);
 extern int v9fs_co_mkdir(V9fsState *, char *, mode_t, uid_t, gid_t);
+extern int v9fs_co_remove(V9fsState *, V9fsString *);
 #endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 114162c..479ad78 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -212,11 +212,6 @@ static int v9fs_do_utimensat(V9fsState *s, V9fsString *path,
     return s->ops->utimensat(&s->ctx, path->data, times);
 }
 
-static int v9fs_do_remove(V9fsState *s, V9fsString *path)
-{
-    return s->ops->remove(&s->ctx, path->data);
-}
-
 static int v9fs_do_fsync(V9fsState *s, int fd, int datasync)
 {
     return s->ops->fsync(&s->ctx, fd, datasync);
@@ -2627,10 +2622,8 @@ static void v9fs_remove(void *opaque)
         err = -EINVAL;
         goto out;
     }
-    err = v9fs_do_remove(copdu->s, &fidp->path);
-    if (err < 0) {
-        err = -errno;
-    } else {
+    err = v9fs_co_remove(copdu->s, &fidp->path);
+    if (!err) {
         err = offset;
     }
 
-- 
1.7.1

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Stefan Hajnoczi]

On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV) wrote:
> VirtFS (fileserver base on 9P) performs many blocking system calls in the
> vCPU context. This effort is to move the blocking calls out of vCPU/IO
> thread context, into asynchronous threads.
>
> Anthony's " Add hard build dependency on glib" patch and
> Kevin/Stefan's coroutine effort is a prerequisite.
>
> This patch set contains:
>  - Converting all 9pfs calls into coroutines.
>  - Each 9P operation will be modified for:
>     - Remove post* functions. These are our call back functions which makes
>       the code very hard to read. Now with coroutines, we can achieve the same
>       state machine model with nice sequential code flow.
>     - Move errno access near to the local_syscall()
>     - Introduce asynchronous threading
>
> This series has the basic infrastructure and few routines like
> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
> Currently we are working on converting and testing other 9P operations also
> into this model and those patches will follow shortly.
>
> Removing callback functions made some of the patches little lengthy.

This long patch series adds temporary structs and marshalling code for
each file system operation - I think none of this is necessary.  Instead
we can exploit coroutines more:

The point of coroutines is that you can suspend a thread of control (a
call-stack, not an OS-level thread) and can re-enter it later.  We
should make coroutines thread-safe (i.e. work outside of the global
mutex) and then allow switching a coroutine from a QEMU thread to a
worker thread and back again:

int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
                                 struct dirent **dent)
{
    int ret = 0;

    v9fs_co_run_in_worker({
        errno = 0;
        *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
        if (!*dent && errno) {
            ret = -errno;
        }
    });
    return ret;
}

v9fs_co_readdir() can be called from a QEMU thread.  The block of code
inside v9fs_co_run_in_worker() will be executed in a worker thread.
Notice that no marshalling variables is necessary at all; we can use the
function arguments and local variables because this is still the same
function!

When control reaches the end of the v9fs_co_run_in_worker() block,
execution is resumed in a QEMU thread and the function then returns ret.
It would be incorrect to return inside the v9fs_co_run_in_worker() block
because at that point we're still inside the worker thread.

Here is how v9fs_co_run_in_worker() does its magic:

#define v9fs_co_run_in_worker(block) \
{ \
    BH *co_bh; \
\
    co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
    qemu_bh_schedule(co_bh); \
    qemu_coroutine_yield(); /* re-entered in worker thread */ \
    qemu_bh_delete(co_bh); \
\
    block; \
\
    qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
}

void co_run_in_worker_bh(void *opaque)
{
    Coroutine *co = opaque;

    g_thread_pool_push(pool, co, NULL);
}

void worker_thread_fn(gpointer data, gpointer user_data)
{
    Coroutine *co = user_data;
    char byte = 0;
    ssize_t len;

    qemu_coroutine_enter(co, NULL);

    g_async_queue_push(v9fs_pool.completed, co);
    do {
        len = write(v9fs_pool.wfd, &byte, sizeof(byte));
    } while (len == -1 && errno == EINTR);
}

void process_req_done(void *arg)
{
    Coroutine *co;
    char byte;
    ssize_t len;

    do {
        len = read(v9fs_pool.rfd, &byte, sizeof(byte));
    } while (len == -1 && errno == EINTR);

    while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
        qemu_coroutine_enter(co, NULL);
    }
}

I typed this code out in the email, it has not been compiled or tested.

If you decide to eliminate coroutines entirely in the future and use
worker threads exclusively to process requests, then there are clearly
marked sections in the code: anything inside v9fs_co_run_in_worker()
must be thread-safe already and anything outside it needs to be audited
and made thread-safe.  The changes required are smaller than those if
your current patch series was applied.  I wanted to mention this point
to show that this doesn't paint virtfs into a corner.

So where does this leave virtfs?  No marshalling is necessary and
blocking operations can be performed inline using
v9fs_co_run_in_worker() blocks.  The codebase will be a lot smaller.

Does this seem reasonable?

Stefan

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Anthony Liguori]

On 05/13/2011 03:55 AM, Stefan Hajnoczi wrote:
> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV) wrote:
>> VirtFS (fileserver base on 9P) performs many blocking system calls in the
>> vCPU context. This effort is to move the blocking calls out of vCPU/IO
>> thread context, into asynchronous threads.
>>
>> Anthony's " Add hard build dependency on glib" patch and
>> Kevin/Stefan's coroutine effort is a prerequisite.
>>
>> This patch set contains:
>>   - Converting all 9pfs calls into coroutines.
>>   - Each 9P operation will be modified for:
>>      - Remove post* functions. These are our call back functions which makes
>>        the code very hard to read. Now with coroutines, we can achieve the same
>>        state machine model with nice sequential code flow.
>>      - Move errno access near to the local_syscall()
>>      - Introduce asynchronous threading
>>
>> This series has the basic infrastructure and few routines like
>> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
>> Currently we are working on converting and testing other 9P operations also
>> into this model and those patches will follow shortly.
>>
>> Removing callback functions made some of the patches little lengthy.
>
> This long patch series adds temporary structs and marshalling code for
> each file system operation - I think none of this is necessary.  Instead
> we can exploit coroutines more:
>
> The point of coroutines is that you can suspend a thread of control (a
> call-stack, not an OS-level thread) and can re-enter it later.  We
> should make coroutines thread-safe (i.e. work outside of the global
> mutex) and then allow switching a coroutine from a QEMU thread to a
> worker thread and back again:
>
> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>                                   struct dirent **dent)
> {
>      int ret = 0;
>
>      v9fs_co_run_in_worker({
>          errno = 0;
>          *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>          if (!*dent&&  errno) {
>              ret = -errno;
>          }
>      });
>      return ret;
> }
>
> v9fs_co_readdir() can be called from a QEMU thread.  The block of code
> inside v9fs_co_run_in_worker() will be executed in a worker thread.

I'd prefer if we did:

threadlet_run({
    ret = readdir_r(dirp, dent);
    if (ret == -1) {
       ret = -errno;
    }
});

And not worry at all about re-entrancy.

> Notice that no marshalling variables is necessary at all; we can use the
> function arguments and local variables because this is still the same
> function!
>
> When control reaches the end of the v9fs_co_run_in_worker() block,
> execution is resumed in a QEMU thread and the function then returns ret.
> It would be incorrect to return inside the v9fs_co_run_in_worker() block
> because at that point we're still inside the worker thread.
>
> Here is how v9fs_co_run_in_worker() does its magic:
>
> #define v9fs_co_run_in_worker(block) \
> { \
>      BH *co_bh; \
> \
>      co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>      qemu_bh_schedule(co_bh); \
>      qemu_coroutine_yield(); /* re-entered in worker thread */ \
>      qemu_bh_delete(co_bh); \
> \
>      block; \
> \
>      qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
> }
>
> void co_run_in_worker_bh(void *opaque)
> {
>      Coroutine *co = opaque;
>
>      g_thread_pool_push(pool, co, NULL);
> }
>
> void worker_thread_fn(gpointer data, gpointer user_data)
> {
>      Coroutine *co = user_data;
>      char byte = 0;
>      ssize_t len;
>
>      qemu_coroutine_enter(co, NULL);
>
>      g_async_queue_push(v9fs_pool.completed, co);
>      do {
>          len = write(v9fs_pool.wfd,&byte, sizeof(byte));
>      } while (len == -1&&  errno == EINTR);
> }
>
> void process_req_done(void *arg)
> {
>      Coroutine *co;
>      char byte;
>      ssize_t len;
>
>      do {
>          len = read(v9fs_pool.rfd,&byte, sizeof(byte));
>      } while (len == -1&&  errno == EINTR);
>
>      while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>          qemu_coroutine_enter(co, NULL);
>      }
> }
>
> I typed this code out in the email, it has not been compiled or tested.

I did this myself a while ago.  The concept is definitely sound.  I 
think it's the way to go.

I also think it's a candidate for the block layer too.

Regards,

Anthony Liguori

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Aneesh Kumar K.V]

On Fri, 13 May 2011 09:55:03 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV) wrote:
> > VirtFS (fileserver base on 9P) performs many blocking system calls in the
> > vCPU context. This effort is to move the blocking calls out of vCPU/IO
> > thread context, into asynchronous threads.
> >
> > Anthony's " Add hard build dependency on glib" patch and
> > Kevin/Stefan's coroutine effort is a prerequisite.
> >
> > This patch set contains:
> >  - Converting all 9pfs calls into coroutines.
> >  - Each 9P operation will be modified for:
> >     - Remove post* functions. These are our call back functions which makes
> >       the code very hard to read. Now with coroutines, we can achieve the same
> >       state machine model with nice sequential code flow.
> >     - Move errno access near to the local_syscall()
> >     - Introduce asynchronous threading
> >
> > This series has the basic infrastructure and few routines like
> > mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
> > Currently we are working on converting and testing other 9P operations also
> > into this model and those patches will follow shortly.
> >
> > Removing callback functions made some of the patches little lengthy.
> 
> This long patch series adds temporary structs and marshalling code for
> each file system operation - I think none of this is necessary.  Instead
> we can exploit coroutines more:
> 
> The point of coroutines is that you can suspend a thread of control (a
> call-stack, not an OS-level thread) and can re-enter it later.  We
> should make coroutines thread-safe (i.e. work outside of the global
> mutex) and then allow switching a coroutine from a QEMU thread to a
> worker thread and back again:
> 
> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>                                  struct dirent **dent)
> {
>     int ret = 0;
> 
>     v9fs_co_run_in_worker({
>         errno = 0;
>         *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>         if (!*dent && errno) {
>             ret = -errno;
>         }
>     });
>     return ret;
> }
> 
> v9fs_co_readdir() can be called from a QEMU thread.  The block of code
> inside v9fs_co_run_in_worker() will be executed in a worker thread.
> Notice that no marshalling variables is necessary at all; we can use the
> function arguments and local variables because this is still the same
> function!
> 
> When control reaches the end of the v9fs_co_run_in_worker() block,
> execution is resumed in a QEMU thread and the function then returns ret.
> It would be incorrect to return inside the v9fs_co_run_in_worker() block
> because at that point we're still inside the worker thread.
> 
> Here is how v9fs_co_run_in_worker() does its magic:
> 
> #define v9fs_co_run_in_worker(block) \
> { \
>     BH *co_bh; \
> \
>     co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>     qemu_bh_schedule(co_bh); \
>     qemu_coroutine_yield(); /* re-entered in worker thread */ \
>     qemu_bh_delete(co_bh); \
> \
>     block; \
> \
>     qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
> }
> 
> void co_run_in_worker_bh(void *opaque)
> {
>     Coroutine *co = opaque;
> 
>     g_thread_pool_push(pool, co, NULL);
> }
> 
> void worker_thread_fn(gpointer data, gpointer user_data)
> {
>     Coroutine *co = user_data;
>     char byte = 0;
>     ssize_t len;
> 
>     qemu_coroutine_enter(co, NULL);
> 
>     g_async_queue_push(v9fs_pool.completed, co);
>     do {
>         len = write(v9fs_pool.wfd, &byte, sizeof(byte));
>     } while (len == -1 && errno == EINTR);
> }
> 
> void process_req_done(void *arg)
> {
>     Coroutine *co;
>     char byte;
>     ssize_t len;
> 
>     do {
>         len = read(v9fs_pool.rfd, &byte, sizeof(byte));
>     } while (len == -1 && errno == EINTR);
> 
>     while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>         qemu_coroutine_enter(co, NULL);
>     }
> }
> 
> I typed this code out in the email, it has not been compiled or tested.
> 
> If you decide to eliminate coroutines entirely in the future and use
> worker threads exclusively to process requests, then there are clearly
> marked sections in the code: anything inside v9fs_co_run_in_worker()
> must be thread-safe already and anything outside it needs to be audited
> and made thread-safe.  The changes required are smaller than those if
> your current patch series was applied.  I wanted to mention this point
> to show that this doesn't paint virtfs into a corner.
> 
> So where does this leave virtfs?  No marshalling is necessary and
> blocking operations can be performed inline using
> v9fs_co_run_in_worker() blocks.  The codebase will be a lot smaller.
> 
> Does this seem reasonable?
> 

Do we really need bottom halfs here ? can't we achieve the same with
v9fs_qemu_submit_request() and making the glib thread
function callback (request.func())to do qemu_coroutine_enter()

like:

int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp, struct dirent **dent)
{
     v9fs_wthread_enter();
     s->ops->readdir(&s->ctx, fidp->fs.dir);
     v9fs_wthread_exit();
}

v9fs_worker_thread_enter()
{
    v9fs_qemu_submit_request(v9fs_worker_request);
    qemu_coroutine_yield();
}

v9fs_coroutine_woker_func()
{
      qemu_coroutine_enter(coroutine, NULL);
}


I also wonder whether additional bottom halfs and additional
setcontext/setjmp that we end up with will have a performance impact compared
to what we have currently ?

-aneesh

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Venkateswararao Jujjuri]

On 05/13/2011 12:26 PM, Aneesh Kumar K.V wrote:
> On Fri, 13 May 2011 09:55:03 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV) wrote:
>>> VirtFS (fileserver base on 9P) performs many blocking system calls in the
>>> vCPU context. This effort is to move the blocking calls out of vCPU/IO
>>> thread context, into asynchronous threads.
>>>
>>> Anthony's " Add hard build dependency on glib" patch and
>>> Kevin/Stefan's coroutine effort is a prerequisite.
>>>
>>> This patch set contains:
>>>   - Converting all 9pfs calls into coroutines.
>>>   - Each 9P operation will be modified for:
>>>      - Remove post* functions. These are our call back functions which makes
>>>        the code very hard to read. Now with coroutines, we can achieve the same
>>>        state machine model with nice sequential code flow.
>>>      - Move errno access near to the local_syscall()
>>>      - Introduce asynchronous threading
>>>
>>> This series has the basic infrastructure and few routines like
>>> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
>>> Currently we are working on converting and testing other 9P operations also
>>> into this model and those patches will follow shortly.
>>>
>>> Removing callback functions made some of the patches little lengthy.
>> This long patch series adds temporary structs and marshalling code for
>> each file system operation - I think none of this is necessary.  Instead
>> we can exploit coroutines more:
>>
>> The point of coroutines is that you can suspend a thread of control (a
>> call-stack, not an OS-level thread) and can re-enter it later.  We
>> should make coroutines thread-safe (i.e. work outside of the global
>> mutex) and then allow switching a coroutine from a QEMU thread to a
>> worker thread and back again:
>>
>> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>>                                   struct dirent **dent)
>> {
>>      int ret = 0;
>>
>>      v9fs_co_run_in_worker({
>>          errno = 0;
>>          *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>>          if (!*dent&&  errno) {
>>              ret = -errno;
>>          }
>>      });
>>      return ret;
>> }
>>
>> v9fs_co_readdir() can be called from a QEMU thread.  The block of code
>> inside v9fs_co_run_in_worker() will be executed in a worker thread.
>> Notice that no marshalling variables is necessary at all; we can use the
>> function arguments and local variables because this is still the same
>> function!
>>
>> When control reaches the end of the v9fs_co_run_in_worker() block,
>> execution is resumed in a QEMU thread and the function then returns ret.
>> It would be incorrect to return inside the v9fs_co_run_in_worker() block
>> because at that point we're still inside the worker thread.
>>
>> Here is how v9fs_co_run_in_worker() does its magic:
>>
>> #define v9fs_co_run_in_worker(block) \
>> { \
>>      BH *co_bh; \
>> \
>>      co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>>      qemu_bh_schedule(co_bh); \
>>      qemu_coroutine_yield(); /* re-entered in worker thread */ \
>>      qemu_bh_delete(co_bh); \
>> \
>>      block; \
>> \
>>      qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>> }
>>
>> void co_run_in_worker_bh(void *opaque)
>> {
>>      Coroutine *co = opaque;
>>
>>      g_thread_pool_push(pool, co, NULL);
>> }
>>
>> void worker_thread_fn(gpointer data, gpointer user_data)
>> {
>>      Coroutine *co = user_data;
>>      char byte = 0;
>>      ssize_t len;
>>
>>      qemu_coroutine_enter(co, NULL);
>>
>>      g_async_queue_push(v9fs_pool.completed, co);
>>      do {
>>          len = write(v9fs_pool.wfd,&byte, sizeof(byte));
>>      } while (len == -1&&  errno == EINTR);
>> }
>>
>> void process_req_done(void *arg)
>> {
>>      Coroutine *co;
>>      char byte;
>>      ssize_t len;
>>
>>      do {
>>          len = read(v9fs_pool.rfd,&byte, sizeof(byte));
>>      } while (len == -1&&  errno == EINTR);
>>
>>      while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>>          qemu_coroutine_enter(co, NULL);
>>      }
>> }
>>
>> I typed this code out in the email, it has not been compiled or tested.
>>
>> If you decide to eliminate coroutines entirely in the future and use
>> worker threads exclusively to process requests, then there are clearly
>> marked sections in the code: anything inside v9fs_co_run_in_worker()
>> must be thread-safe already and anything outside it needs to be audited
>> and made thread-safe.  The changes required are smaller than those if
>> your current patch series was applied.  I wanted to mention this point
>> to show that this doesn't paint virtfs into a corner.
>>
>> So where does this leave virtfs?  No marshalling is necessary and
>> blocking operations can be performed inline using
>> v9fs_co_run_in_worker() blocks.  The codebase will be a lot smaller.
>>
>> Does this seem reasonable?
>>
> Do we really need bottom halfs here ? can't we achieve the same with
> v9fs_qemu_submit_request() and making the glib thread
> function callback (request.func())to do qemu_coroutine_enter()


I had the same question. :)  Tested without BH and touch testing worked 
fine.


#define v9fs_co_run_in_worker(block) \
{ \
     g_thread_pool_push(v9fs_pool.pool, qemu_coroutine_self(), NULL); \
     qemu_coroutine_yield(); /* re-entered in worker thread */ \
\
     block; \
\
     qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
}

void v9fs_qemu_process_req_done(void *arg)
{
     Coroutine *co;
     char byte;
     ssize_t len;

     do {
         len = read(v9fs_pool.rfd, &byte, sizeof(byte));
     } while (len == -1 &&  errno == EINTR);

     while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
         qemu_coroutine_enter(co, NULL);
     }
}

void v9fs_thread_routine(gpointer data, gpointer user_data)
{
     Coroutine *co = data;
     char byte = 0;
     ssize_t len;

     qemu_coroutine_enter(co, NULL);

     g_async_queue_push(v9fs_pool.completed, co);
     do {
         len = write(v9fs_pool.wfd, &byte, sizeof(byte));
     } while (len == -1 && errno == EINTR);
}

This model makes the code simple and also in one shot we can convert all 
v9fs_do_syscalls
into asynchronous threads. But as Aneesh raised will there be any 
additional overhead
for the additional jumps?  We can quickly test it out too.

For this to work; First we need to consolidate errno and then convert 
v9fs_do_syscalls
with v9fs_co_run_in_worker(). After that we can post patches folding in 
the Post* functions.
Just a thought.

Thanks,
JV

> like:
>
> int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp, struct dirent **dent)
> {
>       v9fs_wthread_enter();
>       s->ops->readdir(&s->ctx, fidp->fs.dir);
>       v9fs_wthread_exit();
> }
>
> v9fs_worker_thread_enter()
> {
>      v9fs_qemu_submit_request(v9fs_worker_request);
>      qemu_coroutine_yield();
> }
>
> v9fs_coroutine_woker_func()
> {
>        qemu_coroutine_enter(coroutine, NULL);
> }
>
>
> I also wonder whether additional bottom halfs and additional
> setcontext/setjmp that we end up with will have a performance impact compared
> to what we have currently ?
>
> -aneesh

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Venkateswararao Jujjuri]

On 05/13/2011 05:18 PM, Venkateswararao Jujjuri wrote:
> On 05/13/2011 12:26 PM, Aneesh Kumar K.V wrote:
>> On Fri, 13 May 2011 09:55:03 +0100, Stefan 
>> Hajnoczi<stefanha@gmail.com>  wrote:
>>> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri 
>>> (JV) wrote:
>>>> VirtFS (fileserver base on 9P) performs many blocking system calls 
>>>> in the
>>>> vCPU context. This effort is to move the blocking calls out of vCPU/IO
>>>> thread context, into asynchronous threads.
>>>>
>>>> Anthony's " Add hard build dependency on glib" patch and
>>>> Kevin/Stefan's coroutine effort is a prerequisite.
>>>>
>>>> This patch set contains:
>>>>   - Converting all 9pfs calls into coroutines.
>>>>   - Each 9P operation will be modified for:
>>>>      - Remove post* functions. These are our call back functions 
>>>> which makes
>>>>        the code very hard to read. Now with coroutines, we can 
>>>> achieve the same
>>>>        state machine model with nice sequential code flow.
>>>>      - Move errno access near to the local_syscall()
>>>>      - Introduce asynchronous threading
>>>>
>>>> This series has the basic infrastructure and few routines like
>>>> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
>>>> Currently we are working on converting and testing other 9P 
>>>> operations also
>>>> into this model and those patches will follow shortly.
>>>>
>>>> Removing callback functions made some of the patches little lengthy.
>>> This long patch series adds temporary structs and marshalling code for
>>> each file system operation - I think none of this is necessary.  
>>> Instead
>>> we can exploit coroutines more:
>>>
>>> The point of coroutines is that you can suspend a thread of control (a
>>> call-stack, not an OS-level thread) and can re-enter it later.  We
>>> should make coroutines thread-safe (i.e. work outside of the global
>>> mutex) and then allow switching a coroutine from a QEMU thread to a
>>> worker thread and back again:
>>>
>>> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>>>                                   struct dirent **dent)
>>> {
>>>      int ret = 0;
>>>
>>>      v9fs_co_run_in_worker({
>>>          errno = 0;
>>>          *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>>>          if (!*dent&&  errno) {
>>>              ret = -errno;
>>>          }
>>>      });
>>>      return ret;
>>> }
>>>
>>> v9fs_co_readdir() can be called from a QEMU thread.  The block of code
>>> inside v9fs_co_run_in_worker() will be executed in a worker thread.
>>> Notice that no marshalling variables is necessary at all; we can use 
>>> the
>>> function arguments and local variables because this is still the same
>>> function!
>>>
>>> When control reaches the end of the v9fs_co_run_in_worker() block,
>>> execution is resumed in a QEMU thread and the function then returns 
>>> ret.
>>> It would be incorrect to return inside the v9fs_co_run_in_worker() 
>>> block
>>> because at that point we're still inside the worker thread.
>>>
>>> Here is how v9fs_co_run_in_worker() does its magic:
>>>
>>> #define v9fs_co_run_in_worker(block) \
>>> { \
>>>      BH *co_bh; \
>>> \
>>>      co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>>>      qemu_bh_schedule(co_bh); \
>>>      qemu_coroutine_yield(); /* re-entered in worker thread */ \
>>>      qemu_bh_delete(co_bh); \
>>> \
>>>      block; \
>>> \
>>>      qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>>> }
>>>
>>> void co_run_in_worker_bh(void *opaque)
>>> {
>>>      Coroutine *co = opaque;
>>>
>>>      g_thread_pool_push(pool, co, NULL);
>>> }
>>>
>>> void worker_thread_fn(gpointer data, gpointer user_data)
>>> {
>>>      Coroutine *co = user_data;
>>>      char byte = 0;
>>>      ssize_t len;
>>>
>>>      qemu_coroutine_enter(co, NULL);
>>>
>>>      g_async_queue_push(v9fs_pool.completed, co);
>>>      do {
>>>          len = write(v9fs_pool.wfd,&byte, sizeof(byte));
>>>      } while (len == -1&&  errno == EINTR);
>>> }
>>>
>>> void process_req_done(void *arg)
>>> {
>>>      Coroutine *co;
>>>      char byte;
>>>      ssize_t len;
>>>
>>>      do {
>>>          len = read(v9fs_pool.rfd,&byte, sizeof(byte));
>>>      } while (len == -1&&  errno == EINTR);
>>>
>>>      while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != 
>>> NULL) {
>>>          qemu_coroutine_enter(co, NULL);
>>>      }
>>> }
>>>
>>> I typed this code out in the email, it has not been compiled or tested.
>>>
>>> If you decide to eliminate coroutines entirely in the future and use
>>> worker threads exclusively to process requests, then there are clearly
>>> marked sections in the code: anything inside v9fs_co_run_in_worker()
>>> must be thread-safe already and anything outside it needs to be audited
>>> and made thread-safe.  The changes required are smaller than those if
>>> your current patch series was applied.  I wanted to mention this point
>>> to show that this doesn't paint virtfs into a corner.
>>>
>>> So where does this leave virtfs?  No marshalling is necessary and
>>> blocking operations can be performed inline using
>>> v9fs_co_run_in_worker() blocks.  The codebase will be a lot smaller.
>>>
>>> Does this seem reasonable?
>>>
>> Do we really need bottom halfs here ? can't we achieve the same with
>> v9fs_qemu_submit_request() and making the glib thread
>> function callback (request.func())to do qemu_coroutine_enter()
>
>
> I had the same question. :)  Tested without BH and touch testing 
> worked fine.
>
>
> #define v9fs_co_run_in_worker(block) \
> { \
>     g_thread_pool_push(v9fs_pool.pool, qemu_coroutine_self(), NULL); \
>     qemu_coroutine_yield(); /* re-entered in worker thread */ \
> \
>     block; \
> \
>     qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
> }

I guess there is a need for BH. :)
Without that .. little stress with smp=2 causing

Co-routine re-entered recursively
Aborted

- JV




>
> void v9fs_qemu_process_req_done(void *arg)
> {
>     Coroutine *co;
>     char byte;
>     ssize_t len;
>
>     do {
>         len = read(v9fs_pool.rfd, &byte, sizeof(byte));
>     } while (len == -1 &&  errno == EINTR);
>
>     while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>         qemu_coroutine_enter(co, NULL);
>     }
> }
>
> void v9fs_thread_routine(gpointer data, gpointer user_data)
> {
>     Coroutine *co = data;
>     char byte = 0;
>     ssize_t len;
>
>     qemu_coroutine_enter(co, NULL);
>
>     g_async_queue_push(v9fs_pool.completed, co);
>     do {
>         len = write(v9fs_pool.wfd, &byte, sizeof(byte));
>     } while (len == -1 && errno == EINTR);
> }
>
> This model makes the code simple and also in one shot we can convert 
> all v9fs_do_syscalls
> into asynchronous threads. But as Aneesh raised will there be any 
> additional overhead
> for the additional jumps?  We can quickly test it out too.
>
> For this to work; First we need to consolidate errno and then convert 
> v9fs_do_syscalls
> with v9fs_co_run_in_worker(). After that we can post patches folding 
> in the Post* functions.
> Just a thought.
>
> Thanks,
> JV
>
>> like:
>>
>> int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp, struct dirent 
>> **dent)
>> {
>>       v9fs_wthread_enter();
>>       s->ops->readdir(&s->ctx, fidp->fs.dir);
>>       v9fs_wthread_exit();
>> }
>>
>> v9fs_worker_thread_enter()
>> {
>>      v9fs_qemu_submit_request(v9fs_worker_request);
>>      qemu_coroutine_yield();
>> }
>>
>> v9fs_coroutine_woker_func()
>> {
>>        qemu_coroutine_enter(coroutine, NULL);
>> }
>>
>>
>> I also wonder whether additional bottom halfs and additional
>> setcontext/setjmp that we end up with will have a performance impact 
>> compared
>> to what we have currently ?
>>
>> -aneesh
>
>

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Stefan Hajnoczi]

On Sat, May 14, 2011 at 2:29 AM, Venkateswararao Jujjuri
<jvrao@linux.vnet.ibm.com> wrote:
> On 05/13/2011 05:18 PM, Venkateswararao Jujjuri wrote:
>>
>> On 05/13/2011 12:26 PM, Aneesh Kumar K.V wrote:
>>>
>>> On Fri, 13 May 2011 09:55:03 +0100, Stefan Hajnoczi<stefanha@gmail.com>
>>>  wrote:
>>>>
>>>> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV)
>>>> wrote:
>>>>>
>>>>> VirtFS (fileserver base on 9P) performs many blocking system calls in
>>>>> the
>>>>> vCPU context. This effort is to move the blocking calls out of vCPU/IO
>>>>> thread context, into asynchronous threads.
>>>>>
>>>>> Anthony's " Add hard build dependency on glib" patch and
>>>>> Kevin/Stefan's coroutine effort is a prerequisite.
>>>>>
>>>>> This patch set contains:
>>>>>  - Converting all 9pfs calls into coroutines.
>>>>>  - Each 9P operation will be modified for:
>>>>>     - Remove post* functions. These are our call back functions which
>>>>> makes
>>>>>       the code very hard to read. Now with coroutines, we can achieve
>>>>> the same
>>>>>       state machine model with nice sequential code flow.
>>>>>     - Move errno access near to the local_syscall()
>>>>>     - Introduce asynchronous threading
>>>>>
>>>>> This series has the basic infrastructure and few routines like
>>>>> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
>>>>> Currently we are working on converting and testing other 9P operations
>>>>> also
>>>>> into this model and those patches will follow shortly.
>>>>>
>>>>> Removing callback functions made some of the patches little lengthy.
>>>>
>>>> This long patch series adds temporary structs and marshalling code for
>>>> each file system operation - I think none of this is necessary.  Instead
>>>> we can exploit coroutines more:
>>>>
>>>> The point of coroutines is that you can suspend a thread of control (a
>>>> call-stack, not an OS-level thread) and can re-enter it later.  We
>>>> should make coroutines thread-safe (i.e. work outside of the global
>>>> mutex) and then allow switching a coroutine from a QEMU thread to a
>>>> worker thread and back again:
>>>>
>>>> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>>>>                                  struct dirent **dent)
>>>> {
>>>>     int ret = 0;
>>>>
>>>>     v9fs_co_run_in_worker({
>>>>         errno = 0;
>>>>         *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>>>>         if (!*dent&&  errno) {
>>>>             ret = -errno;
>>>>         }
>>>>     });
>>>>     return ret;
>>>> }
>>>>
>>>> v9fs_co_readdir() can be called from a QEMU thread.  The block of code
>>>> inside v9fs_co_run_in_worker() will be executed in a worker thread.
>>>> Notice that no marshalling variables is necessary at all; we can use the
>>>> function arguments and local variables because this is still the same
>>>> function!
>>>>
>>>> When control reaches the end of the v9fs_co_run_in_worker() block,
>>>> execution is resumed in a QEMU thread and the function then returns ret.
>>>> It would be incorrect to return inside the v9fs_co_run_in_worker() block
>>>> because at that point we're still inside the worker thread.
>>>>
>>>> Here is how v9fs_co_run_in_worker() does its magic:
>>>>
>>>> #define v9fs_co_run_in_worker(block) \
>>>> { \
>>>>     BH *co_bh; \
>>>> \
>>>>     co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>>>>     qemu_bh_schedule(co_bh); \
>>>>     qemu_coroutine_yield(); /* re-entered in worker thread */ \
>>>>     qemu_bh_delete(co_bh); \
>>>> \
>>>>     block; \
>>>> \
>>>>     qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>>>> }
>>>>
>>>> void co_run_in_worker_bh(void *opaque)
>>>> {
>>>>     Coroutine *co = opaque;
>>>>
>>>>     g_thread_pool_push(pool, co, NULL);
>>>> }
>>>>
>>>> void worker_thread_fn(gpointer data, gpointer user_data)
>>>> {
>>>>     Coroutine *co = user_data;
>>>>     char byte = 0;
>>>>     ssize_t len;
>>>>
>>>>     qemu_coroutine_enter(co, NULL);
>>>>
>>>>     g_async_queue_push(v9fs_pool.completed, co);
>>>>     do {
>>>>         len = write(v9fs_pool.wfd,&byte, sizeof(byte));
>>>>     } while (len == -1&&  errno == EINTR);
>>>> }
>>>>
>>>> void process_req_done(void *arg)
>>>> {
>>>>     Coroutine *co;
>>>>     char byte;
>>>>     ssize_t len;
>>>>
>>>>     do {
>>>>         len = read(v9fs_pool.rfd,&byte, sizeof(byte));
>>>>     } while (len == -1&&  errno == EINTR);
>>>>
>>>>     while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>>>>         qemu_coroutine_enter(co, NULL);
>>>>     }
>>>> }
>>>>
>>>> I typed this code out in the email, it has not been compiled or tested.
>>>>
>>>> If you decide to eliminate coroutines entirely in the future and use
>>>> worker threads exclusively to process requests, then there are clearly
>>>> marked sections in the code: anything inside v9fs_co_run_in_worker()
>>>> must be thread-safe already and anything outside it needs to be audited
>>>> and made thread-safe.  The changes required are smaller than those if
>>>> your current patch series was applied.  I wanted to mention this point
>>>> to show that this doesn't paint virtfs into a corner.
>>>>
>>>> So where does this leave virtfs?  No marshalling is necessary and
>>>> blocking operations can be performed inline using
>>>> v9fs_co_run_in_worker() blocks.  The codebase will be a lot smaller.
>>>>
>>>> Does this seem reasonable?
>>>>
>>> Do we really need bottom halfs here ? can't we achieve the same with
>>> v9fs_qemu_submit_request() and making the glib thread
>>> function callback (request.func())to do qemu_coroutine_enter()
>>
>>
>> I had the same question. :)  Tested without BH and touch testing worked
>> fine.
>>
>>
>> #define v9fs_co_run_in_worker(block) \
>> { \
>>    g_thread_pool_push(v9fs_pool.pool, qemu_coroutine_self(), NULL); \
>>    qemu_coroutine_yield(); /* re-entered in worker thread */ \
>> \
>>    block; \
>> \
>>    qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>> }
>
> I guess there is a need for BH. :)
> Without that .. little stress with smp=2 causing
>
> Co-routine re-entered recursively
> Aborted

Right.  It's for synchronization:
1. Yield the coroutine in the QEMU thread.
2. Submit the coroutine to a worker thread.
3. Enter the coroutine in the worker thread.

But it's not safe to swap steps 1 and 2.  Otherwise the worker thread
could enter the coroutine before step 2 while it is still running.

Stefan

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Jamie Lokier]

Venkateswararao Jujjuri wrote:
> This model makes the code simple and also in one shot we can convert
> all v9fs_do_syscalls into asynchronous threads. But as Aneesh raised
> will there be any additional overhead for the additional jumps?  We
> can quickly test it out too.

I'm not sure if this is exactly the right place (I haven't followed
the whole discussion), but there is a useful trick for getting rid of
one of the thread context switches:

Swizzle *which* thread is your "main" coroutine thread.

Instead of queuing up an item on the work queue, waking the worker
thread pool, and having a worker thread pick up the coroutine, you:

Declare the current thread to *be* a worker through from this point,
and queue the calling context for a worker thread to pick up.  When it
picks it up, *that* thread declares itself to be the main thread
coroutine thread.

So the coroutine entry step is just queuing a context for another
thread to pick up, and then diving into the blocking system call
(optimising out the enqueue/dequeue and thread switch).

In a sense, you make the "main" thread a long-lived work queue entry,
and have a symmetric pool, except that the main thread tends to behave
differently than the other work items.

This only works if the main thread's state is able to follow the
swizzling.  I don't know if KVM VCPUs will do that, for example, or if
there's other per-thread state that won't work.

If the main thread can't be swizzled, you can still use this trick
when doing the coroutine->syscall step starting form an existing
worker thread.

-- Jamie

Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines.

[Stefan Hajnoczi]

On Tue, May 24, 2011 at 10:00 PM, Jamie Lokier <jamie@shareable.org> wrote:
> Venkateswararao Jujjuri wrote:
>> This model makes the code simple and also in one shot we can convert
>> all v9fs_do_syscalls into asynchronous threads. But as Aneesh raised
>> will there be any additional overhead for the additional jumps?  We
>> can quickly test it out too.
>
> I'm not sure if this is exactly the right place (I haven't followed
> the whole discussion), but there is a useful trick for getting rid of
> one of the thread context switches:
>
> Swizzle *which* thread is your "main" coroutine thread.
>
> Instead of queuing up an item on the work queue, waking the worker
> thread pool, and having a worker thread pick up the coroutine, you:
>
> Declare the current thread to *be* a worker through from this point,
> and queue the calling context for a worker thread to pick up.  When it
> picks it up, *that* thread declares itself to be the main thread
> coroutine thread.
>
> So the coroutine entry step is just queuing a context for another
> thread to pick up, and then diving into the blocking system call
> (optimising out the enqueue/dequeue and thread switch).
>
> In a sense, you make the "main" thread a long-lived work queue entry,
> and have a symmetric pool, except that the main thread tends to behave
> differently than the other work items.
>
> This only works if the main thread's state is able to follow the
> swizzling.  I don't know if KVM VCPUs will do that, for example, or if
> there's other per-thread state that won't work.
>
> If the main thread can't be swizzled, you can still use this trick
> when doing the coroutine->syscall step starting form an existing
> worker thread.

The point of this trick is to reduce the latency of issuing the
blocking operation?  You still have to tell another thread that it has
now become the main thread, so this seems like an optimization biased
towards I/O latency at the expense of main loop performance.

If the "main" thread is a vcpu thread, I'm not sure we want to
schedule another thread to become the vcpu thread since this adds an
unbounded time until the new vcpu thread is scheduled.  The vcpu
thread is supposed to be non-blocking so that we don't steal time from
the guest and pause execution.

Stefan