| Message ID | 1479764372-29470-4-git-send-email-sstabellini@kernel.org |
|---|---|
| State | New |
| Headers | show |
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);
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); >
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);
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(-)