All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/4] 9pfs: clean-up for multiple transports
@ 2016-11-28 21:24 ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:24 UTC (permalink / raw)
  To: aneesh.kumar
  Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard, wei.liu2

Hi all,

this small patch series provides a few fixes and clean-ups in
preparation for the introduction of a 9pfs Xen transport.

Changes in v2:
- constify virtio_9p_transport and V9fsTransport
- assert !s->transport.
- code style
- add a call to qemu_iovec_destroy
- fix commit description of patch #3
- introduce init_out/in_iov_from_pdu


Stefano Stabellini (4):
      9pfs: move pdus to V9fsState
      9pfs: introduce transport specific callbacks
      9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
      9pfs: introduce init_out/in_iov_from_pdu

 hw/9pfs/9p.c               | 76 ++++++++++++++++++++++++----------------------
 hw/9pfs/9p.h               | 22 ++++++++++++++
 hw/9pfs/virtio-9p-device.c | 46 +++++++++++++++++++---------
 hw/9pfs/virtio-9p.h        | 10 ------
 4 files changed, 94 insertions(+), 60 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 0/4] 9pfs: clean-up for multiple transports
@ 2016-11-28 21:24 ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:24 UTC (permalink / raw)
  To: aneesh.kumar
  Cc: xen-devel, wei.liu2, groug, qemu-devel, sstabellini, anthony.perard

Hi all,

this small patch series provides a few fixes and clean-ups in
preparation for the introduction of a 9pfs Xen transport.

Changes in v2:
- constify virtio_9p_transport and V9fsTransport
- assert !s->transport.
- code style
- add a call to qemu_iovec_destroy
- fix commit description of patch #3
- introduce init_out/in_iov_from_pdu


Stefano Stabellini (4):
      9pfs: move pdus to V9fsState
      9pfs: introduce transport specific callbacks
      9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
      9pfs: introduce init_out/in_iov_from_pdu

 hw/9pfs/9p.c               | 76 ++++++++++++++++++++++++----------------------
 hw/9pfs/9p.h               | 22 ++++++++++++++
 hw/9pfs/virtio-9p-device.c | 46 +++++++++++++++++++---------
 hw/9pfs/virtio-9p.h        | 10 ------
 4 files changed, 94 insertions(+), 60 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v2 1/4] 9pfs: move pdus to V9fsState
  2016-11-28 21:24 ` Stefano Stabellini
@ 2016-11-28 21:27   ` Stefano Stabellini
  -1 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard

pdus are initialized and used in 9pfs common code. Move the array from
V9fsVirtioState to V9fsState.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p.c        | 7 +++----
 hw/9pfs/9p.h        | 1 +
 hw/9pfs/virtio-9p.h | 1 -
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index aea7e9d..05e950f 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3440,7 +3440,6 @@ void pdu_submit(V9fsPDU *pdu)
 /* Returns 0 on success, 1 on failure. */
 int v9fs_device_realize_common(V9fsState *s, Error **errp)
 {
-    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     int i, len;
     struct stat stat;
     FsDriverEntry *fse;
@@ -3451,9 +3450,9 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
     QLIST_INIT(&s->free_list);
     QLIST_INIT(&s->active_list);
     for (i = 0; i < (MAX_REQ - 1); i++) {
-        QLIST_INSERT_HEAD(&s->free_list, &v->pdus[i], next);
-        v->pdus[i].s = s;
-        v->pdus[i].idx = i;
+        QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next);
+        s->pdus[i].s = s;
+        s->pdus[i].idx = i;
     }
 
     v9fs_path_init(&path);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 3976b7f..07cee01 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -229,6 +229,7 @@ typedef struct V9fsState
     char *tag;
     enum p9_proto_version proto_version;
     int32_t msize;
+    V9fsPDU pdus[MAX_REQ];
     /*
      * lock ensuring atomic path update
      * on rename.
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 25c47c7..52c4b9d 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -10,7 +10,6 @@ typedef struct V9fsVirtioState
     VirtIODevice parent_obj;
     VirtQueue *vq;
     size_t config_size;
-    V9fsPDU pdus[MAX_REQ];
     VirtQueueElement *elems[MAX_REQ];
     V9fsState state;
 } V9fsVirtioState;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 1/4] 9pfs: move pdus to V9fsState
@ 2016-11-28 21:27   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: xen-devel, anthony.perard, sstabellini, groug, qemu-devel

pdus are initialized and used in 9pfs common code. Move the array from
V9fsVirtioState to V9fsState.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p.c        | 7 +++----
 hw/9pfs/9p.h        | 1 +
 hw/9pfs/virtio-9p.h | 1 -
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index aea7e9d..05e950f 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3440,7 +3440,6 @@ void pdu_submit(V9fsPDU *pdu)
 /* Returns 0 on success, 1 on failure. */
 int v9fs_device_realize_common(V9fsState *s, Error **errp)
 {
-    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     int i, len;
     struct stat stat;
     FsDriverEntry *fse;
@@ -3451,9 +3450,9 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
     QLIST_INIT(&s->free_list);
     QLIST_INIT(&s->active_list);
     for (i = 0; i < (MAX_REQ - 1); i++) {
-        QLIST_INSERT_HEAD(&s->free_list, &v->pdus[i], next);
-        v->pdus[i].s = s;
-        v->pdus[i].idx = i;
+        QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next);
+        s->pdus[i].s = s;
+        s->pdus[i].idx = i;
     }
 
     v9fs_path_init(&path);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 3976b7f..07cee01 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -229,6 +229,7 @@ typedef struct V9fsState
     char *tag;
     enum p9_proto_version proto_version;
     int32_t msize;
