qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Corey Bryant
Subject: Re: [Qemu-devel] [PATCH v2 3/3] qemu-config: Add new -add-fd command line option
Date: Thu, 11 Oct 2012 13:36:55 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1



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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]