[SLOF] [PATCH v2 07/19] virtio-{net, blk, scsi, 9p}: use status variable
Nikunj A Dadhania
nikunj at linux.vnet.ibm.com
Fri Jan 22 16:11:22 AEDT 2016
Alexey Kardashevskiy <aik at ozlabs.ru> writes:
> On 01/20/2016 11:10 PM, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>> lib/libvirtio/virtio-9p.c | 22 +++++++++++++---------
>> lib/libvirtio/virtio-blk.c | 22 +++++++++++++---------
>> lib/libvirtio/virtio-net.c | 17 +++++++++++------
>> lib/libvirtio/virtio-scsi.c | 10 ++++++----
>> 4 files changed, 43 insertions(+), 28 deletions(-)
>>
>> diff --git a/lib/libvirtio/virtio-9p.c b/lib/libvirtio/virtio-9p.c
>> index 2d1d3f5..f1595c7 100644
>> --- a/lib/libvirtio/virtio-9p.c
>> +++ b/lib/libvirtio/virtio-9p.c
>> @@ -166,6 +166,7 @@ int virtio_9p_init(struct virtio_device *dev, void *tx_buf, void *rx_buf,
>> int buf_size)
>> {
>> struct vring_avail *vq_avail;
>> + uint32_t status = VIRTIO_STAT_ACKNOWLEDGE;
>>
>> /* Check for double open */
>> if (__buf_size)
>> @@ -178,27 +179,25 @@ int virtio_9p_init(struct virtio_device *dev, void *tx_buf, void *rx_buf,
>> virtio_reset_device(dev);
>>
>> /* Acknowledge device. */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE);
>> + virtio_set_status(dev, status);
>>
>> /* Tell HV that we know how to drive the device. */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE | VIRTIO_STAT_DRIVER);
>> + status |= VIRTIO_STAT_DRIVER;
>> + virtio_set_status(dev, status);
>>
>> /* Device specific setup - we do not support special features */
>> virtio_set_guest_features(dev, 0);
>>
>> - if(!virtio_queue_init_vq(dev, &vq, 0)) {
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER
>> - |VIRTIO_STAT_FAILED);
>> - return -1;
>> - }
>> + if(!virtio_queue_init_vq(dev, &vq, 0))
>> + goto dev_error;
>>
>> vq_avail = virtio_get_vring_avail(dev, 0);
>> vq_avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> vq_avail->idx = 0;
>>
>> /* Tell HV that setup succeeded */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE | VIRTIO_STAT_DRIVER
>> - |VIRTIO_STAT_DRIVER_OK);
>> + status |= VIRTIO_STAT_DRIVER_OK;
>> + virtio_set_status(dev, status);
>>
>> /* Setup 9P library. */
>> p9_reg_transport(virtio_9p_transact, dev,(uint8_t *)tx_buf,
>> @@ -206,6 +205,11 @@ int virtio_9p_init(struct virtio_device *dev, void *tx_buf, void *rx_buf,
>>
>> dprintf("%s : complete\n", __func__);
>> return 0;
>> +
>> +dev_error:
>> + status |= VIRTIO_STAT_FAILED;
>> + virtio_set_status(dev, status);
>> + return -1;
>> }
>>
>> /**
>> diff --git a/lib/libvirtio/virtio-blk.c b/lib/libvirtio/virtio-blk.c
>> index 8ae61bc..98c1a80 100644
>> --- a/lib/libvirtio/virtio-blk.c
>> +++ b/lib/libvirtio/virtio-blk.c
>> @@ -31,32 +31,31 @@ virtioblk_init(struct virtio_device *dev)
>> struct vring_avail *vq_avail;
>> int blk_size = DEFAULT_SECTOR_SIZE;
>> int features;
>> + uint32_t status = VIRTIO_STAT_ACKNOWLEDGE;
>>
>> /* Reset device */
>> virtio_reset_device(dev);
>>
>> /* Acknowledge device. */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE);
>> + virtio_set_status(dev, status);
>>
>> /* Tell HV that we know how to drive the device. */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER);
>> + status |= VIRTIO_STAT_DRIVER;
>> + virtio_set_status(dev, status);
>>
>> /* 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;
>> - }
>> + if(!virtio_queue_init_vq(dev, &vq, 0))
>> + goto dev_error;
>>
>> vq_avail = virtio_get_vring_avail(dev, 0);
>> vq_avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> vq_avail->idx = 0;
>>
>> /* Tell HV that setup succeeded */
>> - virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER
>> - |VIRTIO_STAT_DRIVER_OK);
>> + status |= VIRTIO_STAT_DRIVER_OK;
>> + virtio_set_status(dev, status);
>>
>> virtio_get_host_features(dev, &features);
>> if (features & VIRTIO_BLK_F_BLK_SIZE) {
>> @@ -66,6 +65,11 @@ virtioblk_init(struct virtio_device *dev)
>> }
>>
>> return blk_size;
>> +dev_error:
>> + fprintf(stderr, "Device Error \n");
>
>
> This seems to be unrelated change but ok. I'd think that having
> VIRTIO_STAT_FAILED bit set will trigger some message in SLOF anyway...
There wont be any error in SLOF by writing this, its will just inform
the HV about the inability of the guest to drive the device.
That was the reason for this error. As I mentioned in my mail to Thomas,
i will make this change consistent and have it in virtio-9p has well.
>
>
>> + status |= VIRTIO_STAT_FAILED;
>> + virtio_set_status(dev, status);
>> + return 0;
>> }
>>
>>
>> diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c
>> index 5c7d771..cafaa4b 100644
>> --- a/lib/libvirtio/virtio-net.c
>> +++ b/lib/libvirtio/virtio-net.c
>> @@ -97,6 +97,7 @@ static int virtionet_init_pci(struct virtio_device *dev)
>> static int virtionet_init(net_driver_t *driver)
>> {
>> int i;
>> + uint32_t status = VIRTIO_STAT_ACKNOWLEDGE | VIRTIO_STAT_DRIVER;
>>
>> dprintf("virtionet_init(%02x:%02x:%02x:%02x:%02x:%02x)\n",
>> driver->mac_addr[0], driver->mac_addr[1],
>> @@ -107,7 +108,7 @@ static int virtionet_init(net_driver_t *driver)
>> return 0;
>>
>> /* Tell HV that we know how to drive the device. */
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER);
>> + virtio_set_status(&virtiodev, status);
>>
>> /* Device specific setup - we do not support special features right now */
>> virtio_set_guest_features(&virtiodev, 0);
>> @@ -117,8 +118,7 @@ static int virtionet_init(net_driver_t *driver)
>> * RX_QUEUE_SIZE);
>> if (!vq[VQ_RX].buf_mem) {
>> printf("virtionet: Failed to allocate buffers!\n");
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_FAILED);
>> - return -1;
>> + goto dev_error;
>> }
>>
>> /* Prepare receive buffer queue */
>> @@ -151,9 +151,8 @@ static int virtionet_init(net_driver_t *driver)
>> vq[VQ_TX].avail->idx = 0;
>>
>> /* Tell HV that setup succeeded */
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_ACKNOWLEDGE
>> - |VIRTIO_STAT_DRIVER
>> - |VIRTIO_STAT_DRIVER_OK);
>> + status |= VIRTIO_STAT_DRIVER_OK;
>> + virtio_set_status(&virtiodev, status);
>>
>> /* Tell HV that RX queues are ready */
>> virtio_queue_notify(&virtiodev, VQ_RX);
>> @@ -161,6 +160,12 @@ static int virtionet_init(net_driver_t *driver)
>> driver->running = 1;
>>
>> return 0;
>> +
>> +dev_error:
>> + driver->running = 0;
>
>
> This is new, it should not hurt but still - why?
There was no case of error earlier, we have it now, so i need to mark
that the driver is not functional.
Regards
Nikunj
More information about the SLOF
mailing list