+    V9fsPDU pdus[MAX_REQ];
     /*
      * lock ensuring atomic path update
      * on rename.
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 25c47c7..52c4b9d 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -10,7 +10,6 @@ typedef struct V9fsVirtioState
     VirtIODevice parent_obj;
     VirtQueue *vq;
     size_t config_size;
-    V9fsPDU pdus[MAX_REQ];
     VirtQueueElement *elems[MAX_REQ];
     V9fsState state;
 } V9fsVirtioState;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v2 2/4] 9pfs: introduce transport specific callbacks
  2016-11-28 21:27   ` Stefano Stabellini
@ 2016-11-28 21:27     ` Stefano Stabellini
  -1 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard

Don't call virtio functions from 9pfs generic code, use generic function
callbacks instead.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---
Changes in v2:
- constify virtio_9p_transport and V9fsTransport
- assert !s->transport.
- code style
---
 hw/9pfs/9p.c               |  8 ++++----
 hw/9pfs/9p.h               | 19 +++++++++++++++++++
 hw/9pfs/virtio-9p-device.c | 24 +++++++++++++++++-------
 hw/9pfs/virtio-9p.h        |  9 ---------
 4 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 05e950f..5a20a13 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -47,7 +47,7 @@ ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap);
+    ret = pdu->s->transport->pdu_vmarshal(pdu, offset, fmt, ap);
     va_end(ap);
 
     return ret;
@@ -59,7 +59,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap);
+    ret = pdu->s->transport->pdu_vunmarshal(pdu, offset, fmt, ap);
     va_end(ap);
 
     return ret;
@@ -67,7 +67,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
 
 static void pdu_push_and_notify(V9fsPDU *pdu)
 {
-    virtio_9p_push_and_notify(pdu);
+    pdu->s->transport->push_and_notify(pdu);
 }
 
 static int omode_to_uflags(int8_t mode)
@@ -1751,7 +1751,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
     struct iovec *iov;
     unsigned int niov;
 
-    virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write);
+    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
 
     qemu_iovec_init_external(&elem, iov, niov);
     qemu_iovec_init(qiov, niov);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 07cee01..c8c9aa8 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -230,6 +230,7 @@ typedef struct V9fsState
     enum p9_proto_version proto_version;
     int32_t msize;
     V9fsPDU pdus[MAX_REQ];
+    const struct V9fsTransport *transport;
     /*
      * lock ensuring atomic path update
      * on rename.
@@ -343,4 +344,22 @@ void pdu_free(V9fsPDU *pdu);
 void pdu_submit(V9fsPDU *pdu);
 void v9fs_reset(V9fsState *s);
 
+struct V9fsTransport {
+    ssize_t     (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
+                                va_list ap);
+    ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
+                                  va_list ap);
+    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                     unsigned int *pniov, bool is_write);
+    void        (*push_and_notify)(V9fsPDU *pdu);
+};
+
+static inline int v9fs_register_transport(V9fsState *s,
+        const struct V9fsTransport *t)
+{
+    assert(!s->transport);
+    s->transport = t;
+    return 0;
+}
+
 #endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1782e4a..273425b 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -20,7 +20,9 @@
 #include "hw/virtio/virtio-access.h"
 #include "qemu/iov.h"
 
-void virtio_9p_push_and_notify(V9fsPDU *pdu)
+static const struct V9fsTransport virtio_9p_transport;
+
+static void virtio_9p_push_and_notify(V9fsPDU *pdu)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -126,6 +128,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
     v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
     virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
     v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
+    v9fs_register_transport(s, &virtio_9p_transport);
 
 out:
     return;
@@ -148,8 +151,8 @@ static void virtio_9p_reset(VirtIODevice *vdev)
     v9fs_reset(&v->state);
 }
 
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
-                            const char *fmt, va_list ap)
+static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
+                                   const char *fmt, va_list ap)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -158,8 +161,8 @@ ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
 }
 
-ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
-                              const char *fmt, va_list ap)
+static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
+                                     const char *fmt, va_list ap)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -168,8 +171,8 @@ ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
 }
 
-void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                              unsigned int *pniov, bool is_write)
+static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                     unsigned int *pniov, bool is_write)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
     }
 }
 
+static const struct V9fsTransport virtio_9p_transport = {
+    .pdu_vmarshal = virtio_pdu_vmarshal,
+    .pdu_vunmarshal = virtio_pdu_vunmarshal,
+    .init_iov_from_pdu = virtio_init_iov_from_pdu,
+    .push_and_notify = virtio_9p_push_and_notify,
+};
+
 /* virtio-9p device */
 
 static const VMStateDescription vmstate_virtio_9p = {
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 52c4b9d..e763da2c 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -14,15 +14,6 @@ typedef struct V9fsVirtioState
     V9fsState state;
 } V9fsVirtioState;
 
-void virtio_9p_push_and_notify(V9fsPDU *pdu);
-
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
-                            const char *fmt, va_list ap);
-ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
-                              const char *fmt, va_list ap);
-void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                              unsigned int *pniov, bool is_write);
-
 #define TYPE_VIRTIO_9P "virtio-9p-device"
 #define VIRTIO_9P(obj) \
         OBJECT_CHECK(V9fsVirtioState, (obj), TYPE_VIRTIO_9P)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-28 21:27     ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: xen-devel, anthony.perard, sstabellini, groug, qemu-devel

Don't call virtio functions from 9pfs generic code, use generic function
callbacks instead.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---
Changes in v2:
- constify virtio_9p_transport and V9fsTransport
- assert !s->transport.
- code style
---
 hw/9pfs/9p.c               |  8 ++++----
 hw/9pfs/9p.h               | 19 +++++++++++++++++++
 hw/9pfs/virtio-9p-device.c | 24 +++++++++++++++++-------
 hw/9pfs/virtio-9p.h        |  9 ---------
 4 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 05e950f..5a20a13 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -47,7 +47,7 @@ ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap);
+    ret = pdu->s->transport->pdu_vmarshal(pdu, offset, fmt, ap);
     va_end(ap);
 
     return ret;
@@ -59,7 +59,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap);
+    ret = pdu->s->transport->pdu_vunmarshal(pdu, offset, fmt, ap);
     va_end(ap);
 
     return ret;
@@ -67,7 +67,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
 
 static void pdu_push_and_notify(V9fsPDU *pdu)
 {
-    virtio_9p_push_and_notify(pdu);
+    pdu->s->transport->push_and_notify(pdu);
 }
 
 static int omode_to_uflags(int8_t mode)
@@ -1751,7 +1751,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
     struct iovec *iov;
     unsigned int niov;
 
-    virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write);
+    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
 
     qemu_iovec_init_external(&elem, iov, niov);
     qemu_iovec_init(qiov, niov);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 07cee01..c8c9aa8 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -230,6 +230,7 @@ typedef struct V9fsState
     enum p9_proto_version proto_version;
     int32_t msize;
     V9fsPDU pdus[MAX_REQ];
+    const struct V9fsTransport *transport;
     /*
      * lock ensuring atomic path update
      * on rename.
@@ -343,4 +344,22 @@ void pdu_free(V9fsPDU *pdu);
 void pdu_submit(V9fsPDU *pdu);
 void v9fs_reset(V9fsState *s);
 
+struct V9fsTransport {
+    ssize_t     (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
+                                va_list ap);
+    ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
+                                  va_list ap);
+    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                     unsigned int *pniov, bool is_write);
+    void        (*push_and_notify)(V9fsPDU *pdu);
+};
+
+static inline int v9fs_register_transport(V9fsState *s,
+        const struct V9fsTransport *t)
+{
+    assert(!s->transport);
+    s->transport = t;
+    return 0;
+}
+
 #endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1782e4a..273425b 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -20,7 +20,9 @@
 #include "hw/virtio/virtio-access.h"
 #include "qemu/iov.h"
 
-void virtio_9p_push_and_notify(V9fsPDU *pdu)
+static const struct V9fsTransport virtio_9p_transport;
+
+static void virtio_9p_push_and_notify(V9fsPDU *pdu)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -126,6 +128,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
     v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
     virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
     v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
+    v9fs_register_transport(s, &virtio_9p_transport);
 
 out:
     return;
@@ -148,8 +151,8 @@ static void virtio_9p_reset(VirtIODevice *vdev)
     v9fs_reset(&v->state);
 }
 
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
-                            const char *fmt, va_list ap)
+static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
+                                   const char *fmt, va_list ap)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -158,8 +161,8 @@ ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
 }
 
-ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
-                              const char *fmt, va_list ap)
+static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
+                                     const char *fmt, va_list ap)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -168,8 +171,8 @@ ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
 }
 
-void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                              unsigned int *pniov, bool is_write)
+static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                     unsigned int *pniov, bool is_write)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
@@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
     }
 }
 
+static const struct V9fsTransport virtio_9p_transport = {
+    .pdu_vmarshal = virtio_pdu_vmarshal,
+    .pdu_vunmarshal = virtio_pdu_vunmarshal,
+    .init_iov_from_pdu = virtio_init_iov_from_pdu,
+    .push_and_notify = virtio_9p_push_and_notify,
+};
+
 /* virtio-9p device */
 
 static const VMStateDescription vmstate_virtio_9p = {
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 52c4b9d..e763da2c 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -14,15 +14,6 @@ typedef struct V9fsVirtioState
     V9fsState state;
 } V9fsVirtioState;
 
-void virtio_9p_push_and_notify(V9fsPDU *pdu);
-
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
-                            const char *fmt, va_list ap);
-ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
-                              const char *fmt, va_list ap);
-void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                              unsigned int *pniov, bool is_write);
-
 #define TYPE_VIRTIO_9P "virtio-9p-device"
 #define VIRTIO_9P(obj) \
         OBJECT_CHECK(V9fsVirtioState, (obj), TYPE_VIRTIO_9P)
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v2 3/4] 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack
  2016-11-28 21:27   ` Stefano Stabellini
@ 2016-11-28 21:27     ` Stefano Stabellini
  -1 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard

