| Message ID | 1479764372-29470-2-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)
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) > >
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) > > > > > >
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.
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.)
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)
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(-)