On Tue, 12 Apr 2016 14:25:29 +0100
Stefan Hajnoczi <stefa...@redhat.com> 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 <stefa...@redhat.com>
> ---
> 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.
