From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53986) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMN32-0000LG-BM for qemu-devel@nongnu.org; Thu, 11 Oct 2012 13:59:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMN2w-0002Bg-3H for qemu-devel@nongnu.org; Thu, 11 Oct 2012 13:59:52 -0400 Received: from e5.ny.us.ibm.com ([32.97.182.145]:36258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMN2v-00026D-Vm for qemu-devel@nongnu.org; Thu, 11 Oct 2012 13:59:46 -0400 Received: from /spool/local by e5.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Oct 2012 13:58:22 -0400 Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q9BHpI0d232322 for ; Thu, 11 Oct 2012 13:52:05 -0400 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q9BHpZoM029582 for ; Thu, 11 Oct 2012 11:51:35 -0600 Message-ID: <507706C0.3060000@linux.vnet.ibm.com> Date: Thu, 11 Oct 2012 13:49:52 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1349878805-16352-1-git-send-email-coreyb@linux.vnet.ibm.com> <1349878805-16352-4-git-send-email-coreyb@linux.vnet.ibm.com> <5075F727.1060608@redhat.com> <5076EDF4.1010301@redhat.com> In-Reply-To: <5076EDF4.1010301@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [libvirt] [PATCH v2 3/3] qemu-config: Add new -add-fd command line option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, libvir-list@redhat.com, qemu-devel@nongnu.org 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