Re: Re: [PATCH v4 0/3] support NVMe smart critial warning injection

From: zhenwei pi <pizhenwei@bytedance.com>
To: k.jensen@samsung.com
Cc: kwolf@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org,
	mreitz@redhat.com, kbusch@kernel.org, philmd@redhat.com
Subject: Re: Re: [PATCH v4 0/3] support NVMe smart critial warning injection
Date: Tue, 19 Jan 2021 10:05:50 +0800	[thread overview]
Message-ID: <1797a9bb-f0bf-4377-2fb7-e3dc6fcba733@bytedance.com> (raw)
In-Reply-To: <YAVWGC4HKFlaKolQ@apples.localdomain>

On 1/18/21 5:34 PM, Klaus Jensen wrote:
> On Jan 15 11:26, zhenwei pi wrote:
>> v3 -> v4:
>> - Drop "Fix overwritten bar.cap". (Already fixed)
>>
>> - Avoid to enqueue the duplicate event.
>>
>> - Several minor changes for coding style & function/variable name.
>>
>> v2 -> v3:
>> - Introduce "Persistent Memory Region has become read-only or
>>    unreliable"
>>
>> - Fix overwritten bar.cap
>>
>> - Check smart critical warning value from QOM.
>>
>> - Trigger asynchronous event during smart warning injection.
>>
>> v1 -> v2:
>> - Suggested by Philippe & Klaus, set/get smart_critical_warning by QMP.
>>
>> v1:
>> - Add smart_critical_warning for nvme device which can be set by QEMU
>>    command line to emulate hardware error.
>>
>> Zhenwei Pi (3):
>>    block/nvme: introduce bit 5 for critical warning
>>    hw/block/nvme: add smart_critical_warning property
>>    hw/blocl/nvme: trigger async event during injecting smart warning
>>
>>   hw/block/nvme.c      | 91 +++++++++++++++++++++++++++++++++++++++-----
>>   hw/block/nvme.h      |  1 +
>>   include/block/nvme.h |  3 ++
>>   3 files changed, 86 insertions(+), 9 deletions(-)
>>
> 
> This looks pretty good to me.
> 
> I think maybe we want to handle the duplicate event stuff more generally
> from the AER/AEN code, but this does the job.
> 
> Tested-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> 

What's the next step I should take? Should I push a new version to 
implement this purpose? From my understanding, before inserting a new 
event to aer_queue, I can parse all the pending aer to find the same event.

nvme_enqueue_event()
{
     ...

     QTAILQ_FOREACH_SAFE(event, &n->aer_queue, entry, next) {
         if ((event->result.event_type == event_type)
             && (event->result.event_info == event_info)
             && (event->result.log_page == log_page))
             return;
     }

     QTAILQ_INSERT_TAIL(&n->aer_queue, event, entry); 

     n->aer_queued++;
     ...
}

-- 
zhenwei pi