Re: [Qemu-devel] [PATCH v3 05/10] virtio: handle virtqueue_map_desc() errors

[Cornelia Huck]

On Tue, 12 Apr 2016 14:25:29 +0100
Stefan Hajnoczi <address@hidden> wrote:

> Errors can occur during virtqueue_pop(), especially in
> virtqueue_map_desc().  In order to handle this we must unmap iov[]
> before returning NULL.  The caller will consider the virtqueue empty and
> the virtio_error() call will have marked the device broken.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  hw/virtio/virtio.c | 62 
> ++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 49 insertions(+), 13 deletions(-)
> 

> +/* Only used by error code paths before we have a VirtQueueElement (therefore
> + * virtqueue_unmap_sg() can't be used).  Assumes buffers weren't written to
> + * yet.
> + */

Hm... didn't such an undo function float around in another context as
well? Is it worth factoring out?

> +static void virtqueue_undo_map_desc(unsigned out_num, unsigned in_num,
> +                                    struct iovec *iov)
> +{
> +    unsigned i;

Personally, I don't like pure 'unsigned'. Matter of taste, I guess.

> +
> +    for (i = 0; i < out_num + in_num; i++) {
> +        int is_write = i >= out_num;
> +
> +        cpu_physical_memory_unmap(iov->iov_base, iov->iov_len, is_write, 0);
> +        iov++;
> +    }
>  }

Otherwise, looks good.