From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MOdBf-0002zb-Tt for qemu-devel@nongnu.org; Wed, 08 Jul 2009 15:52:15 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MOdBa-0002yk-C9 for qemu-devel@nongnu.org; Wed, 08 Jul 2009 15:52:14 -0400 Received: from [199.232.76.173] (port=53280 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MOdBa-0002ye-9H for qemu-devel@nongnu.org; Wed, 08 Jul 2009 15:52:10 -0400 Received: from mx2.redhat.com ([66.187.237.31]:39322) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MOdBZ-0004Ni-Qa for qemu-devel@nongnu.org; Wed, 08 Jul 2009 15:52:10 -0400 Message-ID: <4A54F8F2.2080103@redhat.com> Date: Wed, 08 Jul 2009 22:52:18 +0300 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 3/5] Add getfd and closefd monitor commands References: <1247064963.3270.63.camel@blaa> <1247065048-15706-1-git-send-email-markmc@redhat.com> <1247065048-15706-2-git-send-email-markmc@redhat.com> <1247065048-15706-3-git-send-email-markmc@redhat.com> <4A54BABD.3040903@redhat.com> <1247069035.3270.82.camel@blaa> <4A54C634.30007@redhat.com> <4A54E0B3.8090305@codemonkey.ws> <4A54E160.9000900@redhat.com> <4A54E3BC.40306@codemonkey.ws> <4A54E64E.8090100@redhat.com> <4A54EA7D.4040602@codemonkey.ws> In-Reply-To: <4A54EA7D.4040602@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Mark McLoughlin , qemu-devel@nongnu.org On 07/08/2009 09:50 PM, Anthony Liguori wrote: > Avi Kivity wrote: >> On 07/08/2009 09:21 PM, Anthony Liguori wrote: >>> Then someone can connect to the monitor and consume an arbitrary >>> number of fds? I'd be very concerned about the potential to leak >>> fds within QEMU from a poorly written client. Seems like a very >>> easy mistake to make. >> >> Well, that's intrinsic to the getfd command. We could limit it by >> saying we support a set number of fds, or even give them fixed names. > > So what's the set number Good question. > and why isn't 1 a reasonable fixed number? > Parallel execution of commands (which I don't propose anytime soon) and commands that accept multiple fds (maybe a command that accepts an irqfd/ioeventfd pair from another guest and exposes the pair to the guest as part of a pci device). > >> If indeed we follow an rpc model, it should be quite easy to have >> multiple threads (each doing an unrelated task) each issuing a series >> of commands and processing the replies. There would be a lock >> protecting the socket, but there would be no reason to limit the >> number of outstanding commands. I think that's a perfectly >> reasonable way to write a client. > > You can't just use a lock on the socket for transmit because you have > to deal with partial transmissions and the subsequent queuing. You > need a single queue mechanism shared by all writers and you need > writers to submit the full rpcs so that they can be queued in the > proper order. > > When we get to the point of the RPC model though, we'll have one > monitor state for each asynchronous message we're processing. I > contend that we only ever want to pass a single fd per command so if > we stick with queueing 1 fd per monitor today, it'll map quite nicely > to how we would handle things in the RPC model. I'm thinking about the client, not qemu, but you're right that the fd and the message that refer to it must be in a single transaction. So at the qemu_chr level, we'll never queue more than one fd. > >> If the client is written in a high level language it's also >> reasonable that some buffering would take place and you'd see a >> single packet containing multiple commands, or a command split into >> multiple packets. Therefore I'd like to avoid any assumptions in >> this area. > > That misses the point though. We process one command at a time in the > monitor so we only need to buffer one fd at a time. When we start to > process multiple commands at once in the monitor, we'll do so with > multiple monitor states and we'll want to have one fd per monitor state. Again I'm thinking of the client. If two client threads issue commands in parallel there'd be >1 fds on the wire. But qemu can consume them sequentially so I agree queue may hold just a single fd. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.