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

[Daniel Henrique Barboza]

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

> [...]