Re: [Qemu-devel] [PATCH v3 0/5] file descriptor passing using pass-fd

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]

On 06/19/2012 09:57 AM, Kevin Wolf wrote:

>> this new fd-passing approach, a file originally opened as O_RDONLY
>> /dev/fd/21 will need to be reopened, but the reopened fd will (likely)
>> not be 21.  In other words, we need to make sure 'block-commit' supports
>> the ability to pass in optional arguments that specify the file name of
>> the backing file to be reopened,

> 
> Adding an extra argument to each command that reopens (as in
> bdrv_reopen(), i.e. changes flags) internally is one option. In my
> opinion not a particularly nice one, though.

Agreed, as that's a lot of work for a lot of commands.

> 
> Maybe it's better to have a monitor command that just prepares a reopen
> and means "for the next reopen of /dev/fd/42, the passed FD will have
> the right flags (if it hasn't, the reopen will fail)". We can use dup2()
> to keep the "name" stable.

Indeed, having one additional up-front command in the pass-fd/closefd
family might make this easier.  But how would it work reliably?
Remember, the current proposal is:

libvirt opens backing file O_RDONLY, and calls 'pass-fd name'
qemu returns 21
libvirt tells qemu to hotplug a drive with /dev/fd/21 as backing file
qemu dup()s 21, and proceeds to use fd 22 for all its real work
libvirt calls 'closefd name', to avoid the leak on fd 21

sometime later...
qemu has opened something else that now occupies fd 21
libvirt wants to call 'block-commit', and knows qemu now needs O_RDWR
access to the file
under your idea, that would mean libvirt would call something like
'pass-reopen-fd name /dev/fd/21', and qemu would get a new fd (let's
assume 23), so that qemu would now know that fd 23 should be used the
next time any qemu interface wants to reopen '/dev/fd/21'

But that's not safe - there could easily have been more than one
'pass-fd' all resulting in 21, as long as each was separated by
'closefd' in the meantime; and since libvirt already called 'closefd' to
avoid an indefinite fd leak, qemu is tracking neither /dev/fd/21 nor
'name' in its passfd list.  That is, the only place tracking
'/dev/fd/21' is the block driver where we used /dev/fd to pass in the
O_RDONLY fd in the first place.  Qemu can't dup2() to move 23 into 21 at
the time of the 'pass-reopen-fd', as that might not be safe.

Unless you have any bright ideas that I'm overlooking, I don't see how
an additional 'pass-reopen-fd' can be made to do what we want.  I'm
afraid that the only way to make a reopen operation reliable is to be
able to specify a new file name to pass in a new /dev/fd/nnn to use as
part of any reopen operation, which means touching every command like
'block-commit' that needs to do a reopen.

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




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]