v9fs_xattr_read should not access VirtQueueElement elems directly.
Move v9fs_init_qiov_from_pdu up in the file and call
v9fs_init_qiov_from_pdu before v9fs_pack. Use v9fs_pack on the new
iovec.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---
Changes in v2:
- add a call to qemu_iovec_destroy
- fix commit description
---
 hw/9pfs/9p.c | 59 ++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 5a20a13..79d7201 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1633,14 +1633,39 @@ out_nofid:
     pdu_complete(pdu, err);
 }
 
+/*
+ * Create a QEMUIOVector for a sub-region of PDU iovecs
+ *
+ * @qiov:       uninitialized QEMUIOVector
+ * @skip:       number of bytes to skip from beginning of PDU
+ * @size:       number of bytes to include
+ * @is_write:   true - write, false - read
+ *
+ * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
+ * with qemu_iovec_destroy().
+ */
+static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
+                                    size_t skip, size_t size,
+                                    bool is_write)
+{
+    QEMUIOVector elem;
+    struct iovec *iov;
+    unsigned int niov;
+
+    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
+
+    qemu_iovec_init_external(&elem, iov, niov);
+    qemu_iovec_init(qiov, niov);
+    qemu_iovec_concat(qiov, &elem, skip, size);
+}
+
 static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
                            uint64_t off, uint32_t max_count)
 {
     ssize_t err;
     size_t offset = 7;
     uint64_t read_count;
-    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
-    VirtQueueElement *elem = v->elems[pdu->idx];
+    QEMUIOVector qiov_full;
 
     if (fidp->fs.xattr.len < off) {
         read_count = 0;
@@ -1656,9 +1681,11 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
     }
     offset += err;
 
-    err = v9fs_pack(elem->in_sg, elem->in_num, offset,
+    v9fs_init_qiov_from_pdu(&qiov_full, pdu, 0, read_count, false);
+    err = v9fs_pack(qiov_full.iov, qiov_full.niov, offset,
                     ((char *)fidp->fs.xattr.value) + off,
                     read_count);
+    qemu_iovec_destroy(&qiov_full);
     if (err < 0) {
         return err;
     }
@@ -1732,32 +1759,6 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
     return count;
 }
 
-/*
- * Create a QEMUIOVector for a sub-region of PDU iovecs
- *
- * @qiov:       uninitialized QEMUIOVector
- * @skip:       number of bytes to skip from beginning of PDU
- * @size:       number of bytes to include
- * @is_write:   true - write, false - read
- *
- * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
- * with qemu_iovec_destroy().
- */
-static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
-                                    size_t skip, size_t size,
-                                    bool is_write)
-{
-    QEMUIOVector elem;
-    struct iovec *iov;
-    unsigned int niov;
-
-    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
-
-    qemu_iovec_init_external(&elem, iov, niov);
-    qemu_iovec_init(qiov, niov);
-    qemu_iovec_concat(qiov, &elem, skip, size);
-}
-
 static void coroutine_fn v9fs_read(void *opaque)
 {
     int32_t fid;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/4] 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack
@ 2016-11-28 21:27     ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: xen-devel, anthony.perard, sstabellini, groug, qemu-devel

v9fs_xattr_read should not access VirtQueueElement elems directly.
Move v9fs_init_qiov_from_pdu up in the file and call
v9fs_init_qiov_from_pdu before v9fs_pack. Use v9fs_pack on the new
iovec.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---
Changes in v2:
- add a call to qemu_iovec_destroy
- fix commit description
---
 hw/9pfs/9p.c | 59 ++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 5a20a13..79d7201 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1633,14 +1633,39 @@ out_nofid:
     pdu_complete(pdu, err);
 }
 
+/*
+ * Create a QEMUIOVector for a sub-region of PDU iovecs
+ *
+ * @qiov:       uninitialized QEMUIOVector
+ * @skip:       number of bytes to skip from beginning of PDU
+ * @size:       number of bytes to include
+ * @is_write:   true - write, false - read
+ *
+ * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
+ * with qemu_iovec_destroy().
+ */
+static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
+                                    size_t skip, size_t size,
+                                    bool is_write)
+{
+    QEMUIOVector elem;
+    struct iovec *iov;
+    unsigned int niov;
+
+    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
+
+    qemu_iovec_init_external(&elem, iov, niov);
+    qemu_iovec_init(qiov, niov);
+    qemu_iovec_concat(qiov, &elem, skip, size);
+}
+
 static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
                            uint64_t off, uint32_t max_count)
 {
     ssize_t err;
     size_t offset = 7;
     uint64_t read_count;
-    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
-    VirtQueueElement *elem = v->elems[pdu->idx];
+    QEMUIOVector qiov_full;
 
     if (fidp->fs.xattr.len < off) {
         read_count = 0;
@@ -1656,9 +1681,11 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
     }
     offset += err;
 
-    err = v9fs_pack(elem->in_sg, elem->in_num, offset,
+    v9fs_init_qiov_from_pdu(&qiov_full, pdu, 0, read_count, false);
+    err = v9fs_pack(qiov_full.iov, qiov_full.niov, offset,
                     ((char *)fidp->fs.xattr.value) + off,
                     read_count);
+    qemu_iovec_destroy(&qiov_full);
     if (err < 0) {
         return err;
     }
@@ -1732,32 +1759,6 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
     return count;
 }
 
-/*
- * Create a QEMUIOVector for a sub-region of PDU iovecs
- *
- * @qiov:       uninitialized QEMUIOVector
- * @skip:       number of bytes to skip from beginning of PDU
- * @size:       number of bytes to include
- * @is_write:   true - write, false - read
- *
- * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
- * with qemu_iovec_destroy().
- */
-static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
-                                    size_t skip, size_t size,
-                                    bool is_write)
-{
-    QEMUIOVector elem;
-    struct iovec *iov;
-    unsigned int niov;
-
-    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
-
-    qemu_iovec_init_external(&elem, iov, niov);
-    qemu_iovec_init(qiov, niov);
-    qemu_iovec_concat(qiov, &elem, skip, size);
-}
-
 static void coroutine_fn v9fs_read(void *opaque)
 {
     int32_t fid;
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
  2016-11-28 21:27   ` Stefano Stabellini
@ 2016-11-28 21:27     ` Stefano Stabellini
  -1 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard

Not all 9pfs transports share memory between request and response. For
those who don't, it is necessary to know how much memory is required in
the response.

Split the existing init_iov_from_pdu function in two:
init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
init_in_iov_from_pdu takes an additional size parameter to specify the
memory required for the response message.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 hw/9pfs/9p.c               |  6 +++++-
 hw/9pfs/9p.h               |  6 ++++--
 hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 79d7201..ce1f9d9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
     struct iovec *iov;
     unsigned int niov;
 
-    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
+    if (is_write) {
+        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
+    } else {
+        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
+    }
 
     qemu_iovec_init_external(&elem, iov, niov);
     qemu_iovec_init(qiov, niov);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index c8c9aa8..4c4feaf 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -349,8 +349,10 @@ struct V9fsTransport {
                                 va_list ap);
     ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
                                   va_list ap);
-    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
-                                     unsigned int *pniov, bool is_write);
+    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                        unsigned int *pniov, size_t size);
+    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                         unsigned int *pniov);
     void        (*push_and_notify)(V9fsPDU *pdu);
 };
 
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 273425b..27a4a32 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
 }
 
-static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                                     unsigned int *pniov, bool is_write)
+/* The size parameter is used by other transports. Do not drop it. */
+static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                        unsigned int *pniov, size_t size)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     VirtQueueElement *elem = v->elems[pdu->idx];
 
