[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds |
Date: |
Thu, 20 Sep 2012 10:47:18 -0300 |
On Thu, 20 Sep 2012 10:09:54 +0200
Paolo Bonzini <address@hidden> wrote:
> Il 19/09/2012 22:42, Luiz Capitulino ha scritto:
> > On Wed, 19 Sep 2012 16:31:04 +0200
> > Paolo Bonzini <address@hidden> wrote:
> >
> >> monitor_handle_fd_param and monitor_get_fd are mostly the same, except
> >> that monitor_handle_fd_param does error reporting wrong. Use it in all
> >> other places that do it wrong, instead of reinventing it.
> >
> > Hmm, why do we want to do this?
> >
> > As far as I understand it the main difference between the two functions
> > is that if fdname is a number (for a weak definition of number),
> > monitor_handle_fd_param() assumes that the fd already exists in qemu
> > (eg. it was passed by the parent process).
>
> Oops, right, I remembered it was the other way round (i.e. first search
> for a named file descriptor, and fall back to a numeric one).
That's a lot better, IMO.
> > Another side effect is that you add the possibility of functions
> > changing from monitor_get_fd() to monitor_handle_fd_param() to also take
> > fds passed by the parent process. Might be positive, but I wonder if that's
> > useful for the commands you're changing.
>
> Making behavior more uniform may be a useful thing on its own. We might
> even consider moving it to monitor_get_fd (with the above tweak for
> compatibility).
Yes, although I can think of another issue: suppose an mngt app passes an
fd with fdname=9, but the fd doesn't reach qemu for some reason. Then the
mngt app executes a qmp command with fdname=9 and fd 9 turns out to exist
in qemu... Actually, a mngt app could do this even w/o passing an fd to
qemu.
Not sure how relevant this issue is though, as this can happen today
with qmp commands using monitor_handle_fd_param().
> BTW, pci-assign.c is open-coding
> monitor_handle_fd_param (including the numeric file descriptor behavior)
> and we can remove the surrounding if.
Yes, that's fine.
[Qemu-devel] [PATCH 03/12] qapi: do not protect enum values from namespace pollution, Paolo Bonzini, 2012/09/19
[Qemu-devel] [PATCH 02/12] monitor: add Error * argument to monitor_get_fd, Paolo Bonzini, 2012/09/19
[Qemu-devel] [PATCH 05/12] build: add QAPI files to the tools, Paolo Bonzini, 2012/09/19
[Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words, Paolo Bonzini, 2012/09/19