Jag Raman <jag.ra...@oracle.com> writes:
>> On Apr 21, 2022, at 10:55 AM, Markus Armbruster <arm...@redhat.com> wrote:
>>
>> Jagannathan Raman <jag.ra...@oracle.com> writes:
>>
>>> Add blocker to prevent hot-unplug of devices
>>
>> Why do you need this? I'm not doubting you do, I just want to read your
>> reasons here :)
>
> Hi Markus, :)
>
> The x-vfio-user-server depends on an attached PCIDevice. As long as
> x-vfio-user-server
> is used, we don’t want the PCIDevice to be unplugged. This blocker prevents
> an user
> from removing PCIDevice while the vfio-user server is in use.
Please work that into your commit message. Perhaps along the lines of
One of the next commits will do <stuff>. <badness> will happen when
the PCI device is unplugged. Create the means to prevent that.
>>> Signed-off-by: Elena Ufimtseva <elena.ufimts...@oracle.com>
>>> Signed-off-by: John G Johnson <john.g.john...@oracle.com>
>>> Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com>
>
> I recall receiving a “Reviewed-by” from Stefan previously.
>
> I’m very sorry I didn’t add that here. I’ll go over all the patches once
> again to confirm that
> the “Reviewed-by” status reflects accurately.
>
>>> ---
>>> 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);
>>>
>>> +/*
>>> + * qdev_add_unplug_blocker: Adds an unplug blocker to a device
>>> + *
>>> + * @dev: Device to be blocked from unplug
>>> + * @reason: Reason for blocking
>>> + */
>>> +void qdev_add_unplug_blocker(DeviceState *dev, Error *reason);
>>> +
>>> +/*
>>> + * qdev_del_unplug_blocker: Removes an unplug blocker from a device
>>> + *
>>> + * @dev: Device to be unblocked
>>> + * @reason: Pointer to the Error used with qdev_add_unplug_blocker.
>>> + * Used as a handle to lookup the blocker for deletion.
>>> + */
>>> +void qdev_del_unplug_blocker(DeviceState *dev, Error *reason);
>>> +
>>> +/*
>>> + * qdev_unplug_blocked: Confirms if a device is blocked from unplug
>>> + *
>>> + * @dev: Device to be tested
>>> + * @reason: Returns one of the reasons why the device is blocked,
>>> + * if any
>>> + *
>>> + * Returns: true if device is blocked from unplug, false otherwise
>>> + */
>>> +bool qdev_unplug_blocked(DeviceState *dev, Error **errp);
>>> +
>>> /**
>>> * GpioPolarity: Polarity of a GPIO line
>>> *
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 84f3019440..0806d8fcaa 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -468,6 +468,28 @@ char *qdev_get_dev_path(DeviceState *dev)
>>> return NULL;
>>> }
>>>
>>> +void qdev_add_unplug_blocker(DeviceState *dev, Error *reason)
>>> +{
>>> + dev->unplug_blockers = g_slist_prepend(dev->unplug_blockers, reason);
>>> +}
>>> +
>>> +void qdev_del_unplug_blocker(DeviceState *dev, Error *reason)
>>> +{
>>> + dev->unplug_blockers = g_slist_remove(dev->unplug_blockers, reason);
>>> +}
>>> +
>>> +bool qdev_unplug_blocked(DeviceState *dev, Error **errp)
>>> +{
>>> + ERRP_GUARD();
>>> +
>>> + if (dev->unplug_blockers) {
>>> + error_propagate(errp, error_copy(dev->unplug_blockers->data));
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>
>> This cites the most recently added blocker as reason. Your function
>> comment covers it: "Returns one of the reasons". Okay.
>
> I could change the comment to say that it returns the recently added reason.
Up to you.