-    if (is_write) {
-        *piov = elem->out_sg;
-        *pniov = elem->out_num;
-    } else {
-        *piov = elem->in_sg;
-        *pniov = elem->in_num;
-    }
+    *piov = elem->in_sg;
+    *pniov = elem->in_num;
+}
+
+static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                         unsigned int *pniov)
+{
+    V9fsState *s = pdu->s;
+    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
+    VirtQueueElement *elem = v->elems[pdu->idx];
+
+    *piov = elem->out_sg;
+    *pniov = elem->out_num;
 }
 
 static const struct V9fsTransport virtio_9p_transport = {
     .pdu_vmarshal = virtio_pdu_vmarshal,
     .pdu_vunmarshal = virtio_pdu_vunmarshal,
-    .init_iov_from_pdu = virtio_init_iov_from_pdu,
+    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
+    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
     .push_and_notify = virtio_9p_push_and_notify,
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
@ 2016-11-28 21:27     ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:27 UTC (permalink / raw)
  To: aneesh.kumar; +Cc: xen-devel, anthony.perard, sstabellini, groug, qemu-devel

Not all 9pfs transports share memory between request and response. For
those who don't, it is necessary to know how much memory is required in
the response.

Split the existing init_iov_from_pdu function in two:
init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
init_in_iov_from_pdu takes an additional size parameter to specify the
memory required for the response message.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 hw/9pfs/9p.c               |  6 +++++-
 hw/9pfs/9p.h               |  6 ++++--
 hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 79d7201..ce1f9d9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
     struct iovec *iov;
     unsigned int niov;
 
-    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
+    if (is_write) {
+        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
+    } else {
+        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
+    }
 
     qemu_iovec_init_external(&elem, iov, niov);
     qemu_iovec_init(qiov, niov);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index c8c9aa8..4c4feaf 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -349,8 +349,10 @@ struct V9fsTransport {
                                 va_list ap);
     ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
                                   va_list ap);
-    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
-                                     unsigned int *pniov, bool is_write);
+    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                        unsigned int *pniov, size_t size);
+    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
+                                         unsigned int *pniov);
     void        (*push_and_notify)(V9fsPDU *pdu);
 };
 
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 273425b..27a4a32 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
     return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
 }
 
-static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-                                     unsigned int *pniov, bool is_write)
+/* The size parameter is used by other transports. Do not drop it. */
+static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                        unsigned int *pniov, size_t size)
 {
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     VirtQueueElement *elem = v->elems[pdu->idx];
 
-    if (is_write) {
-        *piov = elem->out_sg;
-        *pniov = elem->out_num;
-    } else {
-        *piov = elem->in_sg;
-        *pniov = elem->in_num;
-    }
+    *piov = elem->in_sg;
+    *pniov = elem->in_num;
+}
+
+static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
+                                         unsigned int *pniov)
+{
+    V9fsState *s = pdu->s;
+    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
+    VirtQueueElement *elem = v->elems[pdu->idx];
+
+    *piov = elem->out_sg;
+    *pniov = elem->out_num;
 }
 
 static const struct V9fsTransport virtio_9p_transport = {
     .pdu_vmarshal = virtio_pdu_vmarshal,
     .pdu_vunmarshal = virtio_pdu_vunmarshal,
-    .init_iov_from_pdu = virtio_init_iov_from_pdu,
+    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
+    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
     .push_and_notify = virtio_9p_push_and_notify,
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/4] 9pfs: introduce transport specific callbacks
  2016-11-28 21:27     ` Stefano Stabellini
@ 2016-12-02 11:03       ` Greg Kurz
  -1 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:03 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard

