diff mbox

[v2,4/4] 9pfs: introduce init_out/in_iov_from_pdu

Message ID 1480368444-4310-4-git-send-email-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini Nov. 28, 2016, 9:27 p.m. UTC
Not all 9pfs transports share memory between request and response. For
those who don't, it is necessary to know how much memory is required in
the response.

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

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

Comments

Greg Kurz Dec. 2, 2016, 11:05 a.m. UTC | #1
On Mon, 28 Nov 2016 13:27:24 -0800
Stefano Stabellini <sstabellini@kernel.org> wrote:

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

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

>  hw/9pfs/9p.c               |  6 +++++-
>  hw/9pfs/9p.h               |  6 ++++--
>  hw/9pfs/virtio-9p-device.c | 28 ++++++++++++++++++----------
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 79d7201..ce1f9d9 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1652,7 +1652,11 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
>      struct iovec *iov;
>      unsigned int niov;
>  
> -    pdu->s->transport->init_iov_from_pdu(pdu, &iov, &niov, is_write);
> +    if (is_write) {
> +        pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> +    } else {
> +        pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size);
> +    }
>  
>      qemu_iovec_init_external(&elem, iov, niov);
>      qemu_iovec_init(qiov, niov);
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index c8c9aa8..4c4feaf 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -349,8 +349,10 @@ struct V9fsTransport {
>                                  va_list ap);
>      ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
>                                    va_list ap);
> -    void        (*init_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write);
> +    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size);
> +    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov);
>      void        (*push_and_notify)(V9fsPDU *pdu);
>  };
>  
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 273425b..27a4a32 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -171,26 +171,34 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
> -static void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> -                                     unsigned int *pniov, bool is_write)
> +/* The size parameter is used by other transports. Do not drop it. */
> +static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                        unsigned int *pniov, size_t size)
>  {
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>      VirtQueueElement *elem = v->elems[pdu->idx];
>  
> -    if (is_write) {
> -        *piov = elem->out_sg;
> -        *pniov = elem->out_num;
> -    } else {
> -        *piov = elem->in_sg;
> -        *pniov = elem->in_num;
> -    }
> +    *piov = elem->in_sg;
> +    *pniov = elem->in_num;
> +}
> +
> +static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> +                                         unsigned int *pniov)
> +{
> +    V9fsState *s = pdu->s;
> +    V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> +    VirtQueueElement *elem = v->elems[pdu->idx];
> +
> +    *piov = elem->out_sg;
> +    *pniov = elem->out_num;
>  }
>  
>  static const struct V9fsTransport virtio_9p_transport = {
>      .pdu_vmarshal = virtio_pdu_vmarshal,
>      .pdu_vunmarshal = virtio_pdu_vunmarshal,
> -    .init_iov_from_pdu = virtio_init_iov_from_pdu,
> +    .init_in_iov_from_pdu = virtio_init_in_iov_from_pdu,
> +    .init_out_iov_from_pdu = virtio_init_out_iov_from_pdu,
>      .push_and_notify = virtio_9p_push_and_notify,
>  };
>
Stefano Stabellini Dec. 2, 2016, 7:02 p.m. UTC | #2
On Fri, 2 Dec 2016, Greg Kurz wrote:
> On Mon, 28 Nov 2016 13:27:24 -0800
> Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> > Not all 9pfs transports share memory between request and response. For
> > those who don't, it is necessary to know how much memory is required in
> > the response.
> > 
> > Split the existing init_iov_from_pdu function in two:
> > init_out_iov_from_pdu (for writes) and init_in_iov_from_pdu (for reads).
> > init_in_iov_from_pdu takes an additional size parameter to specify the
> > memory required for the response message.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

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

Thanks again,

Stefano


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

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

Done.

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

Cheers.

--
Greg

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

Patch

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