[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGR
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS |
Date: |
Thu, 2 Aug 2012 11:54:11 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Aug 01, 2012 at 10:02:37PM -0300, Luiz Capitulino wrote:
> This error is currently returned by inet_connect_opts(), however
> it causes the follow spurious message on HMP:
>
> (qemu) migrate tcp:0:4444
> migrate: Connection can not be completed immediately
> (qemu)
>
> But migration succeeds.
I think the core issue is that inet_connect_opts() passes back the
QERR_SOCKET_CONNECT_IN_PROGRESS via Error (which is fine), but that
we have users that erroneous pass this error up the stack, when really,
when specifying blocking=on as one of the options, they should be
expecting and doing specific handling for this error.
So if we fix that (by simply using a local Error when doing the call and
using error_propagate() for non QSCIP errors), I think we can basically
drop patches 14-17 by fixing the callers in that manner and just giving QSCIP
it's own error class.
Relying on the errno result was something these socket errors were
specifically meant to fix, since errno is set multiple times
throughout the function and extracting an errno reliably requires
callers to examine all the possible error paths and errno setters. So I
think it's a regression to go back to the old behavior, and these were
issues found in inet_connect() when we attempted to generalize it's
usage for non-blocking connections.
>
> inet_connect_opts() has a 'in_progress' argument that callers can
> use to check whether a connection is in progress. The QERR_ macro
> is not needed anymore.
>
> PS: I didn't test with QMP, but I guess the migrate command will
> return an error response.
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
> qemu-sockets.c | 2 --
> qerror.c | 4 ----
> qerror.h | 3 ---
> 3 files changed, 9 deletions(-)
>
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 82f4736..7196c5f 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress,
> Error **errp)
> if (in_progress) {
> *in_progress = true;
> }
> -
> - error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
> } else if (rc < 0) {
> if (NULL == e->ai_next)
> fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n",
> __FUNCTION__,
> diff --git a/qerror.c b/qerror.c
> index 691d8a8..33b8780 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -309,10 +309,6 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Could not start VNC server on %(target)",
> },
> {
> - .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> - .desc = "Connection can not be completed immediately",
> - },
> - {
> .error_fmt = QERR_SOCKET_CONNECT_FAILED,
> .desc = "Failed to connect to socket",
> },
> diff --git a/qerror.h b/qerror.h
> index de8497d..52ce58d 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -240,9 +240,6 @@ char *qerror_format(const char *fmt, QDict *error);
> #define QERR_VNC_SERVER_FAILED \
> "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
>
> -#define QERR_SOCKET_CONNECT_IN_PROGRESS \
> - "{ 'class': 'SockConnectInprogress', 'data': {} }"
> -
> #define QERR_SOCKET_CONNECT_FAILED \
> "{ 'class': 'SockConnectFailed', 'data': {} }"
>
> --
> 1.7.11.2.249.g31c7954.dirty
>
- Re: [Qemu-devel] [PATCH 13/34] hmp: hmp_change(): don't rely on QERR_DEVICE_ENCRYPTED, (continued)
- [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Luiz Capitulino, 2012/08/01
- Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Luiz Capitulino, 2012/08/02
- Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Markus Armbruster, 2012/08/02
- Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Luiz Capitulino, 2012/08/02
- Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Amos Kong, 2012/08/06
- Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno, Luiz Capitulino, 2012/08/06
[Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 16/34] migration: don't rely on QERR_SOCKET_*, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 09/34] qerror: don't delay error message construction, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 18/34] error: drop unused functions, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 21/34] qapi: qapi-types.h: don't include qapi/qapi-types-core.h, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 19/34] block: block_int: include qerror.h, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 20/34] hmp: hmp.h: include qdict.h, Luiz Capitulino, 2012/08/01
[Qemu-devel] [PATCH 22/34] qapi: generate correct enum names for camel case enums, Luiz Capitulino, 2012/08/01