[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 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 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]