[libvirt] [PATCH v3] qemu: Pass file descriptor when using TPM passthrough

[Stefan Berger]

On 02/06/2015 01:56 PM, Eric Blake wrote:
> On 02/06/2015 09:41 AM, Stefan Berger wrote:
>
>>>> @@ -214,6 +215,12 @@ virCommandReorderFDs(virCommandPtr cmd)
>>>>        if (!cmd || cmd->has_error || !cmd->npassfd)
>>>>            return;
>>>>    +    if ((cmd->flags & VIR_EXEC_FIXED_FDS)) {
>>>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>>> +                       _("The fds are fixed and cannot be reordered"));
>>>> +        goto error;
>>>> +    }
>>> Okay, so you are enforcing that a caller can't pick two conflicting fd
>>> passing methods on the same virCommand, but my point remains that the
>>> testsuite should prove we can hit this error case, to avoid regressions.
>>>    Or we should redo virCommand to ALWAYS rearrange fds in the child, and
>>> make it possible for a caller passing an fd down to know what fd the
>>> child will see it as.
>> That's the part I don't quite follow. We are passing indices via
>> /dev/fdset/%d, right? At the time we build up the command line, we need
>> to know the index that the fd will have. If we shuffle things around
>> later in a way that we cannot determine at the point we build up the
>> command line, how would this work then?
> When you register an fd with virCommand, you would know at that point
> what fd the child will see (regardless of what the fd number was in the
> parent); ideally, the first fd you register will be fd 3 in the guest,
> the next one will be fd 4 in the guest. I'm further proposing that we
> number our /dev/fdset/%n after the first fd we place in the set (that
> is, we know the guest will see fd 3, so place it in /dev/fdset/3).  The
> only time an fdset needs more than one member is if we pass multiple fds
> pointing to the same file (a read-only fd and a read-write fd, for
> example), but we don't have a use for that yet.
>
>> We are not passing all file descriptors to the child. So how would you
>> handle passing fd's 10,3,55 to the child? Do we pass empty slots for 4-9
>> and 11-54 then, like pass fd = -1, so we can pass indices 10,3, and 55
>> or something like that?
> In my proposal, the parent's 10, 3, 55 become the child's 3, 4, 5.  The
> current code has the parent's 10 become the child's 10, with closed fds
> in between, except when using the newer code that packs things and
> merely passes an environment variable to say how many were packed.  I'm
> proposing that we just always pack things, and make it easier to know
> what number we packed into, and that the parent then build the command
> line around the packed number instead of around the parent's number.
>
>

I see. I guess a modified virCommandFDSet may then look like this:

/*
  * virCommandFDSet:
  * @fd: FD to be put into @set
  * @set: the set
  * @set_size: actual size of @set
  *
  * This is practically generalized implementation
  * of FD_SET() as we do not want to be limited
  * by FD_SETSIZE.
  *
  * Returns: the target filedescriptor on success,
  *          -1 on usage error,
  *          -ENOMEM on OOM      // -> no returns negative errno to not 
collide with file descriptor
  */
static int
virCommandFDSet(virCommandPtr cmd,
                 int fd,
                 unsigned int flags)
{
     if (!cmd || fd < 0)
         return -1;

     if (virCommandFDIsSet(cmd, fd))
         return 0;

     if (VIR_EXPAND_N(cmd->passfd, cmd->npassfd, 1) < 0)
         return -ENOMEM;

     cmd->passfd[cmd->npassfd - 1].fd = fd;
     cmd->passfd[cmd->npassfd - 1].flags = flags;

     return cmd->npassfd - 1 + 3;  // the filedescriptor given to the 
child; base is '3'
}

virCommandReorderFDs is not used on WIN32 -- that would then mean that 
the fdsets could not be used on WIN32 or a different return value would 
have to be given here. Like

return cmd->npassfd -1

on WIN32 ?


    Stefan