Re: [PATCH v7 5/7] qapi/qdev.json: add DEVICE_UNPLUG_GUEST_ERROR QAPI event

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 9/4/21 8:49 AM, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
>> On Wed, Sep 01, 2021 at 03:19:26PM +0200, Markus Armbruster wrote:
>>> Daniel Henrique Barboza <danielhb413@gmail.com> writes:
>>>
>>>> At this moment we only provide one event to report a hotunplug error,
>>>> MEM_UNPLUG_ERROR. As of Linux kernel 5.12 and QEMU 6.0.0, the pseries
>>>> machine is now able to report unplug errors for other device types, such
>>>> as CPUs.
>>>>
>>>> Instead of creating a (device_type)_UNPLUG_ERROR for each new device,
>>>> create a generic DEVICE_UNPLUG_GUEST_ERROR event that can be used by all
>>>> guest side unplug errors in the future. This event has a similar API as
>>>> the existing DEVICE_DELETED event, always providing the QOM path of the
>>>> device and dev->id if there's any.
>>>>
>>>> With this new generic event, MEM_UNPLUG_ERROR is now marked as deprecated.
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>> ---
>>>
>>> [...]
>>>
>>>> diff --git a/qapi/qdev.json b/qapi/qdev.json
>>>> index 0e9cb2ae88..8b1a1dd43b 100644
>>>> --- a/qapi/qdev.json
>>>> +++ b/qapi/qdev.json
>>>> @@ -84,7 +84,9 @@
>>>>   #        This command merely requests that the guest begin the hot removal
>>>>   #        process.  Completion of the device removal process is signaled with a
>>>>   #        DEVICE_DELETED event. Guest reset will automatically complete removal
>>>> -#        for all devices.
>>>> +#        for all devices.  If a guest-side error in the hot removal process is
>>>> +#        detected, the device will not be removed and a DEVICE_UNPLUG_GUEST_ERROR
>>>> +#        event is sent.  Some errors cannot be detected.
>>>>   #
>>>>   # Since: 0.14
>>>>   #
>>>> @@ -124,3 +126,27 @@
>>>>   ##
>>>>   { 'event': 'DEVICE_DELETED',
>>>>     'data': { '*device': 'str', 'path': 'str' } }
>>>> +
>>>> +##
>>>> +# @DEVICE_UNPLUG_GUEST_ERROR:
>>>> +#
>>>> +# Emitted when a device hot unplug fails due to an internal guest
>>>> +# error.
>>>
>>> Suggest to scratch "internal".
>>
>> I'd suggest s/internal guest/guest reported/.  "guest error" is a bit
>> vague, this doesn't neccessarily indicate a bug in the guest.  The key
>> point is that we're reporting this event because the guest performed
>> some explicit action to tell us something went wrong with the plug
>> attempt.
> 
> Yes, that's better.


I agree.  David, let me know if you need another spin with this change.



Thanks,


Daniel

> 
> [...]
>