On Mon, 28 Nov 2016 13:27:22 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> Don't call virtio functions from 9pfs generic code, use generic function
> callbacks instead.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes in v2:
> - constify virtio_9p_transport and V9fsTransport
> - assert !s->transport.
> - code style
> ---
>  hw/9pfs/9p.c               |  8 ++++----
>  hw/9pfs/9p.h               | 19 +++++++++++++++++++
>  hw/9pfs/virtio-9p-device.c | 24 +++++++++++++++++-------
>  hw/9pfs/virtio-9p.h        |  9 ---------
>  4 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 05e950f..5a20a13 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -47,7 +47,7 @@ ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>      va_list ap;
>  
>      va_start(ap, fmt);
> -    ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap);
> +    ret = pdu->s->transport->pdu_vmarshal(pdu, offset, fmt, ap);
>      va_end(ap);
>  
>      return ret;
> @@ -59,7 +59,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>      va_list ap;
>  
>      va_start(ap, fmt);
> -    ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap);
> +    ret = pdu->s->transport->pdu_vunmarshal(pdu, offset, fmt, ap);
>      va_end(ap);
>  
>      return ret;
> @@ -67,7 +67,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>  
>  static void pdu_push_and_notify(V9fsPDU *pdu)
>  {
> -    virtio_9p_push_and_notify(pdu);
> +    pdu->s->transport->push_and_notify(pdu);
>  }
>  
>  static int omode_to_uflags(int8_t mode)
> @@ -1751,7 +1751,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
>      struct iovec *iov;
>      unsigned int niov;
>  
> -    virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
>  
>      qemu_iovec_init_external(&elem, iov, niov);
>      qemu_iovec_init(qiov, niov);
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 07cee01..c8c9aa8 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -230,6 +230,7 @@ typedef struct V9fsState
>      enum p9_proto_version proto_version;
>      int32_t msize;
>      V9fsPDU pdus[MAX_REQ];
> +    const struct V9fsTransport *transport;
>      /*
>       * lock ensuring atomic path update
>       * on rename.
> @@ -343,4 +344,22 @@ void pdu_free(V9fsPDU *pdu);
>  void pdu_submit(V9fsPDU *pdu);
>  void v9fs_reset(V9fsState *s);
>  
> +struct V9fsTransport {
> +    ssize_t     (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> +                                va_list ap);
> +    ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> +                                  va_list ap);
> +    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                     unsigned int *pniov, bool is_write);
> +    void        (*push_and_notify)(V9fsPDU *pdu);
> +};
> +
> +static inline int v9fs_register_transport(V9fsState *s,
> +        const struct V9fsTransport *t)
> +{
> +    assert(!s->transport);
> +    s->transport = t;
> +    return 0;
> +}
> +
>  #endif
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 1782e4a..273425b 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -20,7 +20,9 @@
>  #include "hw/virtio/virtio-access.h"
>  #include "qemu/iov.h"
>  
> -void virtio_9p_push_and_notify(V9fsPDU *pdu)
> +static const struct V9fsTransport virtio_9p_transport;
> +
> +static void virtio_9p_push_and_notify(V9fsPDU *pdu)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -126,6 +128,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
>      v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
>      virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
>      v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
> +    v9fs_register_transport(s, &virtio_9p_transport);
>  
>  out:
>      return;
> @@ -148,8 +151,8 @@ static void virtio_9p_reset(VirtIODevice *vdev)
>      v9fs_reset(&v->state);
>  }
>  
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> -                            const char *fmt, va_list ap)
> +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> +                                   const char *fmt, va_list ap)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -158,8 +161,8 @@ ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
>  }
>  
> -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> -                              const char *fmt, va_list ap)
> +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> +                                     const char *fmt, va_list ap)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -168,8 +171,8 @@ ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
> -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                              unsigned int *pniov, bool is_write)
> +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                     unsigned int *pniov, bool is_write)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
>      }
>  }
>  
> +static const struct V9fsTransport virtio_9p_transport = {
> +    .pdu_vmarshal = virtio_pdu_vmarshal,
> +    .pdu_vunmarshal = virtio_pdu_vunmarshal,
> +    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> +    .push_and_notify = virtio_9p_push_and_notify,
> +};
> +
>  /* virtio-9p device */
>  
>  static const VMStateDescription vmstate_virtio_9p = {
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 52c4b9d..e763da2c 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -14,15 +14,6 @@ typedef struct V9fsVirtioState
>      V9fsState state;
>  } V9fsVirtioState;
>  
> -void virtio_9p_push_and_notify(V9fsPDU *pdu);
> -
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> -                            const char *fmt, va_list ap);
> -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> -                              const char *fmt, va_list ap);
> -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                              unsigned int *pniov, bool is_write);
> -
>  #define TYPE_VIRTIO_9P "virtio-9p-device"
>  #define VIRTIO_9P(obj) \
>          OBJECT_CHECK(V9fsVirtioState, (obj), TYPE_VIRTIO_9P)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/4] 9pfs: introduce transport specific callbacks
@ 2016-12-02 11:03       ` Greg Kurz
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:03 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Mon, 28 Nov 2016 13:27:22 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> Don't call virtio functions from 9pfs generic code, use generic function
> callbacks instead.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes in v2:
> - constify virtio_9p_transport and V9fsTransport
> - assert !s->transport.
> - code style
> ---
>  hw/9pfs/9p.c               |  8 ++++----
>  hw/9pfs/9p.h               | 19 +++++++++++++++++++
>  hw/9pfs/virtio-9p-device.c | 24 +++++++++++++++++-------
>  hw/9pfs/virtio-9p.h        |  9 ---------
>  4 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 05e950f..5a20a13 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -47,7 +47,7 @@ ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>      va_list ap;
>  
>      va_start(ap, fmt);
> -    ret = virtio_pdu_vmarshal(pdu, offset, fmt, ap);
> +    ret = pdu->s->transport->pdu_vmarshal(pdu, offset, fmt, ap);
>      va_end(ap);
>  
>      return ret;
> @@ -59,7 +59,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>      va_list ap;
>  
>      va_start(ap, fmt);
> -    ret = virtio_pdu_vunmarshal(pdu, offset, fmt, ap);
> +    ret = pdu->s->transport->pdu_vunmarshal(pdu, offset, fmt, ap);
>      va_end(ap);
>  
>      return ret;
> @@ -67,7 +67,7 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
>  
>  static void pdu_push_and_notify(V9fsPDU *pdu)
>  {
> -    virtio_9p_push_and_notify(pdu);
> +    pdu->s->transport->push_and_notify(pdu);
>  }
>  
>  static int omode_to_uflags(int8_t mode)
> @@ -1751,7 +1751,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
>      struct iovec *iov;
>      unsigned int niov;
>  
> -    virtio_init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
>  
>      qemu_iovec_init_external(&elem, iov, niov);
>      qemu_iovec_init(qiov, niov);
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 07cee01..c8c9aa8 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -230,6 +230,7 @@ typedef struct V9fsState
>      enum p9_proto_version proto_version;
>      int32_t msize;
>      V9fsPDU pdus[MAX_REQ];
> +    const struct V9fsTransport *transport;
>      /*
>       * lock ensuring atomic path update
>       * on rename.
> @@ -343,4 +344,22 @@ void pdu_free(V9fsPDU *pdu);
>  void pdu_submit(V9fsPDU *pdu);
>  void v9fs_reset(V9fsState *s);
>  
> +struct V9fsTransport {
> +    ssize_t     (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> +                                va_list ap);
> +    ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> +                                  va_list ap);
> +    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                     unsigned int *pniov, bool is_write);
> +    void        (*push_and_notify)(V9fsPDU *pdu);
> +};
> +
> +static inline int v9fs_register_transport(V9fsState *s,
> +        const struct V9fsTransport *t)
> +{
> +    assert(!s->transport);
> +    s->transport = t;
> +    return 0;
> +}
> +
>  #endif
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 1782e4a..273425b 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -20,7 +20,9 @@
>  #include "hw/virtio/virtio-access.h"
>  #include "qemu/iov.h"
>  
> -void virtio_9p_push_and_notify(V9fsPDU *pdu)
> +static const struct V9fsTransport virtio_9p_transport;
> +
> +static void virtio_9p_push_and_notify(V9fsPDU *pdu)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -126,6 +128,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
>      v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
>      virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
>      v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
> +    v9fs_register_transport(s, &virtio_9p_transport);
>  
>  out:
>      return;
> @@ -148,8 +151,8 @@ static void virtio_9p_reset(VirtIODevice *vdev)
>      v9fs_reset(&v->state);
>  }
>  
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> -                            const char *fmt, va_list ap)
> +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> +                                   const char *fmt, va_list ap)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -158,8 +161,8 @@ ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
>  }
>  
> -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> -                              const char *fmt, va_list ap)
> +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> +                                     const char *fmt, va_list ap)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -168,8 +171,8 @@ ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
> -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                              unsigned int *pniov, bool is_write)
> +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                     unsigned int *pniov, bool is_write)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
>      }
>  }
>  
> +static const struct V9fsTransport virtio_9p_transport = {
> +    .pdu_vmarshal = virtio_pdu_vmarshal,
> +    .pdu_vunmarshal = virtio_pdu_vunmarshal,
> +    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> +    .push_and_notify = virtio_9p_push_and_notify,
> +};
> +
>  /* virtio-9p device */
>  
>  static const VMStateDescription vmstate_virtio_9p = {
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 52c4b9d..e763da2c 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -14,15 +14,6 @@ typedef struct V9fsVirtioState
>      V9fsState state;
>  } V9fsVirtioState;
>  
> -void virtio_9p_push_and_notify(V9fsPDU *pdu);
> -
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> -                            const char *fmt, va_list ap);
> -ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> -                              const char *fmt, va_list ap);
> -void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                              unsigned int *pniov, bool is_write);
> -
>  #define TYPE_VIRTIO_9P "virtio-9p-device"
>  #define VIRTIO_9P(obj) \
>          OBJECT_CHECK(V9fsVirtioState, (obj), TYPE_VIRTIO_9P)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack
  2016-11-28 21:27     ` Stefano Stabellini
@ 2016-12-02 11:04       ` Greg Kurz
  -1 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:04 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard

On Mon, 28 Nov 2016 13:27:23 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> v9fs_xattr_read should not access VirtQueueElement elems directly.
> Move v9fs_init_qiov_from_pdu up in the file and call
> v9fs_init_qiov_from_pdu before v9fs_pack. Use v9fs_pack on the new
> iovec.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes in v2:
> - add a call to qemu_iovec_destroy
> - fix commit description
> ---
>  hw/9pfs/9p.c | 59 ++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 5a20a13..79d7201 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1633,14 +1633,39 @@ out_nofid:
>      pdu_complete(pdu, err);
>  }
>  
> +/*
> + * Create a QEMUIOVector for a sub-region of PDU iovecs
> + *
> + * @qiov:       uninitialized QEMUIOVector
> + * @skip:       number of bytes to skip from beginning of PDU
> + * @size:       number of bytes to include
> + * @is_write:   true - write, false - read
> + *
> + * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
> + * with qemu_iovec_destroy().
> + */
> +static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> +                                    size_t skip, size_t size,
> +                                    bool is_write)
> +{
> +    QEMUIOVector elem;
> +    struct iovec *iov;
> +    unsigned int niov;
> +
> +    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +
> +    qemu_iovec_init_external(&elem, iov, niov);
> +    qemu_iovec_init(qiov, niov);
> +    qemu_iovec_concat(qiov, &elem, skip, size);
> +}
> +
>  static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>                             uint64_t off, uint32_t max_count)
>  {
>      ssize_t err;
>      size_t offset = 7;
>      uint64_t read_count;
> -    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> -    VirtQueueElement *elem = v->elems[pdu->idx];
> +    QEMUIOVector qiov_full;
>  
>      if (fidp->fs.xattr.len < off) {
>          read_count = 0;
> @@ -1656,9 +1681,11 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>      }
>      offset += err;
>  
> -    err = v9fs_pack(elem->in_sg, elem->in_num, offset,
> +    v9fs_init_qiov_from_pdu(&qiov_full, pdu, 0, read_count, false);
> +    err = v9fs_pack(qiov_full.iov, qiov_full.niov, offset,
>                      ((char *)fidp->fs.xattr.value) + off,
>                      read_count);
> +    qemu_iovec_destroy(&qiov_full);
>      if (err < 0) {
>          return err;
>      }
> @@ -1732,32 +1759,6 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>      return count;
>  }
>  
> -/*
> - * Create a QEMUIOVector for a sub-region of PDU iovecs
> - *
> - * @qiov:       uninitialized QEMUIOVector
> - * @skip:       number of bytes to skip from beginning of PDU
> - * @size:       number of bytes to include
> - * @is_write:   true - write, false - read
> - *
> - * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
> - * with qemu_iovec_destroy().
> - */
> -static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> -                                    size_t skip, size_t size,
> -                                    bool is_write)
> -{
> -    QEMUIOVector elem;
> -    struct iovec *iov;
> -    unsigned int niov;
> -
> -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> -
> -    qemu_iovec_init_external(&elem, iov, niov);
> -    qemu_iovec_init(qiov, niov);
> -    qemu_iovec_concat(qiov, &elem, skip, size);
> -}
> -
>  static void coroutine_fn v9fs_read(void *opaque)
>  {
>      int32_t fid;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/4] 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack
@ 2016-12-02 11:04       ` Greg Kurz
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:04 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Mon, 28 Nov 2016 13:27:23 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> v9fs_xattr_read should not access VirtQueueElement elems directly.
> Move v9fs_init_qiov_from_pdu up in the file and call
> v9fs_init_qiov_from_pdu before v9fs_pack. Use v9fs_pack on the new
> iovec.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

