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

[Markus Armbruster]

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.

[...]