[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets |
Date: |
Wed, 17 Oct 2012 15:44:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121009 Thunderbird/16.0 |
Il 17/10/2012 15:33, Markus Armbruster ha scritto:
> Paolo Bonzini <address@hidden> writes:
>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> v1->v2: fixed connect_state memory leaks
>>
>> qemu-char.c | 2 +-
>> qemu-sockets.c | 77
>> ++++++++++++++++++++++++++++++++++++++++++++++++----------
>> qemu_socket.h | 6 ++++-
>> 3 file modificati, 70 inserzioni(+), 15 rimozioni(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 3cc6cb5..8ebd582 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts
>> *opts)
>> if (is_listen) {
>> fd = unix_listen_opts(opts, NULL);
>> } else {
>> - fd = unix_connect_opts(opts, NULL);
>> + fd = unix_connect_opts(opts, NULL, NULL, NULL);
>> }
>> } else {
>> if (is_listen) {
>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>> index f7e67b6..6a49429 100644
>> --- a/qemu-sockets.c
>> +++ b/qemu-sockets.c
>> @@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque)
>> }
>>
>> /* try to connect to the next address on the list */
>> - while (s->current_addr->ai_next != NULL && s->fd < 0) {
>> - s->current_addr = s->current_addr->ai_next;
>> - s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
>> - /* connect in progress */
>> - if (in_progress) {
>> - return;
>> + if (s->current_addr) {
>> + while (s->current_addr->ai_next != NULL && s->fd < 0) {
>> + s->current_addr = s->current_addr->ai_next;
>> + s->fd = inet_connect_addr(s->current_addr, &in_progress, s);
>> + /* connect in progress */
>> + if (in_progress) {
>> + return;
>> + }
>> }
>> +
>> + freeaddrinfo(s->addr_list);
>> }
>>
>> - freeaddrinfo(s->addr_list);
>> if (s->callback) {
>> s->callback(s->fd, s->opaque);
>> }
>> @@ -701,11 +704,13 @@ err:
>> return -1;
>> }
>>
>> -int unix_connect_opts(QemuOpts *opts, Error **errp)
>> +int unix_connect_opts(QemuOpts *opts, Error **errp,
>> + NonBlockingConnectHandler *callback, void *opaque)
>> {
>> struct sockaddr_un un;
>> const char *path = qemu_opt_get(opts, "path");
>> - int sock;
>> + ConnectState *connect_state = NULL;
>> + int sock, rc;
>>
>> if (NULL == path) {
>> fprintf(stderr, "unix connect: no path specified\n");
>> @@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp)
>> perror("socket(unix)");
>> return -1;
>> }
>> + if (callback != NULL) {
>> + connect_state = g_malloc0(sizeof(*connect_state));
>> + connect_state->callback = callback;
>> + connect_state->opaque = opaque;
>> + socket_set_nonblock(sock);
>> + }
>>
>> memset(&un, 0, sizeof(un));
>> un.sun_family = AF_UNIX;
>> snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
>> - if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
>> +
>> + /* connect to peer */
>> + do {
>> + rc = 0;
>> + if (connect(sock, (struct sockaddr *) &un, sizeof(un)) < 0) {
>> + rc = -socket_error();
>> + }
>> + } while (rc == -EINTR);
>
> Isn't this a silent EINTR bug fix?
>
>> +
>> + if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
>> + connect_state->fd = sock;
>> + qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
>> + connect_state);
>> + return sock;
>> + } else {
>> + /* non blocking socket immediate success, call callback */
>> + if (callback != NULL) {
>> + callback(sock, opaque);
>> + }
>> + }
>> +
>> + if (rc < 0) {
>> fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno));
>> close(sock);
>> - return -1;
>> + sock = -1;
>> }
>>
>> + g_free(connect_state);
>> return sock;
>> }
>>
>
> Unlike inet_connect_opts(), this one runs callback() even when connect()
> fails. Are you sure that's what you want?
I think it's harmless, but it's worth harmonizing it.
Paolo
- [Qemu-devel] [PATCH 05/25] migration: avoid using error_is_set and thus relying on errp != NULL, (continued)
- [Qemu-devel] [PATCH 02/25] qemu-sockets: add Error ** to all functions, Paolo Bonzini, 2012/10/10
- [Qemu-devel] [PATCH 07/25] migration: use qemu-sockets to establish Unix sockets, Paolo Bonzini, 2012/10/10
- [Qemu-devel] [PATCH 06/25] migration: centralize call to migrate_fd_error(), Paolo Bonzini, 2012/10/10
- [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets, Paolo Bonzini, 2012/10/10
- [Qemu-devel] [PATCH 09/25] migration (incoming): add error propagation to fd and exec protocols, Paolo Bonzini, 2012/10/10
- [Qemu-devel] [PATCH 08/25] migration (outgoing): add error propagation for all protocols, Paolo Bonzini, 2012/10/10
[Qemu-devel] [PATCH 12/25] qemu-ga: ask and print error information from qemu-sockets, Paolo Bonzini, 2012/10/10
[Qemu-devel] [PATCH 13/25] vnc: add error propagation to vnc_display_open, Paolo Bonzini, 2012/10/10