[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: |
Paolo Bonzini |
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:09:54 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120828 Thunderbird/15.0 |
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).
> 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). BTW, pci-assign.c is open-coding
monitor_handle_fd_param (including the numeric file descriptor behavior)
and we can remove the surrounding if.
Paolo
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> hw/kvm/pci-assign.c | 4 +---
>> migration-fd.c | 3 +--
>> monitor.c | 6 +++---
>> 3 file modificati, 5 inserzioni(+), 8 rimozioni(-)
>>
>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
>> index 05b93d9..8b96a57 100644
>> --- a/hw/kvm/pci-assign.c
>> +++ b/hw/kvm/pci-assign.c
>> @@ -582,10 +582,8 @@ static int get_real_device(AssignedDevice *pci_dev,
>> uint16_t r_seg,
>> if (qemu_isdigit(pci_dev->configfd_name[0])) {
>> dev->config_fd = strtol(pci_dev->configfd_name, NULL, 0);
>> } else {
>> - dev->config_fd = monitor_get_fd(cur_mon,
>> pci_dev->configfd_name);
>> + dev->config_fd = monitor_handle_fd_param(cur_mon,
>> pci_dev->configfd_name);
>> if (dev->config_fd < 0) {
>> - error_report("%s: (%s) unkown", __func__,
>> - pci_dev->configfd_name);
>> return 1;
>> }
>> }
>> diff --git a/migration-fd.c b/migration-fd.c
>> index 50138ed..3eb53d9 100644
>> --- a/migration-fd.c
>> +++ b/migration-fd.c
>> @@ -75,9 +75,8 @@ static int fd_close(MigrationState *s)
>>
>> int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
>> {
>> - s->fd = monitor_get_fd(cur_mon, fdname);
>> + s->fd = monitor_handle_fd_param(cur_mon, fdname);
>> if (s->fd == -1) {
>> - DPRINTF("fd_migration: invalid file descriptor identifier\n");
>> goto err_after_get_fd;
>> }
>>
>> diff --git a/monitor.c b/monitor.c
>> index 67064e2..4901600 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -951,7 +951,7 @@ static int add_graphics_client(Monitor *mon, const QDict
>> *qdict, QObject **ret_d
>> CharDriverState *s;
>>
>> if (strcmp(protocol, "spice") == 0) {
>> - int fd = monitor_get_fd(mon, fdname);
>> + int fd = monitor_handle_fd_param(mon, fdname);
>> int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
>> int tls = qdict_get_try_bool(qdict, "tls", 0);
>> if (!using_spice) {
>> @@ -965,13 +965,13 @@ static int add_graphics_client(Monitor *mon, const
>> QDict *qdict, QObject **ret_d
>> return 0;
>> #ifdef CONFIG_VNC
>> } else if (strcmp(protocol, "vnc") == 0) {
>> - int fd = monitor_get_fd(mon, fdname);
>> + int fd = monitor_handle_fd_param(mon, fdname);
>> int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
>> vnc_display_add_client(NULL, fd, skipauth);
>> return 0;
>> #endif
>> } else if ((s = qemu_chr_find(protocol)) != NULL) {
>> - int fd = monitor_get_fd(mon, fdname);
>> + int fd = monitor_handle_fd_param(mon, fdname);
>> if (qemu_chr_add_client(s, fd) < 0) {
>> qerror_report(QERR_ADD_CLIENT_FAILED);
>> return -1;
>
[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