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

[Eric Blake]

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(-)
> + *
> + * Returns true on success, false otherwise.
> + */
> +static bool
> +virCommandFDSet(int fd,
> +                int **set,
> +                int *set_size)
> +{
> +    if (fd < 0 || !set || !set_size)
> +        return false;

I'd rather return -1 here on usage error,

> +
> +    if (virCommandFDIsSet(fd, *set, *set_size))
> +        return true;
> +
> +    if (VIR_REALLOC_N(*set, *set_size + 1) < 0) {
> +        virReportOOMError();
> +        return false;

and ENOMEM on memory allocation failure, with no virReportOOMError() at
this phase of the game,

> +    }
> +
> +    (*set)[*set_size] = fd;
> +    (*set_size)++;
> +
> +    return true;

and 0 on success...

> @@ -739,16 +798,16 @@ virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
>      if (!cmd)
>          return fd > STDERR_FILENO;
>  
> -    if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) {
> +    if (fd <= STDERR_FILENO ||
> +        !virCommandFDSet(fd, &cmd->preserve, &cmd->preserve_size) ||
> +        (transfer && !virCommandFDSet(fd, &cmd->transfer,
> +                                      &cmd->transfer_size))) {
>          if (!cmd->has_error)
>              cmd->has_error = -1;

so that down here, we capture the output of virCommandFDSet and assign
it to cmd->has_error if it was non-zero.  That way, an allocation error
will propagate all the way back to the user, by giving them the
virReportOOMError() at the point in time where they call virCommandRun().

That is, if the user is constructing a virCommandPtr by pieces, and
encounters some _other_ error along the way, we aren't overwriting their
other error with virReportOOMError().  The virCommand code should _only_
emit errors on the code paths where the user has completed all their
other processing, and requires the virCommandPtr to be successfully
built by reaching the virCommandRun() code.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120103/cbeec388/attachment-0001.sig>