[SLOF] [PATCH v1 05/27] virtio-blk: use virt queue init and reset init

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> On 01/13/2016 10:16 PM, Nikunj A Dadhania wrote:
>> Move initialization of virt queue to the init and
>> with this reset will start working now.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>>   board-qemu/slof/pci-device_1af4_1001.fs |  9 ---------
>>   lib/libvirtio/virtio-blk.c              | 13 +++++++++----
>>   2 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/board-qemu/slof/pci-device_1af4_1001.fs b/board-qemu/slof/pci-device_1af4_1001.fs
>> index 4591ccb..db0bb3f 100644
>> --- a/board-qemu/slof/pci-device_1af4_1001.fs
>> +++ b/board-qemu/slof/pci-device_1af4_1001.fs
>> @@ -22,13 +22,4 @@ pci-io-enable
>>
>>   s" virtio-block.fs" included
>>
>> -0 VALUE queue-addr
>> -
>> -\ Allocate memory for virtio queue:
>> -virtiodev 0 virtio-get-qsize virtio-vring-size
>> -alloc-mem to queue-addr
>> -
>> -\ Write queue address into device:
>> -virtiodev 0 queue-addr virtio-set-qaddr
>> -
>
> This seems to belong to 04/27.

04/27 introduces a generic api, so this is the right place.There are
other drivers as well using this.

I am using the api here and earlier driver never called
virtio_reset_device, because the init was done in the above code. Now
with the api, I do virt queue init in virtio_blk_init of the driver
after the device reset.

>
>
>>   pci-device-disable
>> diff --git a/lib/libvirtio/virtio-blk.c b/lib/libvirtio/virtio-blk.c
>> index b8c4da2..2d8982d 100644
>> --- a/lib/libvirtio/virtio-blk.c
>> +++ b/lib/libvirtio/virtio-blk.c
>> @@ -18,6 +18,8 @@
>>
>>   #define DEFAULT_SECTOR_SIZE 512
>>
>> +struct vqs vq;
>> +
>>   /**
>>    * Initialize virtio-block device.
>>    * @param  dev  pointer to virtio device information
>> @@ -30,10 +32,7 @@ virtioblk_init(struct virtio_device *dev)
>>   	int features;
>>
>>   	/* Reset device */
>> -	// XXX That will clear the virtq base. We need to move
>> -	//     initializing it to here anyway
>> -	//
>> -	//	 virtio_reset_device(dev);
>> +	virtio_reset_device(dev);
>>
>>   	/* Acknowledge device. */
>>   	virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE);
>> @@ -44,6 +43,12 @@ virtioblk_init(struct virtio_device *dev)
>>   	/* Device specific setup - we support F_BLK_SIZE */
>>   	virtio_set_guest_features(dev,  VIRTIO_BLK_F_BLK_SIZE);
>>
>> +	if(!virtio_queue_init_vq(dev, &vq, 0)) {
>> +		virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER
>> +				  |VIRTIO_STAT_FAILED);
>> +		return 0;
>> +	}
>> +
>
> This is confusing chunk - you call a helper to init a queue now but I 
> assume it used to work already - how did it work? That removed chunk of 
> forth code above did the job? So this initialization has moved? It did not 
> set status before and now it does? Commit log would not hurt anyway.

I guess i need to detail more in the log ?

"Move initialization of virt queue to the init and with this reset will start working now."

>
>>   	vq_avail = virtio_get_vring_avail(dev, 0);
>>   	vq_avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>>   	vq_avail->idx = 0;
>>
>
>
> -- 
> Alexey