> Changes in v2:
> - add a call to qemu_iovec_destroy
> - fix commit description
> ---
>  hw/9pfs/9p.c | 59 ++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 5a20a13..79d7201 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1633,14 +1633,39 @@ out_nofid:
>      pdu_complete(pdu, err);
>  }
>  
> +/*
> + * Create a QEMUIOVector for a sub-region of PDU iovecs
> + *
> + * @qiov:       uninitialized QEMUIOVector
> + * @skip:       number of bytes to skip from beginning of PDU
> + * @size:       number of bytes to include
> + * @is_write:   true - write, false - read
> + *
> + * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
> + * with qemu_iovec_destroy().
> + */
> +static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> +                                    size_t skip, size_t size,
> +                                    bool is_write)
> +{
> +    QEMUIOVector elem;
> +    struct iovec *iov;
> +    unsigned int niov;
> +
> +    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +
> +    qemu_iovec_init_external(&elem, iov, niov);
> +    qemu_iovec_init(qiov, niov);
> +    qemu_iovec_concat(qiov, &elem, skip, size);
> +}
> +
>  static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>                             uint64_t off, uint32_t max_count)
>  {
>      ssize_t err;
>      size_t offset = 7;
>      uint64_t read_count;
> -    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> -    VirtQueueElement *elem = v->elems[pdu->idx];
> +    QEMUIOVector qiov_full;
>  
>      if (fidp->fs.xattr.len < off) {
>          read_count = 0;
> @@ -1656,9 +1681,11 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
>      }
>      offset += err;
>  
> -    err = v9fs_pack(elem->in_sg, elem->in_num, offset,
> +    v9fs_init_qiov_from_pdu(&qiov_full, pdu, 0, read_count, false);
> +    err = v9fs_pack(qiov_full.iov, qiov_full.niov, offset,
>                      ((char *)fidp->fs.xattr.value) + off,
>                      read_count);
> +    qemu_iovec_destroy(&qiov_full);
>      if (err < 0) {
>          return err;
>      }
> @@ -1732,32 +1759,6 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>      return count;
>  }
>  
> -/*
> - * Create a QEMUIOVector for a sub-region of PDU iovecs
> - *
> - * @qiov:       uninitialized QEMUIOVector
> - * @skip:       number of bytes to skip from beginning of PDU
> - * @size:       number of bytes to include
> - * @is_write:   true - write, false - read
> - *
> - * The resulting QEMUIOVector has heap-allocated iovecs and must be cleaned up
> - * with qemu_iovec_destroy().
> - */
> -static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> -                                    size_t skip, size_t size,
> -                                    bool is_write)
> -{
> -    QEMUIOVector elem;
> -    struct iovec *iov;
> -    unsigned int niov;
> -
> -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> -
> -    qemu_iovec_init_external(&elem, iov, niov);
> -    qemu_iovec_init(qiov, niov);
> -    qemu_iovec_concat(qiov, &elem, skip, size);
> -}
> -
>  static void coroutine_fn v9fs_read(void *opaque)
>  {
>      int32_t fid;


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
  2016-11-28 21:27     ` Stefano Stabellini
@ 2016-12-02 11:05       ` Greg Kurz
  -1 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:05 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard

On Mon, 28 Nov 2016 13:27:24 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> Not all 9pfs transports share memory between request and response. For
> those who don't, it is necessary to know how much memory is required in
> the response.
> 
> Split the existing init_iov_from_pdu function in two:
> init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> init_in_iov_from_pdu takes an additional size parameter to specify the
> memory required for the response message.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/9pfs/9p.c               |  6 +++++-
>  hw/9pfs/9p.h               |  6 ++++--
>  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 79d7201..ce1f9d9 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
>      struct iovec *iov;
>      unsigned int niov;
>  
> -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +    if (is_write) {
> +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> +    } else {
> +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> +    }
>  
>      qemu_iovec_init_external(&elem, iov, niov);
>      qemu_iovec_init(qiov, niov);
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index c8c9aa8..4c4feaf 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -349,8 +349,10 @@ struct V9fsTransport {
>                                  va_list ap);
>      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
>                                    va_list ap);
> -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write);
> +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size);
> +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov);
>      void        (*push_and_notify)(V9fsPDU *pdu);
>  };
>  
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 273425b..27a4a32 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
> -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write)
> +/* The size parameter is used by other transports. Do not drop it. */
> +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>      VirtQueueElement *elem = v->elems[pdu->idx];
>  
> -    if (is_write) {
> -        *piov = elem->out_sg;
> -        *pniov = elem->out_num;
> -    } else {
> -        *piov = elem->in_sg;
> -        *pniov = elem->in_num;
> -    }
> +    *piov = elem->in_sg;
> +    *pniov = elem->in_num;
> +}
> +
> +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov)
> +{
> +    V9fsState *s = pdu->s;
> +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> +    VirtQueueElement *elem = v->elems[pdu->idx];
> +
> +    *piov = elem->out_sg;
> +    *pniov = elem->out_num;
>  }
>  
>  static const struct V9fsTransport virtio_9p_transport = {
>      .pdu_vmarshal = virtio_pdu_vmarshal,
>      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
>      .push_and_notify = virtio_9p_push_and_notify,
>  };
>  

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
@ 2016-12-02 11:05       ` Greg Kurz
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-02 11:05 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Mon, 28 Nov 2016 13:27:24 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

> Not all 9pfs transports share memory between request and response. For
> those who don't, it is necessary to know how much memory is required in
> the response.
> 
> Split the existing init_iov_from_pdu function in two:
> init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> init_in_iov_from_pdu takes an additional size parameter to specify the
> memory required for the response message.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/9pfs/9p.c               |  6 +++++-
>  hw/9pfs/9p.h               |  6 ++++--
>  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 79d7201..ce1f9d9 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
>      struct iovec *iov;
>      unsigned int niov;
>  
> -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +    if (is_write) {
> +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> +    } else {
> +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> +    }
>  
>      qemu_iovec_init_external(&elem, iov, niov);
>      qemu_iovec_init(qiov, niov);
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index c8c9aa8..4c4feaf 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -349,8 +349,10 @@ struct V9fsTransport {
>                                  va_list ap);
>      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
>                                    va_list ap);
> -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write);
> +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size);
> +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov);
>      void        (*push_and_notify)(V9fsPDU *pdu);
>  };
>  
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 273425b..27a4a32 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
> -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write)
> +/* The size parameter is used by other transports. Do not drop it. */
> +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>      VirtQueueElement *elem = v->elems[pdu->idx];
>  
> -    if (is_write) {
> -        *piov = elem->out_sg;
> -        *pniov = elem->out_num;
> -    } else {
> -        *piov = elem->in_sg;
> -        *pniov = elem->in_num;
> -    }
> +    *piov = elem->in_sg;
> +    *pniov = elem->in_num;
> +}
> +
> +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov)
> +{
> +    V9fsState *s = pdu->s;
> +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> +    VirtQueueElement *elem = v->elems[pdu->idx];
> +
> +    *piov = elem->out_sg;
> +    *pniov = elem->out_num;
>  }
>  
>  static const struct V9fsTransport virtio_9p_transport = {
>      .pdu_vmarshal = virtio_pdu_vmarshal,
>      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
>      .push_and_notify = virtio_9p_push_and_notify,
>  };
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
  2016-12-02 11:05       ` Greg Kurz
