* [PATCH 1/4] 9pfs: move pdus to V9fsState
@ 2016-11-21 21:39 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: xen-devel, wei.liu2, groug, qemu-devel, sstabellini,
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>
---
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-21 21:39 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard,
wei.liu2
Don't call virtio functions from 9pfs generic code, use generic function
callbacks instead.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 8 ++++----
hw/9pfs/9p.h | 18 ++++++++++++++++++
hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
hw/9pfs/virtio-9p.h | 9 ---------
4 files changed, 36 insertions(+), 17 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..ab398d0 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];
+ struct V9fsTransport *transport;
/*
* lock ensuring atomic path update
* on rename.
@@ -343,4 +344,21 @@ 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, struct V9fsTransport *t)
+{
+ if (s->transport) {
+ return -EINVAL;
+ }
+ s->transport = t;
+ return 0;
+}
+
#endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1782e4a..e1a37a4 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 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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
v9fs_reset(&v->state);
}
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
+static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap)
{
V9fsState *s = pdu->s;
@@ -158,7 +161,7 @@ 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,
+static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap)
{
V9fsState *s = pdu->s;
@@ -168,7 +171,7 @@ 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,
+static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
unsigned int *pniov, bool is_write)
{
V9fsState *s = pdu->s;
@@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
}
}
+static 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] 30+ messages in thread* [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-21 21:39 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: xen-devel, wei.liu2, groug, qemu-devel, sstabellini,
anthony.perard
Don't call virtio functions from 9pfs generic code, use generic function
callbacks instead.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 8 ++++----
hw/9pfs/9p.h | 18 ++++++++++++++++++
hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
hw/9pfs/virtio-9p.h | 9 ---------
4 files changed, 36 insertions(+), 17 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..ab398d0 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];
+ struct V9fsTransport *transport;
/*
* lock ensuring atomic path update
* on rename.
@@ -343,4 +344,21 @@ 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, struct V9fsTransport *t)
+{
+ if (s->transport) {
+ return -EINVAL;
+ }
+ s->transport = t;
+ return 0;
+}
+
#endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1782e4a..e1a37a4 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 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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
v9fs_reset(&v->state);
}
-ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
+static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap)
{
V9fsState *s = pdu->s;
@@ -158,7 +161,7 @@ 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,
+static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap)
{
V9fsState *s = pdu->s;
@@ -168,7 +171,7 @@ 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,
+static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
unsigned int *pniov, bool is_write)
{
V9fsState *s = pdu->s;
@@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
}
}
+static 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] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-24 8:31 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 8:31 UTC (permalink / raw)
To: Stefano Stabellini
Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard, wei.liu2
On Mon, 21 Nov 2016 13:39:30 -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>
> ---
Just a couple of indentation and line over 80 characters nits. I'll fix them
before pushing to 9p-next.
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/9pfs/9p.c | 8 ++++----
> hw/9pfs/9p.h | 18 ++++++++++++++++++
> hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
> hw/9pfs/virtio-9p.h | 9 ---------
> 4 files changed, 36 insertions(+), 17 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..ab398d0 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];
> + struct V9fsTransport *transport;
> /*
> * lock ensuring atomic path update
> * on rename.
> @@ -343,4 +344,21 @@ 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);
over 80 characters
> + ssize_t (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap);
ditto
> + void (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> + unsigned int *pniov, bool is_write);
indent
> + void (*push_and_notify)(V9fsPDU *pdu);
> +};
> +
> +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> +{
> + if (s->transport) {
> + return -EINVAL;
> + }
> + s->transport = t;
> + return 0;
> +}
> +
> #endif
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 1782e4a..e1a37a4 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 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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> v9fs_reset(&v->state);
> }
>
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> const char *fmt, va_list ap)
indent
> {
> V9fsState *s = pdu->s;
> @@ -158,7 +161,7 @@ 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,
> +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> const char *fmt, va_list ap)
indent
> {
> V9fsState *s = pdu->s;
> @@ -168,7 +171,7 @@ 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,
> +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> unsigned int *pniov, bool is_write)
indent
> {
> V9fsState *s = pdu->s;
> @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> }
> }
>
> +static 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] 30+ messages in thread* Re: [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-24 8:31 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 8:31 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Mon, 21 Nov 2016 13:39:30 -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>
> ---
Just a couple of indentation and line over 80 characters nits. I'll fix them
before pushing to 9p-next.
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/9pfs/9p.c | 8 ++++----
> hw/9pfs/9p.h | 18 ++++++++++++++++++
> hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
> hw/9pfs/virtio-9p.h | 9 ---------
> 4 files changed, 36 insertions(+), 17 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..ab398d0 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];
> + struct V9fsTransport *transport;
> /*
> * lock ensuring atomic path update
> * on rename.
> @@ -343,4 +344,21 @@ 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);
over 80 characters
> + ssize_t (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap);
ditto
> + void (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> + unsigned int *pniov, bool is_write);
indent
> + void (*push_and_notify)(V9fsPDU *pdu);
> +};
> +
> +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> +{
> + if (s->transport) {
> + return -EINVAL;
> + }
> + s->transport = t;
> + return 0;
> +}
> +
> #endif
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 1782e4a..e1a37a4 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 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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> v9fs_reset(&v->state);
> }
>
> -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> const char *fmt, va_list ap)
indent
> {
> V9fsState *s = pdu->s;
> @@ -158,7 +161,7 @@ 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,
> +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> const char *fmt, va_list ap)
indent
> {
> V9fsState *s = pdu->s;
> @@ -168,7 +171,7 @@ 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,
> +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> unsigned int *pniov, bool is_write)
indent
> {
> V9fsState *s = pdu->s;
> @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> }
> }
>
> +static 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)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-24 8:31 ` Greg Kurz
@ 2016-11-24 14:23 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:23 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016 09:31:52 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Mon, 21 Nov 2016 13:39:30 -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>
> > ---
>
> Just a couple of indentation and line over 80 characters nits. I'll fix them
> before pushing to 9p-next.
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
>
Hmm... second thought...
> > hw/9pfs/9p.c | 8 ++++----
> > hw/9pfs/9p.h | 18 ++++++++++++++++++
> > hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
> > hw/9pfs/virtio-9p.h | 9 ---------
> > 4 files changed, 36 insertions(+), 17 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..ab398d0 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];
> > + struct V9fsTransport *transport;
> > /*
> > * lock ensuring atomic path update
> > * on rename.
> > @@ -343,4 +344,21 @@ 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);
>
> over 80 characters
>
> > + ssize_t (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap);
>
> ditto
>
> > + void (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > + unsigned int *pniov, bool is_write);
>
> indent
>
> > + void (*push_and_notify)(V9fsPDU *pdu);
> > +};
> > +
> > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > +{
> > + if (s->transport) {
> > + return -EINVAL;
> > + }
> > + s->transport = t;
> > + return 0;
> > +}
> > +
> > #endif
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
... shouldn't this be const ?
> > +
> > +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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> > v9fs_reset(&v->state);
> > }
> >
> > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > const char *fmt, va_list ap)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -158,7 +161,7 @@ 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,
> > +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > const char *fmt, va_list ap)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -168,7 +171,7 @@ 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,
> > +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > unsigned int *pniov, bool is_write)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > }
> > }
> >
> > +static 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] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-24 14:23 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:23 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016 09:31:52 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Mon, 21 Nov 2016 13:39:30 -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>
> > ---
>
> Just a couple of indentation and line over 80 characters nits. I'll fix them
> before pushing to 9p-next.
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
>
Hmm... second thought...
> > hw/9pfs/9p.c | 8 ++++----
> > hw/9pfs/9p.h | 18 ++++++++++++++++++
> > hw/9pfs/virtio-9p-device.c | 18 ++++++++++++++----
> > hw/9pfs/virtio-9p.h | 9 ---------
> > 4 files changed, 36 insertions(+), 17 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..ab398d0 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];
> > + struct V9fsTransport *transport;
> > /*
> > * lock ensuring atomic path update
> > * on rename.
> > @@ -343,4 +344,21 @@ 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);
>
> over 80 characters
>
> > + ssize_t (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap);
>
> ditto
>
> > + void (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > + unsigned int *pniov, bool is_write);
>
> indent
>
> > + void (*push_and_notify)(V9fsPDU *pdu);
> > +};
> > +
> > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > +{
> > + if (s->transport) {
> > + return -EINVAL;
> > + }
> > + s->transport = t;
> > + return 0;
> > +}
> > +
> > #endif
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
... shouldn't this be const ?
> > +
> > +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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> > v9fs_reset(&v->state);
> > }
> >
> > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > const char *fmt, va_list ap)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -158,7 +161,7 @@ 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,
> > +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > const char *fmt, va_list ap)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -168,7 +171,7 @@ 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,
> > +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > unsigned int *pniov, bool is_write)
>
> indent
>
> > {
> > V9fsState *s = pdu->s;
> > @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > }
> > }
> >
> > +static 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)
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-24 14:23 ` Greg Kurz
@ 2016-11-24 14:43 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:43 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016 15:23:10 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Thu, 24 Nov 2016 09:31:52 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Mon, 21 Nov 2016 13:39:30 -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>
> > > ---
> >
> > Just a couple of indentation and line over 80 characters nits. I'll fix them
> > before pushing to 9p-next.
> >
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> >
>
> Hmm... second thought...
>
[...]
> > > +
> > > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > > +{
> > > + if (s->transport) {
> > > + return -EINVAL;
> > > + }
Calling v9fs_register_transport() several times for the same V9fsState looks
more like a bug than an error... also, is it possible to have several 9pfs
shares with different transports ?
> > > + s->transport = t;
> > > + return 0;
> > > +}
> > > +
> > > #endif
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
>
> ... shouldn't this be const ?
>
> > > +
> > > +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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> > > v9fs_reset(&v->state);
> > > }
> > >
> > > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > > +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > > const char *fmt, va_list ap)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -158,7 +161,7 @@ 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,
> > > +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > > const char *fmt, va_list ap)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -168,7 +171,7 @@ 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,
> > > +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > unsigned int *pniov, bool is_write)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > }
> > > }
> > >
> > > +static 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] 30+ messages in thread* Re: [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-24 14:43 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:43 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016 15:23:10 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Thu, 24 Nov 2016 09:31:52 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Mon, 21 Nov 2016 13:39:30 -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>
> > > ---
> >
> > Just a couple of indentation and line over 80 characters nits. I'll fix them
> > before pushing to 9p-next.
> >
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> >
>
> Hmm... second thought...
>
[...]
> > > +
> > > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > > +{
> > > + if (s->transport) {
> > > + return -EINVAL;
> > > + }
Calling v9fs_register_transport() several times for the same V9fsState looks
more like a bug than an error... also, is it possible to have several 9pfs
shares with different transports ?
> > > + s->transport = t;
> > > + return 0;
> > > +}
> > > +
> > > #endif
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
>
> ... shouldn't this be const ?
>
> > > +
> > > +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,7 +151,7 @@ static void virtio_9p_reset(VirtIODevice *vdev)
> > > v9fs_reset(&v->state);
> > > }
> > >
> > > -ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > > +static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
> > > const char *fmt, va_list ap)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -158,7 +161,7 @@ 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,
> > > +static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > > const char *fmt, va_list ap)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -168,7 +171,7 @@ 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,
> > > +static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > unsigned int *pniov, bool is_write)
> >
> > indent
> >
> > > {
> > > V9fsState *s = pdu->s;
> > > @@ -184,6 +187,13 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > > }
> > > }
> > >
> > > +static 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] 30+ messages in thread* Re: [Qemu-devel] [Xen-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-24 14:43 ` Greg Kurz
@ 2016-11-28 20:12 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:12 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, anthony.perard, xen-devel, wei.liu2,
aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Thu, 24 Nov 2016 15:23:10 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Thu, 24 Nov 2016 09:31:52 +0100
> > Greg Kurz <groug@kaod.org> wrote:
> >
> > > On Mon, 21 Nov 2016 13:39:30 -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>
> > > > ---
> > >
> > > Just a couple of indentation and line over 80 characters nits. I'll fix them
> > > before pushing to 9p-next.
> > >
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > >
> >
> > Hmm... second thought...
> >
>
> [...]
>
> > > > +
> > > > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > > > +{
> > > > + if (s->transport) {
> > > > + return -EINVAL;
> > > > + }
>
> Calling v9fs_register_transport() several times for the same V9fsState looks
> more like a bug than an error...
Yes, you are right. I can turn this into an assert.
> also, is it possible to have several 9pfs
> shares with different transports ?
I think, at least theoretically, is possible. For example Xen HVM
guests, the ones most similar to KVM guests, already support virtio as a
transport. Additionally they will be able to support the Xen based
transport too. (Xen PV guests will only support
the Xen based transport as they don't support virtio in general.)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-28 20:12 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:12 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, wei.liu2, qemu-devel, xen-devel, aneesh.kumar,
anthony.perard
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Thu, 24 Nov 2016 15:23:10 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Thu, 24 Nov 2016 09:31:52 +0100
> > Greg Kurz <groug@kaod.org> wrote:
> >
> > > On Mon, 21 Nov 2016 13:39:30 -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>
> > > > ---
> > >
> > > Just a couple of indentation and line over 80 characters nits. I'll fix them
> > > before pushing to 9p-next.
> > >
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > >
> >
> > Hmm... second thought...
> >
>
> [...]
>
> > > > +
> > > > +static inline int v9fs_register_transport(V9fsState *s, struct V9fsTransport *t)
> > > > +{
> > > > + if (s->transport) {
> > > > + return -EINVAL;
> > > > + }
>
> Calling v9fs_register_transport() several times for the same V9fsState looks
> more like a bug than an error...
Yes, you are right. I can turn this into an assert.
> also, is it possible to have several 9pfs
> shares with different transports ?
I think, at least theoretically, is possible. For example Xen HVM
guests, the ones most similar to KVM guests, already support virtio as a
transport. Additionally they will be able to support the Xen based
transport too. (Xen PV guests will only support
the Xen based transport as they don't support virtio in general.)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
2016-11-24 14:23 ` Greg Kurz
@ 2016-11-28 20:11 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:11 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, anthony.perard, xen-devel, wei.liu2,
aneesh.kumar, qemu-devel
On Thu, 24 Nov 2016, Greg Kurz wrote:
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
>
> ... shouldn't this be const ?
Yes, makes sense.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] 9pfs: introduce transport specific callbacks
@ 2016-11-28 20:11 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:11 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, wei.liu2, qemu-devel, xen-devel, aneesh.kumar,
anthony.perard
On Thu, 24 Nov 2016, Greg Kurz wrote:
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 1782e4a..e1a37a4 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 struct V9fsTransport virtio_9p_transport;
>
> ... shouldn't this be const ?
Yes, makes sense.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-21 21:39 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard,
wei.liu2
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 instead of v9fs_pack.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
if (err < 0) {
@@ -1732,32 +1758,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] 30+ messages in thread* [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
@ 2016-11-21 21:39 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: xen-devel, wei.liu2, groug, qemu-devel, sstabellini,
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 instead of v9fs_pack.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
if (err < 0) {
@@ -1732,32 +1758,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] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-24 14:48 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:48 UTC (permalink / raw)
To: Stefano Stabellini
Cc: aneesh.kumar, xen-devel, wei.liu2, qemu-devel, anthony.perard
On Mon, 21 Nov 2016 13:39:31 -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 instead of v9fs_pack.
>
instead of ? I see v9fs_init_qiov_from_pdu() gets called before calling v9fs_pack().
Also, I don't see the corresponding call to qemu_iovec_destroy() to free the
allocated iovec.
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
> if (err < 0) {
> @@ -1732,32 +1758,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] 30+ messages in thread* Re: [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
@ 2016-11-24 14:48 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 14:48 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Mon, 21 Nov 2016 13:39:31 -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 instead of v9fs_pack.
>
instead of ? I see v9fs_init_qiov_from_pdu() gets called before calling v9fs_pack().
Also, I don't see the corresponding call to qemu_iovec_destroy() to free the
allocated iovec.
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
> if (err < 0) {
> @@ -1732,32 +1758,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] 30+ messages in thread* Re: [Qemu-devel] [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
2016-11-24 14:48 ` Greg Kurz
@ 2016-11-28 20:35 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:35 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, aneesh.kumar, xen-devel, wei.liu2, qemu-devel,
anthony.perard
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Mon, 21 Nov 2016 13:39:31 -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 instead of v9fs_pack.
> >
>
> instead of ? I see v9fs_init_qiov_from_pdu() gets called before calling v9fs_pack().
Sorry wrong description. I'll fix it.
> Also, I don't see the corresponding call to qemu_iovec_destroy() to free the
> allocated iovec.
Good point! I'll add a call.
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> > hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
> > 1 file changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
> > if (err < 0) {
> > @@ -1732,32 +1758,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] 30+ messages in thread* Re: [PATCH 3/4] 9pfs: use v9fs_init_qiov_from_pdu instead of v9fs_pack
@ 2016-11-28 20:35 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 20:35 UTC (permalink / raw)
To: Greg Kurz
Cc: xen-devel, wei.liu2, qemu-devel, Stefano Stabellini, aneesh.kumar,
anthony.perard
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Mon, 21 Nov 2016 13:39:31 -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 instead of v9fs_pack.
> >
>
> instead of ? I see v9fs_init_qiov_from_pdu() gets called before calling v9fs_pack().
Sorry wrong description. I'll fix it.
> Also, I don't see the corresponding call to qemu_iovec_destroy() to free the
> allocated iovec.
Good point! I'll add a call.
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> > hw/9pfs/9p.c | 58 +++++++++++++++++++++++++++++-----------------------------
> > 1 file changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 5a20a13..b6ec042 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,7 +1681,8 @@ 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);
> > if (err < 0) {
> > @@ -1732,32 +1758,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] 30+ messages in thread
* [Qemu-devel] [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-21 21:39 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: groug, sstabellini, qemu-devel, xen-devel, anthony.perard,
wei.liu2
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.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 2 +-
hw/9pfs/9p.h | 2 +-
hw/9pfs/virtio-9p-device.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index b6ec042..b82212b 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1652,7 +1652,7 @@ 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);
+ pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -348,7 +348,7 @@ 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);
+ unsigned int *pniov, bool is_write, size_t size);
void (*push_and_notify)(V9fsPDU *pdu);
};
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index e1a37a4..e2b27e8 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
}
static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
- unsigned int *pniov, bool is_write)
+ unsigned int *pniov, bool is_write, size_t size)
{
V9fsState *s = pdu->s;
V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
@ 2016-11-21 21:39 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-21 21:39 UTC (permalink / raw)
To: aneesh.kumar
Cc: xen-devel, wei.liu2, groug, qemu-devel, sstabellini,
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.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
hw/9pfs/9p.c | 2 +-
hw/9pfs/9p.h | 2 +-
hw/9pfs/virtio-9p-device.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index b6ec042..b82212b 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1652,7 +1652,7 @@ 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);
+ pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -348,7 +348,7 @@ 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);
+ unsigned int *pniov, bool is_write, size_t size);
void (*push_and_notify)(V9fsPDU *pdu);
};
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index e1a37a4..e2b27e8 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
}
static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
- unsigned int *pniov, bool is_write)
+ unsigned int *pniov, bool is_write, size_t size)
{
V9fsState *s = pdu->s;
V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-24 16:17 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 16:17 UTC (permalink / raw)
To: Stefano Stabellini
Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard, wei.liu2
On Mon, 21 Nov 2016 13:39:32 -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.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
IIUC the transport used in Xen requires you pass the size when sending a
P9_RREAD message back to the guest, i.e. only in the case when is_write is
false, correct ?
If so, for better clarity, what about having two distinct init_in_iov_from_pdu
and init_out_iov_from_pdu ops, with an explicit comment in the virtio-9p
implementation of init_in_iov_from_pdu so that someone isn't tempted to drop
the apparently unused size argument ? Would this be ok for you on the Xen
side ?
Cheers.
--
Greg
> hw/9pfs/9p.c | 2 +-
> hw/9pfs/9p.h | 2 +-
> hw/9pfs/virtio-9p-device.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index b6ec042..b82212b 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1652,7 +1652,7 @@ 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);
> + pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -348,7 +348,7 @@ 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);
> + unsigned int *pniov, bool is_write, size_t size);
> void (*push_and_notify)(V9fsPDU *pdu);
> };
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index e1a37a4..e2b27e8 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> }
>
> static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> - unsigned int *pniov, bool is_write)
> + unsigned int *pniov, bool is_write, size_t size)
> {
> V9fsState *s = pdu->s;
> V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
@ 2016-11-24 16:17 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 16:17 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Mon, 21 Nov 2016 13:39:32 -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.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
IIUC the transport used in Xen requires you pass the size when sending a
P9_RREAD message back to the guest, i.e. only in the case when is_write is
false, correct ?
If so, for better clarity, what about having two distinct init_in_iov_from_pdu
and init_out_iov_from_pdu ops, with an explicit comment in the virtio-9p
implementation of init_in_iov_from_pdu so that someone isn't tempted to drop
the apparently unused size argument ? Would this be ok for you on the Xen
side ?
Cheers.
--
Greg
> hw/9pfs/9p.c | 2 +-
> hw/9pfs/9p.h | 2 +-
> hw/9pfs/virtio-9p-device.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index b6ec042..b82212b 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1652,7 +1652,7 @@ 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);
> + pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -348,7 +348,7 @@ 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);
> + unsigned int *pniov, bool is_write, size_t size);
> void (*push_and_notify)(V9fsPDU *pdu);
> };
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index e1a37a4..e2b27e8 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> }
>
> static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> - unsigned int *pniov, bool is_write)
> + unsigned int *pniov, bool is_write, size_t size)
> {
> V9fsState *s = pdu->s;
> V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
2016-11-24 16:17 ` Greg Kurz
@ 2016-11-28 21:21 ` Stefano Stabellini
-1 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:21 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefano Stabellini, aneesh.kumar, qemu-devel, xen-devel,
anthony.perard, wei.liu2
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Mon, 21 Nov 2016 13:39:32 -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.
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
>
> IIUC the transport used in Xen requires you pass the size when sending a
> P9_RREAD message back to the guest, i.e. only in the case when is_write is
> false, correct ?
That's right
> If so, for better clarity, what about having two distinct init_in_iov_from_pdu
> and init_out_iov_from_pdu ops, with an explicit comment in the virtio-9p
> implementation of init_in_iov_from_pdu so that someone isn't tempted to drop
> the apparently unused size argument ? Would this be ok for you on the Xen
> side ?
Sure, that's fine by me. I'll do that.
> Cheers.
>
> --
> Greg
>
> > hw/9pfs/9p.c | 2 +-
> > hw/9pfs/9p.h | 2 +-
> > hw/9pfs/virtio-9p-device.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index b6ec042..b82212b 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1652,7 +1652,7 @@ 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);
> > + pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -348,7 +348,7 @@ 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);
> > + unsigned int *pniov, bool is_write, size_t size);
> > void (*push_and_notify)(V9fsPDU *pdu);
> > };
> >
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index e1a37a4..e2b27e8 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > }
> >
> > static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > - unsigned int *pniov, bool is_write)
> > + unsigned int *pniov, bool is_write, size_t size)
> > {
> > V9fsState *s = pdu->s;
> > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 4/4] 9pfs: add a size parameter to init_iov_from_pdu
@ 2016-11-28 21:21 ` Stefano Stabellini
0 siblings, 0 replies; 30+ messages in thread
From: Stefano Stabellini @ 2016-11-28 21:21 UTC (permalink / raw)
To: Greg Kurz
Cc: xen-devel, wei.liu2, qemu-devel, Stefano Stabellini, aneesh.kumar,
anthony.perard
On Thu, 24 Nov 2016, Greg Kurz wrote:
> On Mon, 21 Nov 2016 13:39:32 -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.
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
>
> IIUC the transport used in Xen requires you pass the size when sending a
> P9_RREAD message back to the guest, i.e. only in the case when is_write is
> false, correct ?
That's right
> If so, for better clarity, what about having two distinct init_in_iov_from_pdu
> and init_out_iov_from_pdu ops, with an explicit comment in the virtio-9p
> implementation of init_in_iov_from_pdu so that someone isn't tempted to drop
> the apparently unused size argument ? Would this be ok for you on the Xen
> side ?
Sure, that's fine by me. I'll do that.
> Cheers.
>
> --
> Greg
>
> > hw/9pfs/9p.c | 2 +-
> > hw/9pfs/9p.h | 2 +-
> > hw/9pfs/virtio-9p-device.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index b6ec042..b82212b 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1652,7 +1652,7 @@ 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);
> > + pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write, skip + 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 ab398d0..c830188 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -348,7 +348,7 @@ 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);
> > + unsigned int *pniov, bool is_write, size_t size);
> > void (*push_and_notify)(V9fsPDU *pdu);
> > };
> >
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index e1a37a4..e2b27e8 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -172,7 +172,7 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > }
> >
> > static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > - unsigned int *pniov, bool is_write)
> > + unsigned int *pniov, bool is_write, size_t size)
> > {
> > V9fsState *s = pdu->s;
> > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] 9pfs: move pdus to V9fsState
2016-11-21 21:39 ` Stefano Stabellini
@ 2016-11-24 8:30 ` Greg Kurz
-1 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 8:30 UTC (permalink / raw)
To: Stefano Stabellini
Cc: aneesh.kumar, qemu-devel, xen-devel, anthony.perard, wei.liu2
On Mon, 21 Nov 2016 13:39:29 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:
> 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;
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 1/4] 9pfs: move pdus to V9fsState
@ 2016-11-24 8:30 ` Greg Kurz
0 siblings, 0 replies; 30+ messages in thread
From: Greg Kurz @ 2016-11-24 8:30 UTC (permalink / raw)
To: Stefano Stabellini
Cc: anthony.perard, xen-devel, wei.liu2, aneesh.kumar, qemu-devel
On Mon, 21 Nov 2016 13:39:29 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:
> 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;
^ permalink raw reply [flat|nested] 30+ messages in thread