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

[Corey Bryant]

On 10/11/2012 11:55 AM, Eric Blake wrote:
> On 10/11/2012 08:45 AM, Corey Bryant 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.
>>>
>>
>> I don't see this validation check for duplicate use of fd's being
>> necessary.  Like you say below, in the QMP add-fd case we can add the
>> same fd multiple times.  So we should be able to add the same fd
>> multiple times via the command line.  The only difference between QMP
>> and command line in this case is that the QMP fd is a dup and therefore
>> a different number and the command line fd will be the same fd.  I'd
>> prefer to leave this alone unless there's a compelling reason to block
>> adding of the same fd.
>
> There is a compelling reason to prevent duplicates among your sets:
> qemu_close().
>
> Suppose I add fd 4 into set 1 and 2, and then discard set 2 via monitor
> commands.  Then, when qemu_close() drops the last reference to set 2, it
> steps through and calls close() on all fds in that set, including fd 4.
>   Oops - now set 1 is invalid, because it is tracking a closed fd.  And
> worse, if qemu then does something else to open a new fd, it will get fd
> 4 again, and now set 1 will be tracking the WRONG fd.
>

Ah yes, that is compelling.  So we do need something here.  I'll reply 
to your other email regarding the approach to take.

-- 
Regards,
Corey Bryant