On Thu, 20 Sep 2012 10:09:54 +0200 Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 19/09/2012 22:42, Luiz Capitulino ha scritto: > > On Wed, 19 Sep 2012 16:31:04 +0200 > > Paolo Bonzini <pbonz...@redhat.com> 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.