[SLOF] [PATCH v2 07/19] virtio-{net, blk, scsi, 9p}: use status variable

[Alexey Kardashevskiy]

On 01/22/2016 05:53 PM, Nikunj A Dadhania wrote:
> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>
>> On 01/22/2016 04:41 PM, Nikunj A Dadhania wrote:
>>> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>>>
>>>>>>>
>>>>>>>      	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.
>>>>
>>>>
>>>> I'd add a printf into virtio_set_status() when VIRTIO_STAT_FAILED is set...
>>>
>>> How do you know which file/device is calling it?
>>
>> Why does this matter? I just want to know if device stopped working,
>> straight away, helps with debugging, no?
>
> Dont you need to know which device to debug virtio-{net,9p,scsi,blk}?
> For SLOF, its single threaded and you can figure that out. In case
> of multi-thread, its not a good practice.

Ok, ok :)


>>> On the side note, I would not want to do checking in my low-level
>>> routines to log errors. This slows down virtio_set_status fast path,
>>> isn't it ?
>>
>>
>> You do not have to print an error every time you call virtio_set_status,
>> just when it becomes set.
>
> You would need to do:
>
> virtio_set_status() {
> ...
>     if ((status & VIRTIO_STAT_FAILED) == VIRTIO_STAT_FAILED) {
>        print something
>     }
> ...
> }

I thought of checking if the new status has FAILED and if it does, then 
read the old status and if it does not have FAILED bit, then print the message.


>
> So every call using set_status, and delays. The caller knows when its
> setting this, it can print that.






-- 
Alexey