[SLOF] [PATCH v2 03/19] virtio: fix code style/design issues.

Thomas Huth thuth at redhat.com
Wed Jan 20 23:42:33 AEDT 2016


On 20.01.2016 13:10, Nikunj A Dadhania wrote:
> The patch does not make any functional changes.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>  lib/libvirtio/virtio-net.c  |   2 +-
>  lib/libvirtio/virtio-scsi.c | 156 ++++++++++++++++++++++----------------------
>  lib/libvirtio/virtio.c      |  63 ++++++++----------
>  lib/libvirtio/virtio.h      |   5 +-
>  4 files changed, 110 insertions(+), 116 deletions(-)
...
> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
> index f9c00a6..8da10c8 100644
> --- a/lib/libvirtio/virtio.c
> +++ b/lib/libvirtio/virtio.c
...
> @@ -222,20 +219,18 @@ uint64_t virtio_get_config(struct virtio_device *dev, int offset, int size)
>   * Get config blob
>   */
>  int __virtio_read_config(struct virtio_device *dev, void *dst,
> -			  int offset, int len)
> +			 int offset, int len)
>  {
>  	void *confbase;
>  	unsigned char *buf = dst;
>  	int i;
>  
> -	switch (dev->type) {
> -	 case VIRTIO_TYPE_PCI:
> -		confbase = dev->base+VIRTIOHDR_DEVICE_CONFIG;
> -		break;
> -	 default:
> +	if (dev->type != VIRTIO_TYPE_PCI)
>  		return 0;
> -	}

The original purpose of the switch statement was to support other access
types like virtio-mmio (which we used before there was PCI support in
the pseries machine of QEMU) ... I guess that dev->type stuff could be
cleaned up completely nowadays since we likely don't need it at all
anymore...

> +	confbase = dev->base + VIRTIOHDR_DEVICE_CONFIG;
>  	for (i = 0; i < len; i++)
>  		buf[i] = ci_read_8(confbase + offset + i);
> +
>  	return len;
>  }
> diff --git a/lib/libvirtio/virtio.h b/lib/libvirtio/virtio.h
> index d5759b4..9d6d8da 100644
> --- a/lib/libvirtio/virtio.h
> +++ b/lib/libvirtio/virtio.h
> @@ -34,7 +34,7 @@ struct vring_desc {
>  	uint32_t len;		/* Length */
>  	uint16_t flags;		/* The flags as indicated above */
>  	uint16_t next;		/* Next field if flags & NEXT */
> -}; 
> +};
>  
>  /* Definitions for vring_avail.flags */
>  #define VRING_AVAIL_F_NO_INTERRUPT	1
> @@ -44,8 +44,7 @@ struct vring_avail {
>  	uint16_t flags;
>  	uint16_t idx;
>  	uint16_t ring[];
> -}; 
> -
> +};
>  
>  /* Definitions for vring_used.flags */
>  #define VRING_USED_F_NO_NOTIFY		1

Reviewed-by: Thomas Huth <thuth at redhat.com>




More information about the SLOF mailing list