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

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> 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...

Yes, that will save lot of checking in the register accessors.

>
>> +	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>