@ 2016-12-02 19:02         ` Stefano Stabellini
  -1 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-12-02 19:02 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Stefano Stabellini, anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Fri, 2 Dec 2016, Greg Kurz wrote:
> On Mon, 28 Nov 2016 13:27:24 -0800
> Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> > Not all 9pfs transports share memory between request and response. For
> > those who don't, it is necessary to know how much memory is required in
> > the response.
> > 
> > Split the existing init_iov_from_pdu function in two:
> > init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> > init_in_iov_from_pdu takes an additional size parameter to specify the
> > memory required for the response message.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks Greg! I assume you are going to apply them to your tree, right?

Thanks again,

Stefano


> >  hw/9pfs/9p.c               |  6 +++++-
> >  hw/9pfs/9p.h               |  6 ++++--
> >  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
> >  3 files changed, 27 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 79d7201..ce1f9d9 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> >      struct iovec *iov;
> >      unsigned int niov;
> >  
> > -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> > +    if (is_write) {
> > +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> > +    } else {
> > +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> > +    }
> >  
> >      qemu_iovec_init_external(&elem, iov, niov);
> >      qemu_iovec_init(qiov, niov);
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index c8c9aa8..4c4feaf 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -349,8 +349,10 @@ struct V9fsTransport {
> >                                  va_list ap);
> >      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> >                                    va_list ap);
> > -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > -                                     unsigned int *pniov, bool is_write);
> > +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > +                                        unsigned int *pniov, size_t size);
> > +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > +                                         unsigned int *pniov);
> >      void        (*push_and_notify)(V9fsPDU *pdu);
> >  };
> >  
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 273425b..27a4a32 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> >      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
> >  }
> >  
> > -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > -                                     unsigned int *pniov, bool is_write)
> > +/* The size parameter is used by other transports. Do not drop it. */
> > +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > +                                        unsigned int *pniov, size_t size)
> >  {
> >      V9fsState *s = pdu->s;
> >      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> >      VirtQueueElement *elem = v->elems[pdu->idx];
> >  
> > -    if (is_write) {
> > -        *piov = elem->out_sg;
> > -        *pniov = elem->out_num;
> > -    } else {
> > -        *piov = elem->in_sg;
> > -        *pniov = elem->in_num;
> > -    }
> > +    *piov = elem->in_sg;
> > +    *pniov = elem->in_num;
> > +}
> > +
> > +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > +                                         unsigned int *pniov)
> > +{
> > +    V9fsState *s = pdu->s;
> > +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > +    VirtQueueElement *elem = v->elems[pdu->idx];
> > +
> > +    *piov = elem->out_sg;
> > +    *pniov = elem->out_num;
> >  }
> >  
> >  static const struct V9fsTransport virtio_9p_transport = {
> >      .pdu_vmarshal = virtio_pdu_vmarshal,
> >      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> > -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> > +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> > +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
> >      .push_and_notify = virtio_9p_push_and_notify,
> >  };
> >  
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Xen-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
@ 2016-12-02 19:02         ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2016-12-02 19:02 UTC (permalink / raw)
  To: Greg Kurz
  Cc: anthony.perard, qemu-devel, Stefano Stabellini, xen-devel, aneesh.kumar

On Fri, 2 Dec 2016, Greg Kurz wrote:
> On Mon, 28 Nov 2016 13:27:24 -0800
> Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> > Not all 9pfs transports share memory between request and response. For
> > those who don't, it is necessary to know how much memory is required in
> > the response.
> > 
> > Split the existing init_iov_from_pdu function in two:
> > init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> > init_in_iov_from_pdu takes an additional size parameter to specify the
> > memory required for the response message.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks Greg! I assume you are going to apply them to your tree, right?

Thanks again,

Stefano


> >  hw/9pfs/9p.c               |  6 +++++-
> >  hw/9pfs/9p.h               |  6 ++++--
> >  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
> >  3 files changed, 27 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 79d7201..ce1f9d9 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> >      struct iovec *iov;
> >      unsigned int niov;
> >  
> > -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> > +    if (is_write) {
> > +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> > +    } else {
> > +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> > +    }
> >  
> >      qemu_iovec_init_external(&elem, iov, niov);
> >      qemu_iovec_init(qiov, niov);
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index c8c9aa8..4c4feaf 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -349,8 +349,10 @@ struct V9fsTransport {
> >                                  va_list ap);
> >      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> >                                    va_list ap);
> > -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > -                                     unsigned int *pniov, bool is_write);
> > +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > +                                        unsigned int *pniov, size_t size);
> > +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > +                                         unsigned int *pniov);
> >      void        (*push_and_notify)(V9fsPDU *pdu);
> >  };
> >  
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 273425b..27a4a32 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> >      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
> >  }
> >  
> > -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > -                                     unsigned int *pniov, bool is_write)
> > +/* The size parameter is used by other transports. Do not drop it. */
> > +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > +                                        unsigned int *pniov, size_t size)
> >  {
> >      V9fsState *s = pdu->s;
> >      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> >      VirtQueueElement *elem = v->elems[pdu->idx];
> >  
> > -    if (is_write) {
> > -        *piov = elem->out_sg;
> > -        *pniov = elem->out_num;
> > -    } else {
> > -        *piov = elem->in_sg;
> > -        *pniov = elem->in_num;
> > -    }
> > +    *piov = elem->in_sg;
> > +    *pniov = elem->in_num;
> > +}
> > +
> > +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > +                                         unsigned int *pniov)
> > +{
> > +    V9fsState *s = pdu->s;
> > +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > +    VirtQueueElement *elem = v->elems[pdu->idx];
> > +
> > +    *piov = elem->out_sg;
> > +    *pniov = elem->out_num;
> >  }
> >  
> >  static const struct V9fsTransport virtio_9p_transport = {
> >      .pdu_vmarshal = virtio_pdu_vmarshal,
> >      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> > -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> > +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> > +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
> >      .push_and_notify = virtio_9p_push_and_notify,
> >  };
> >  
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
  2016-12-02 19:02         ` Stefano Stabellini
