[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] command: Discard FD_SETSIZE limit for opened files

On 01/03/2012 11:06 AM, Daniel P. Berrange wrote:
> On Tue, Jan 03, 2012 at 09:33:03AM -0700, Eric Blake wrote:
>> On 01/03/2012 04:14 AM, Michal Privoznik wrote:
>>> Currently, virCommand implementation uses FD_ macros from
>>> sys/select.h. However, those cannot handle more opened files
>>> than FD_SETSIZE. Therefore switch to generalized implementation
>>> based on array of integers.
>>> ---
>>>  src/util/command.c |  108 ++++++++++++++++++++++++++++++++++++++++------------
>>>  1 files changed, 83 insertions(+), 25 deletions(-)
>> NACK.  While I agree with the idea, I'd rather use virBitmap rather than
>> open-coding an int array ourselves.
> Is it worth considering the efficiency implications of virBitmap
> in this use case ?
> Michal's initial patch is pretty time & space efficient, since we
> only need to store & iterate over an array that is equal in size
> to the number of preserved FDs.
> Now if we want to create a new 'virSet' class for efficiently dealing
> with sparse sets.....otherwise I'm inclined to say this patch is fine
> as it is.

Fair points.  I'd call such a new class virBitset, rather than virSet,
but agree that the number of preserved FDs is small enough that the
efficiency hit of iterating through the set is not that bad, so the
patch as-is shouldn't be tied up waiting for a new efficient sparse
bit-set data structure.

I retract my NACK.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]