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

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

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.

Corey Bryant

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