On 07/04/2012 04:09 AM, Kevin Wolf wrote:
Am 03.07.2012 20:21, schrieb Corey Bryant:
On 07/03/2012 02:00 PM, Eric Blake wrote:
On 07/03/2012 11:46 AM, Corey Bryant wrote:
Yes, I think adding a +1 to the refcount for the monitor makes sense.
I'm a bit unsure how to increment the refcount when a monitor reconnects
though. Maybe it is as simple as adding a +1 to each fd's refcount when
the next QMP monitor connects.
Or maybe delay a +1 until after a 'query-fds' - it is not until the
monitor has reconnected and learned what fds it should be aware of that
incrementing the refcount again makes sense. But that would mean making
'query-fds' track whether this is the first call since the monitor
reconnected, as it shouldn't normally increase refcounts.
This doesn't sound ideal.
Yes, it's less than ideal.
The other alternative is that the monitor never re-increments a
refcount. Once a monitor disconnects, that fd is lost to the monitor,
and a reconnected monitor must pass in a new fd to be re-associated with
the fdset. In other words, the monitor's use of an fd is a one-way
operation, starting life in use but ending at the first disconnect or
remove-fd.
I would vote for this 2nd alternative. As long as we're not introducing
an fd leak. And I don't think we are if we decrement the refcount on
remove-fd or on QMP disconnect.
In fact, I believe this one is even worse. I can already see hacks like
adding a dummy FD with invalid flags and removing it again just to
regain control over the fdset...
You earlier suggestion made a lot of sense to me: Whenever a new QMP
monitor is connected, increase the refcount. That is, as long as any
monitor is there, don't drop any fdsets unless explicitly requested via QMP.
Ok. So refcount would be incremented (for the fd or fdset, whatever we
decide on) when QMP reconnects. I'm assuming we wouldn't wait until
after a query-fds call.
I'll go with this approach until someone objects.
--
Regards,
Corey
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list