@ 2016-12-03  8:57           ` Greg Kurz
  -1 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-03  8:57 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Fri, 2 Dec 2016 11:02:44 -0800 (PST)
Stefano Stabellini <sstabellini@kernel.org> wrote:

> On Fri, 2 Dec 2016, Greg Kurz wrote:
> > On Mon, 28 Nov 2016 13:27:24 -0800
> > Stefano Stabellini <sstabellini@kernel.org> wrote:
> >   
> > > Not all 9pfs transports share memory between request and response. For
> > > those who don't, it is necessary to know how much memory is required in
> > > the response.
> > > 
> > > Split the existing init_iov_from_pdu function in two:
> > > init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> > > init_in_iov_from_pdu takes an additional size parameter to specify the
> > > memory required for the response message.
> > > 
> > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > ---  
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>  
> 
> Thanks Greg! I assume you are going to apply them to your tree, right?
> 

Done.

https://github.com/gkurz/qemu/commits/9p-next

Cheers.

--
Greg

> Thanks again,
> 
> Stefano
> 
> 
> > >  hw/9pfs/9p.c               |  6 +++++-
> > >  hw/9pfs/9p.h               |  6 ++++--
> > >  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
> > >  3 files changed, 27 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 79d7201..ce1f9d9 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> > >      struct iovec *iov;
> > >      unsigned int niov;
> > >  
> > > -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> > > +    if (is_write) {
> > > +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> > > +    } else {
> > > +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> > > +    }
> > >  
> > >      qemu_iovec_init_external(&elem, iov, niov);
> > >      qemu_iovec_init(qiov, niov);
> > > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > > index c8c9aa8..4c4feaf 100644
> > > --- a/hw/9pfs/9p.h
> > > +++ b/hw/9pfs/9p.h
> > > @@ -349,8 +349,10 @@ struct V9fsTransport {
> > >                                  va_list ap);
> > >      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> > >                                    va_list ap);
> > > -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > -                                     unsigned int *pniov, bool is_write);
> > > +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > +                                        unsigned int *pniov, size_t size);
> > > +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > +                                         unsigned int *pniov);
> > >      void        (*push_and_notify)(V9fsPDU *pdu);
> > >  };
> > >  
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 273425b..27a4a32 100644
> > > --- a/hw/9pfs/virtio-9p-device.c
> > > +++ b/hw/9pfs/virtio-9p-device.c
> > > @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > >      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
> > >  }
> > >  
> > > -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > -                                     unsigned int *pniov, bool is_write)
> > > +/* The size parameter is used by other transports. Do not drop it. */
> > > +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > +                                        unsigned int *pniov, size_t size)
> > >  {
> > >      V9fsState *s = pdu->s;
> > >      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > >      VirtQueueElement *elem = v->elems[pdu->idx];
> > >  
> > > -    if (is_write) {
> > > -        *piov = elem->out_sg;
> > > -        *pniov = elem->out_num;
> > > -    } else {
> > > -        *piov = elem->in_sg;
> > > -        *pniov = elem->in_num;
> > > -    }
> > > +    *piov = elem->in_sg;
> > > +    *pniov = elem->in_num;
> > > +}
> > > +
> > > +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > +                                         unsigned int *pniov)
> > > +{
> > > +    V9fsState *s = pdu->s;
> > > +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > > +    VirtQueueElement *elem = v->elems[pdu->idx];
> > > +
> > > +    *piov = elem->out_sg;
> > > +    *pniov = elem->out_num;
> > >  }
> > >  
> > >  static const struct V9fsTransport virtio_9p_transport = {
> > >      .pdu_vmarshal = virtio_pdu_vmarshal,
> > >      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> > > -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> > > +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> > > +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
> > >      .push_and_notify = virtio_9p_push_and_notify,
> > >  };
> > >    
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel
> >   

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Xen-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu
@ 2016-12-03  8:57           ` Greg Kurz
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kurz @ 2016-12-03  8:57 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: anthony.perard, xen-devel, aneesh.kumar, qemu-devel

On Fri, 2 Dec 2016 11:02:44 -0800 (PST)
Stefano Stabellini <sstabellini@kernel.org> wrote:

> On Fri, 2 Dec 2016, Greg Kurz wrote:
> > On Mon, 28 Nov 2016 13:27:24 -0800
> > Stefano Stabellini <sstabellini@kernel.org> wrote:
> >   
> > > Not all 9pfs transports share memory between request and response. For
> > > those who don't, it is necessary to know how much memory is required in
> > > the response.
> > > 
> > > Split the existing init_iov_from_pdu function in two:
> > > init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> > > init_in_iov_from_pdu takes an additional size parameter to specify the
> > > memory required for the response message.
> > > 
> > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > ---  
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>  
> 
> Thanks Greg! I assume you are going to apply them to your tree, right?
> 

Done.

https://github.com/gkurz/qemu/commits/9p-next

Cheers.

--
Greg

> Thanks again,
> 
> Stefano
> 
> 
> > >  hw/9pfs/9p.c               |  6 +++++-
> > >  hw/9pfs/9p.h               |  6 ++++--
> > >  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
> > >  3 files changed, 27 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 79d7201..ce1f9d9 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
> > >      struct iovec *iov;
> > >      unsigned int niov;
> > >  
> > > -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> > > +    if (is_write) {
> > > +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> > > +    } else {
> > > +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> > > +    }
> > >  
> > >      qemu_iovec_init_external(&elem, iov, niov);
> > >      qemu_iovec_init(qiov, niov);
> > > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > > index c8c9aa8..4c4feaf 100644
> > > --- a/hw/9pfs/9p.h
> > > +++ b/hw/9pfs/9p.h
> > > @@ -349,8 +349,10 @@ struct V9fsTransport {
> > >                                  va_list ap);
> > >      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> > >                                    va_list ap);
> > > -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > -                                     unsigned int *pniov, bool is_write);
> > > +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > +                                        unsigned int *pniov, size_t size);
> > > +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > > +                                         unsigned int *pniov);
> > >      void        (*push_and_notify)(V9fsPDU *pdu);
> > >  };
> > >  
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 273425b..27a4a32 100644
> > > --- a/hw/9pfs/virtio-9p-device.c
> > > +++ b/hw/9pfs/virtio-9p-device.c
> > > @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > >      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
> > >  }
> > >  
> > > -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > -                                     unsigned int *pniov, bool is_write)
> > > +/* The size parameter is used by other transports. Do not drop it. */
> > > +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > +                                        unsigned int *pniov, size_t size)
> > >  {
> > >      V9fsState *s = pdu->s;
> > >      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > >      VirtQueueElement *elem = v->elems[pdu->idx];
> > >  
> > > -    if (is_write) {
> > > -        *piov = elem->out_sg;
> > > -        *pniov = elem->out_num;
> > > -    } else {
> > > -        *piov = elem->in_sg;
> > > -        *pniov = elem->in_num;
> > > -    }
> > > +    *piov = elem->in_sg;
> > > +    *pniov = elem->in_num;
> > > +}
> > > +
> > > +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > +                                         unsigned int *pniov)
> > > +{
> > > +    V9fsState *s = pdu->s;
> > > +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> > > +    VirtQueueElement *elem = v->elems[pdu->idx];
> > > +
> > > +    *piov = elem->out_sg;
> > > +    *pniov = elem->out_num;
> > >  }
> > >  
> > >  static const struct V9fsTransport virtio_9p_transport = {
> > >      .pdu_vmarshal = virtio_pdu_vmarshal,
> > >      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> > > -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> > > +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> > > +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
> > >      .push_and_notify = virtio_9p_push_and_notify,
> > >  };
> > >    
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel
> >   

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-12-03  8:57 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-28 21:24 [Qemu-devel] [PATCH v2 0/4] 9pfs: clean-up for multiple transports Stefano Stabellini
2016-11-28 21:24 ` Stefano Stabellini
2016-11-28 21:27 ` [Qemu-devel] [PATCH v2 1/4] 9pfs: move pdus to V9fsState Stefano Stabellini
2016-11-28 21:27   ` Stefano Stabellini
2016-11-28 21:27   ` [Qemu-devel] [PATCH v2 2/4] 9pfs: introduce transport specific callbacks Stefano Stabellini
2016-11-28 21:27     ` Stefano Stabellini
2016-12-02 11:03     ` [Qemu-devel] " Greg Kurz
2016-12-02 11:03       ` Greg Kurz
2016-11-28 21:27   ` [Qemu-devel] [PATCH v2 3/4] 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack Stefano Stabellini
2016-11-28 21:27     ` Stefano Stabellini
2016-12-02 11:04     ` [Qemu-devel] " Greg Kurz
2016-12-02 11:04       ` Greg Kurz
2016-11-28 21:27   ` [Qemu-devel] [PATCH v2 4/4] 9pfs: introduce init_out/in_iov_from_pdu Stefano Stabellini
2016-11-28 21:27     ` Stefano Stabellini
2016-12-02 11:05     ` [Qemu-devel] " Greg Kurz
2016-12-02 11:05       ` Greg Kurz
2016-12-02 19:02       ` [Qemu-devel] [Xen-devel] " Stefano Stabellini
2016-12-02 19:02         ` Stefano Stabellini
2016-12-03  8:57         ` [Qemu-devel] " Greg Kurz
2016-12-03  8:57           ` Greg Kurz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.