Re: [PATCH v9 02/17] qdev: unplug blocker for devices

[Jag Raman]

> On May 4, 2022, at 7:13 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Jagannathan Raman <jag.raman@oracle.com> writes:
> 
>> Add blocker to prevent hot-unplug of devices
>> 
>> TYPE_VFIO_USER_SERVER, which is introduced shortly, attaches itself to a
>> PCIDevice on which it depends. If the attached PCIDevice gets removed
>> while the server in use, it could cause it crash. To prevent this,
>> TYPE_VFIO_USER_SERVER adds an unplug blocker for the PCIDevice.
> 
> Appreciate the explanation :)
> 
>> 
>> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
>> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> include/hw/qdev-core.h | 29 +++++++++++++++++++++++++++++
>> hw/core/qdev.c         | 24 ++++++++++++++++++++++++
>> softmmu/qdev-monitor.c |  4 ++++
>> 3 files changed, 57 insertions(+)
>> 
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 92c3d65208..1b9fa25e5c 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -193,6 +193,7 @@ struct DeviceState {
>>     int instance_id_alias;
>>     int alias_required_for_version;
>>     ResettableState reset;
>> +    GSList *unplug_blockers;
>> };
>> 
>> struct DeviceListener {
>> @@ -419,6 +420,34 @@ void qdev_simple_device_unplug_cb(HotplugHandler 
>> *hotplug_dev,
>> void qdev_machine_creation_done(void);
>> bool qdev_machine_modified(void);
>> 
>> +/*
> 
> Use /** here like we do in the other function comments nearby.
> 
> In case you're curious: it's GTK-Doc format.  It's intended for
> generating documentation from doc comments.  Which we don't do, and
> perhaps never will.  But let's be locally consistent.

Sure, will do.

> 
>> + * qdev_add_unplug_blocker: Adds an unplug blocker to a device
> 
> Recommend imperative mood for function comments: "Add an unplug
> blocker to a device".

OK, got it. Will update the comments. :)

--
Jag

> 
> More of the same below.
> 
> [...]
>