4:25 a.m.
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
---
src/qemu/qemu_monitor.c | 34 +++++++++++++++++++++++++---------
src/qemu/qemu_monitor.h | 3 ++-
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e593642..4c6c66f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1552,7 +1552,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon,
ret = qemuMonitorTextMigrate(mon, flags, "fd:migrate");
if (ret < 0) {
- if (qemuMonitorCloseFileHandle(mon, "migrate") < 0)
+ if (qemuMonitorCloseFileHandle(mon, "migrate", true) < 0)
VIR_WARN("failed to close migration handle");
}
@@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
- const char *fdname)
+ const char *fdname,
+ bool preserveError)
{
- int ret;
+ int ret = -1;
+ virErrorPtr error = NULL;
+
VIR_DEBUG("mon=%p fdname=%s",
mon, fdname);
+ if (preserveError)
+ error = virSaveLastError();
+
if (!mon) {
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("monitor must not be NULL"));
- return -1;
+ goto cleanup;
}
if (mon->json)
ret = qemuMonitorJSONCloseFileHandle(mon, fdname);
else
ret = qemuMonitorTextCloseFileHandle(mon, fdname);
+
+cleanup:
+ if (error) {
+ virSetError(error);
+ virFreeError(error);
+ }
return ret;
}
@@ -2014,9 +2026,11 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon,
cleanup:
if (ret < 0) {
- if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0)
+ if (tapfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, tapfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'",
tapfd_name);
- if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) <
0)
+ if (vhostfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, vhostfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'",
vhostfd_name);
}
@@ -2078,9 +2092,11 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
cleanup:
if (ret < 0) {
- if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0)
+ if (tapfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, tapfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'",
tapfd_name);
- if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) <
0)
+ if (vhostfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, vhostfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'",
vhostfd_name);
}
@@ -2258,7 +2274,7 @@ int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon,
ret = qemuMonitorTextAddDevice(mon, devicestr);
if (ret < 0 && fd >= 0) {
- if (qemuMonitorCloseFileHandle(mon, fdname) < 0)
+ if (qemuMonitorCloseFileHandle(mon, fdname, true) < 0)
VIR_WARN("failed to close device handle '%s'", fdname);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 893f3e9..71ee932 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -363,7 +363,8 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int fd);
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
- const char *fdname);
+ const char *fdname,
+ bool preserveError);
/* XXX do we really want to hardcode 'netstr' as the
--
1.7.6
11:09 a.m.
New subject: [libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths
On 07/13/2011 03:25 AM, Jiri Denemark wrote:
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
---
src/qemu/qemu_monitor.c | 34 +++++++++++++++++++++++++---------
src/qemu/qemu_monitor.h | 3 ++-
2 files changed, 27 insertions(+), 10 deletions(-)
@@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
- const char *fdname)
+ const char *fdname,
+ bool preserveError)
Why the bool argument? Every one of the callers was adjusted to set it
to true, so if no one sets it to false, it seems like it makes more
sense to blindly preserveError instead of make it parameterizable,
without having to tweak any callers.
{
- int ret;
+ int ret = -1;
+ virErrorPtr error = NULL;
+
VIR_DEBUG("mon=%p fdname=%s",
mon, fdname);
+ if (preserveError)
+ error = virSaveLastError();
+
if (!mon) {
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("monitor must not be NULL"));
- return -1;
+ goto cleanup;
}
if (mon->json)
ret = qemuMonitorJSONCloseFileHandle(mon, fdname);
else
ret = qemuMonitorTextCloseFileHandle(mon, fdname);
+
+cleanup:
+ if (error) {
+ virSetError(error);
+ virFreeError(error);
+ }
return ret;
}
ACK to the concept, though.
If you have a future patch that will pass false, then this looks okay,
otherwise, it's probably worth a simpler v2.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:45 a.m.
New subject: [libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths
On Wed, Jul 13, 2011 at 10:09:21 -0600, Eric Blake wrote:
On 07/13/2011 03:25 AM, Jiri Denemark wrote:
> When qemuMonitorCloseFileHandle is called in error path, we need to
> preserve the original error since a possible further error when running
> closefd monitor command is not very useful to users.
> ---
> src/qemu/qemu_monitor.c | 34 +++++++++++++++++++++++++---------
> src/qemu/qemu_monitor.h | 3 ++-
> 2 files changed, 27 insertions(+), 10 deletions(-)
>
>
> @@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
>
>
> int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
> - const char *fdname)
> + const char *fdname,
> + bool preserveError)
Why the bool argument? Every one of the callers was adjusted to set it
to true, so if no one sets it to false, it seems like it makes more
sense to blindly preserveError instead of make it parameterizable,
without having to tweak any callers.
I guess I just wanted to stress that qemuMonitorCloseFileHandle preserves the
error :-) The simpler v2 follows...
Jirka
7:50 a.m.
New subject: [libvirt] [PATCH v2] qemu: Don't overwrite errors by closefd in error paths
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
---
src/qemu/qemu_monitor.c | 14 ++++++++++++--
src/qemu/qemu_monitor.h | 3 +++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cff7220..3a30a15 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1964,20 +1964,30 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
const char *fdname)
{
- int ret;
+ int ret = -1;
+ virErrorPtr error;
+
VIR_DEBUG("mon=%p fdname=%s",
mon, fdname);
+ error = virSaveLastError();
+
if (!mon) {
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("monitor must not be NULL"));
- return -1;
+ goto cleanup;
}
if (mon->json)
ret = qemuMonitorJSONCloseFileHandle(mon, fdname);
else
ret = qemuMonitorTextCloseFileHandle(mon, fdname);
+
+cleanup:
+ if (error) {
+ virSetError(error);
+ virFreeError(error);
+ }
return ret;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 893f3e9..f246d21 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -362,6 +362,9 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
const char *fdname,
int fd);
+/* The function preserves previous error and only sets it's own error if no
+ * error was set before.
+ */
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
const char *fdname);
--
1.7.6
8:02 a.m.
New subject: [libvirt] [PATCH v2] qemu: Don't overwrite errors by closefd in error paths
On 07/14/2011 06:50 AM, Jiri Denemark wrote:
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
---
src/qemu/qemu_monitor.c | 14 ++++++++++++--
src/qemu/qemu_monitor.h | 3 +++
2 files changed, 15 insertions(+), 2 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:13 a.m.
New subject: [libvirt] [PATCH v2] qemu: Don't overwrite errors by closefd in error paths
On Thu, Jul 14, 2011 at 07:02:57 -0600, Eric Blake wrote:
On 07/14/2011 06:50 AM, Jiri Denemark wrote:
> When qemuMonitorCloseFileHandle is called in error path, we need to
> preserve the original error since a possible further error when running
> closefd monitor command is not very useful to users.
> ---
> src/qemu/qemu_monitor.c | 14 ++++++++++++--
> src/qemu/qemu_monitor.h | 3 +++
> 2 files changed, 15 insertions(+), 2 deletions(-)
ACK.
Pushed, thanks.
Jirka
4881
days inactive
4882
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
Jiri Denemark