Re: [Qemu-devel] [libvirt] [PATCH v2 3/3] qemu-config: Add new -add-fd command line option

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 10/11/2012 12:04 PM, Eric Blake wrote:
> On 10/10/2012 04:31 PM, Eric Blake wrote:
>
>> Another missing validation check is for duplicate use.  With the monitor
>> command, you ALWAYS have a unique fd (thanks to SCM_RIGHTS).  But with
>> the command line, I can type 'qemu -add-fd fd=4,set=1 -add-fd
>> fd=4,set=2'.  Oops - I've now corrupted your set layout, unless you
>> validate that every fd requested in -add-fd does not already reside in
>> any existing set.
>>
>> Thinking aloud:
>> On the other hand, being able to pass in one fd to multiple sets MIGHT
>> be useful; in the SCM_RIGHTS monitor command case, I can pass the same
>> fd from the management perspective into multiple sets, even though in
>> qemu's perspective, there will be multiple fds created (one per call).
>> Perhaps instead of directly adding the inherited fd to a set, and having
>> to then sweep all sets to check for duplicates, it might make sense to
>> add dup(fd) to a set, so that if I call:
>>
>> qemu -add-fd fd=4,set=1 -add-fd fd=4,set=2 -add-fd fd=5,set=2
>>
>> what REALLY happens is that qemu adds dup(4)==6 to set 1, dup(4)==7 to
>> set 2, and dup(5)==8 to set 3.  Then, after all ALL -add-fd have been
>> processed, qemu then does another pass through them calling close(4) and
>> close(5) (to avoid holding the original fds open indefinitely if the
>> corresponding sets are discarded).
>
> Another idea: a hybrid approach - the _first_ -add-fd 4 directly adds 4
> to the set, all other -add-fd 4 end up adding dup(4) instead (well,
> fcntl(F_DUPFD_CLOEXEC), but you get the picture).  That is, do the
> duplicate scanning, and if there is no duplicate, use the fd directly;
> if there IS a duplicate, then put a unique fd number as a copy into the
> remaining sets.  That way, you don't have to do a final close() sweep
> across the -add-fd arguments passed on the command line, and you still
> don't have to worry about duplicated fds across multiple sets causing
> mayhem in qemu_close().
>

This would simplify the code, but I wonder if it would be confusing to 
users when they call query-fdsets and only see a single fd 4.  It may 
make more sense to dup all fds that are passed with -add-fd, and then it 
basically works the same as the QMP add-fd via SCM_RIGHTS.

On a somewhat related note, one major difference between the QMP add-fd 
and  command line -add-fd, is that -add-fd doesn't return the fd that 
was added.  So opaque strings will be even more important when passing 
fds on the command line.

-- 
Regards,
Corey Bryant