[libvirt] [PATCH] libxl: report correct errno from virNetSocketNewConnectTCP on migration
saved_errno is never written to in this function after it is initialised and it is only used to log the failure from virNetSocketNewConnectTCP masking the real errno from that function. Drop saved_errno and use errno itself. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/libxl/libxl_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 39e4a65..e291d71 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -480,7 +480,6 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, virURIPtr uri = NULL; virNetSocketPtr sock; int sockfd = -1; - int saved_errno = EINVAL; int ret = -1; /* parse dst host:port from uri */ @@ -496,7 +495,7 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, if (virNetSocketNewConnectTCP(hostname, portstr, AF_UNSPEC, &sock) < 0) { - virReportSystemError(saved_errno, + virReportSystemError(errno, _("unable to connect to '%s:%s'"), hostname, portstr); goto cleanup; -- 2.1.4
On 03.09.2015 12:14, Ian Campbell wrote:
saved_errno is never written to in this function after it is initialised and it is only used to log the failure from virNetSocketNewConnectTCP masking the real errno from that function.
Drop saved_errno and use errno itself.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/libxl/libxl_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 39e4a65..e291d71 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -480,7 +480,6 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, virURIPtr uri = NULL; virNetSocketPtr sock; int sockfd = -1; - int saved_errno = EINVAL; int ret = -1;
/* parse dst host:port from uri */ @@ -496,7 +495,7 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, if (virNetSocketNewConnectTCP(hostname, portstr, AF_UNSPEC, &sock) < 0) { - virReportSystemError(saved_errno, + virReportSystemError(errno, _("unable to connect to '%s:%s'"), hostname, portstr); goto cleanup;
In fact, virNetSocketNewConnectTCP itself returns meaningful message on error. I think this overwriting should be dropped completely. Michal
On 09/03/2015 06:58 AM, Michal Privoznik wrote:
On 03.09.2015 12:14, Ian Campbell wrote:
saved_errno is never written to in this function after it is initialised and it is only used to log the failure from virNetSocketNewConnectTCP masking the real errno from that function.
Drop saved_errno and use errno itself.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/libxl/libxl_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 39e4a65..e291d71 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -480,7 +480,6 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, virURIPtr uri = NULL; virNetSocketPtr sock; int sockfd = -1; - int saved_errno = EINVAL; int ret = -1;
/* parse dst host:port from uri */ @@ -496,7 +495,7 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, if (virNetSocketNewConnectTCP(hostname, portstr, AF_UNSPEC, &sock) < 0) { - virReportSystemError(saved_errno, + virReportSystemError(errno, _("unable to connect to '%s:%s'"), hostname, portstr); goto cleanup;
In fact, virNetSocketNewConnectTCP itself returns meaningful message on error. I think this overwriting should be dropped completely.
Agreed. How about the following patch? Regards, Jim
On 09/03/2015 10:26 AM, Jim Fehlig wrote:
Agreed. How about the following patch?
From a30c493bd9e20c9a7a423789a202c444a5eba344 Mon Sep 17 00:00:00 2001 From: Jim Fehlig<jfehlig@suse.com> Date: Thu, 3 Sep 2015 10:14:20 -0600 Subject: [PATCH] libxl: don't overwrite error from virNetSocketNewConnectTCP()
Remove redundant error reporting libxlDomainMigrationPerform().
FYI, I've changed s/reporting/reporting in/ in the commit message. Regards, Jim
virNetSocketNewConnectTCP() is perfectly capable of reporting sensible errors.
Signed-off-by: Jim Fehlig<jfehlig@suse.com>
On Thu, 2015-09-03 at 10:26 -0600, Jim Fehlig wrote:
From a30c493bd9e20c9a7a423789a202c444a5eba344 Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Thu, 3 Sep 2015 10:14:20 -0600 Subject: [PATCH] libxl: don't overwrite error from virNetSocketNewConnectTCP()
Remove redundant error reporting libxlDomainMigrationPerform(). virNetSocketNewConnectTCP() is perfectly capable of reporting sensible errors.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
FWIW: Acked-by: Ian Campbell <ian.campbell@citrix.com>
On 09/03/2015 10:26 AM, Jim Fehlig wrote:
Agreed. How about the following patch?
From a30c493bd9e20c9a7a423789a202c444a5eba344 Mon Sep 17 00:00:00 2001 From: Jim Fehlig<jfehlig@suse.com> Date: Thu, 3 Sep 2015 10:14:20 -0600 Subject: [PATCH] libxl: don't overwrite error from virNetSocketNewConnectTCP()
Remove redundant error reporting libxlDomainMigrationPerform(). virNetSocketNewConnectTCP() is perfectly capable of reporting sensible errors.
Signed-off-by: Jim Fehlig<jfehlig@suse.com> --- src/libxl/libxl_migration.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 9609e06..de2de91 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -487,12 +487,8 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, /* socket connect to dst host:port */ if (virNetSocketNewConnectTCP(hostname, portstr, AF_UNSPEC, - &sock) < 0) { - virReportSystemError(saved_errno, - _("unable to connect to '%s:%s'"), - hostname, portstr); + &sock) < 0) goto cleanup; - }
I should try compiling before sending patches: libxl/libxl_migration.c:475:9: error: unused variable 'saved_errno' [-Werror=unused-variable] int saved_errno = EINVAL; I posted a working V2: https://www.redhat.com/archives/libvir-list/2015-September/msg00107.html Regards, Jim
participants (3)
-
Ian Campbell -
Jim Fehlig -
Michal Privoznik