Re: [Qemu-devel] [PATCH 3/5] Add getfd and closefd monitor commands

[Avi Kivity]

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.