> On Dec 20, 2021, at 3:29 AM, Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> On Fri, Dec 17, 2021 at 05:59:48PM +0000, Jag Raman wrote:
>>
>>
>>> On Dec 16, 2021, at 6:17 AM, Stefan Hajnoczi <stefa...@redhat.com> wrote:
>>>
>>> On Wed, Dec 15, 2021 at 10:35:31AM -0500, Jagannathan Raman wrote:
>>>> @@ -114,6 +118,62 @@ static void vfu_object_set_device(Object *obj, const
>>>> char *str, Error **errp)
>>>> vfu_object_init_ctx(o, errp);
>>>> }
>>>>
>>>> +static void vfu_object_ctx_run(void *opaque)
>>>> +{
>>>> + VfuObject *o = opaque;
>>>> + int ret = -1;
>>>> +
>>>> + while (ret != 0) {
>>>> + ret = vfu_run_ctx(o->vfu_ctx);
>>>> + if (ret < 0) {
>>>> + if (errno == EINTR) {
>>>> + continue;
>>>> + } else if (errno == ENOTCONN) {
>>>> + qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL);
>>>> + o->vfu_poll_fd = -1;
>>>> + object_unparent(OBJECT(o));
>>>> + break;
>>>
>>> If nothing else logs a message then I think that should be done here so
>>> users know why their vfio-user server object disappeared.
>>
>> Sure will do.
>>
>> Do you prefer a trace, or a message to the console? Trace makes sense to me.
>> Presently, the client could unplug the vfio-user device which would trigger
>> the
>> deletion of this object. This process could happen quietly.
>
> If there is no way to differentiate graceful disconnect from unexpected
> disconnect then logging might be too noisy.
I think that’s what happens in the regular VFIO case also.
vfio_put_base_device() closes the FD used for ioctls.
>
> Regarding the automatic deletion of the object, that might not be
> desirable for two reasons:
> 1. It prevents reconnection or another client connecting.
> 2. Management tools are in the dark about it.
>
> For #2 there are monitor events that QEMU emits to notify management
> tools about state changes like disconnections.
This is very interesting. I suppose you’re referring to something like
‘BLOCK_JOB_COMPLETED’ event.
It’d be good to inform the management tools about disconnection. Not
used this before, will check it out to gather ideas on how to use it.
>
> It's worth thinking about current and future use cases before baking in
> a policy like automatically deleting VfuObject on disconnect because
> it's inflexible and would require a QEMU update in the future to support
> a different policy.
>
> One approach is to emit a disconnect event but leave the VfuObject in a
> disconnected state. The management tool can then restart or clean up,
> depending on its policy.
>
> I'm not sure what's best because it depends on the use cases, but maybe
> you and others have ideas here.
>
>>>> @@ -208,6 +284,8 @@ static void vfu_object_init(Object *obj)
>>>> TYPE_VFU_OBJECT, TYPE_REMOTE_MACHINE);
>>>> return;
>>>> }
>>>> +
>>>> + o->vfu_poll_fd = -1;
>>>> }
>>>
>>> This must call qemu_set_fd_handler(o->vfu_poll_fd, NULL, NULL, NULL)
>>> when o->vfu_poll_fd != -1 to avoid leaving a dangling fd handler
>>> callback registered.
>>
>> This is during the init phase, and the FD handlers are not set. Do you mean
>> to add this at finalize?
>>
>> I agree it’s good to explicitly add this at finalize. But vfu_destroy_ctx()
>> should
>> trigger a ENOTCONN, which would do it anyway.
>
> I'm not sure my comment makes sense since this is the init function, not
> finalize.
>
> However, it's not clear to me that the o->vfu_poll_fd fd handler is
> unregistered from the event loop when VfuObject is finalized (e.g. by
> the object-del monitor command). You say vfu_destroy_ctx() triggers
> ENOTCONN, but the VfuObject is freed after finalize returns so when is
> the fd handler deregistered?
That is correct - will remove the FD handler in finalize also.
Thank you!
--
Jag
>
> Stefan
