[Qemu-devel] [PATCH] virtio-9p: don't break device on marshaling fail

[Qemu-devel] [PATCH] virtio-9p: don't break device on marshaling fail

[Jan Dakinevich]

Fix regression from commit 8d37de41cab145cbba250a8f1f1372b0a5cdf26f.

The situation when pdu_marshal returns an error should be considered
valid. At least `v9fs_do_readdir_with_stat' relies on that to place in
responce buffer as much data as can fit.

Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
---
 hw/9pfs/virtio-9p-device.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 62650b0..c3e5935 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -146,16 +146,8 @@ static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     VirtQueueElement *elem = v->elems[pdu->idx];
-    ssize_t ret;
 
-    ret = v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
-    if (ret < 0) {
-        VirtIODevice *vdev = VIRTIO_DEVICE(v);
-
-        virtio_error(vdev, "Failed to encode VirtFS reply type %d",
-                     pdu->id + 1);
-    }
-    return ret;
+    return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
 }
 
 static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
@@ -164,15 +156,8 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
     V9fsState *s = pdu->s;
     V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
     VirtQueueElement *elem = v->elems[pdu->idx];
-    ssize_t ret;
 
-    ret = v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
-    if (ret < 0) {
-        VirtIODevice *vdev = VIRTIO_DEVICE(v);
-
-        virtio_error(vdev, "Failed to decode VirtFS request type %d", pdu->id);
-    }
-    return ret;
+    return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
 }
 
 static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
-- 
2.1.4

Re: [Qemu-devel] [PATCH] virtio-9p: don't break device on marshaling fail

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 2425 bytes --]

On Wed, 13 Sep 2017 17:16:43 +0300
Jan Dakinevich <jan.dakinevich@gmail.com> wrote:

> Fix regression from commit 8d37de41cab145cbba250a8f1f1372b0a5cdf26f.
> 
> The situation when pdu_marshal returns an error should be considered
> valid. At least `v9fs_do_readdir_with_stat' relies on that to place in
> responce buffer as much data as can fit.
> 

Indeed, that's embarrassing... but since v9fs_do_readdir_with_stat() is the
only guy that does that, I'd prefer you send a patch to handle this special
case instead of reverting 8d37de41ca. Please add a new transport operation
for this.

Thanks!

> Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
> ---
>  hw/9pfs/virtio-9p-device.c | 19 ++-----------------
>  1 file changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 62650b0..c3e5935 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -146,16 +146,8 @@ static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>      VirtQueueElement *elem = v->elems[pdu->idx];
> -    ssize_t ret;
>  
> -    ret = v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
> -    if (ret < 0) {
> -        VirtIODevice *vdev = VIRTIO_DEVICE(v);
> -
> -        virtio_error(vdev, "Failed to encode VirtFS reply type %d",
> -                     pdu->id + 1);
> -    }
> -    return ret;
> +    return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
>  }
>  
>  static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> @@ -164,15 +156,8 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
>      V9fsState *s = pdu->s;
>      V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
>      VirtQueueElement *elem = v->elems[pdu->idx];
> -    ssize_t ret;
>  
> -    ret = v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
> -    if (ret < 0) {
> -        VirtIODevice *vdev = VIRTIO_DEVICE(v);
> -
> -        virtio_error(vdev, "Failed to decode VirtFS request type %d", pdu->id);
> -    }
> -    return ret;
> +    return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, ap);
>  }
>  
>  static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]