[libvirt] [PATCH v2] qemu: Move last error save/restore to qemuBuildNetCommandLine
Commit 'ef2ab8fd' moved just the virDomainConfNWFilterTeardown and left the logic to save/restore the current error essentially doing nothing in the error path for qemuBuildCommandLine. So move it to where it was meant to be. Although the original code would reset the filter on command creation errors after building the network command portion and commit 'ef2ab8fd' altered that logic, the teardown is called during qemuProcessStop from virDomainConfVMNWFilterTeardown and that code has the save/restore last error logic, so just allow that code to handle the teardown rather than running it twice. The qemuProcessStop would be called in the failure path of qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2016-March/msg00566.html Changes - just move the save/restore error logic from mainline to the BuildNetCommandLine function where it's used. src/qemu/qemu_command.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b18e425..954f802 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8077,8 +8077,13 @@ qemuBuildNetCommandLine(virCommandPtr cmd, return 0; error: + /* free up any resources in the network driver + * but don't overwrite the original error */ + originalError = virSaveLastError(); for (i = 0; last_good_net != -1 && i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); + virSetError(originalError); + virFreeError(originalError); return -1; } @@ -9407,11 +9412,6 @@ qemuBuildCommandLine(virConnectPtr conn, error: virObjectUnref(cfg); - /* free up any resources in the network driver - * but don't overwrite the original error */ - originalError = virSaveLastError(); - virSetError(originalError); - virFreeError(originalError); virCommandFree(cmd); return NULL; } -- 2.5.0
On 03/15/2016 07:36 AM, John Ferlan wrote:
Commit 'ef2ab8fd' moved just the virDomainConfNWFilterTeardown and left the logic to save/restore the current error essentially doing nothing in the error path for qemuBuildCommandLine. So move it to where it was meant to be.
Although the original code would reset the filter on command creation errors after building the network command portion and commit 'ef2ab8fd' altered that logic, the teardown is called during qemuProcessStop from virDomainConfVMNWFilterTeardown and that code has the save/restore last error logic, so just allow that code to handle the teardown rather than running it twice. The qemuProcessStop would be called in the failure path of qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2016-March/msg00566.html
Changes - just move the save/restore error logic from mainline to the BuildNetCommandLine function where it's used.
src/qemu/qemu_command.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b18e425..954f802 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8077,8 +8077,13 @@ qemuBuildNetCommandLine(virCommandPtr cmd, return 0;
error: + /* free up any resources in the network driver + * but don't overwrite the original error */ + originalError = virSaveLastError(); for (i = 0; last_good_net != -1 && i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); + virSetError(originalError); + virFreeError(originalError); return -1; }
@@ -9407,11 +9412,6 @@ qemuBuildCommandLine(virConnectPtr conn,
error: virObjectUnref(cfg); - /* free up any resources in the network driver - * but don't overwrite the original error */ - originalError = virSaveLastError(); - virSetError(originalError); - virFreeError(originalError); virCommandFree(cmd); return NULL; }
<sigh> forgot to save/merge my necessary local branch change: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 954f802..3e7f1fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8024,6 +8024,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd, { size_t i; int last_good_net = -1; + virErrorPtr originalError = NULL; if (!def->nnets) { /* If we have -device, then we set -nodefault already */ @@ -9176,7 +9177,6 @@ qemuBuildCommandLine(virConnectPtr conn, const char *domainLibDir, const char *domainChannelTargetDir) { - virErrorPtr originalError = NULL; size_t i; char uuid[VIR_UUID_STRING_BUFLEN]; virCommandPtr cmd = NULL;
On Tue, Mar 15, 2016 at 07:40:01AM -0400, John Ferlan wrote:
On 03/15/2016 07:36 AM, John Ferlan wrote:
Commit 'ef2ab8fd' moved just the virDomainConfNWFilterTeardown and left the logic to save/restore the current error essentially doing nothing in the error path for qemuBuildCommandLine. So move it to where it was meant to be.
Although the original code would reset the filter on command creation errors after building the network command portion and commit 'ef2ab8fd' altered that logic, the teardown is called during qemuProcessStop from virDomainConfVMNWFilterTeardown and that code has the save/restore last error logic, so just allow that code to handle the teardown rather than running it twice. The qemuProcessStop would be called in the failure path of qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2016-March/msg00566.html
Changes - just move the save/restore error logic from mainline to the BuildNetCommandLine function where it's used.
src/qemu/qemu_command.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
ACK with the additional change it. Martin
On Tue, Mar 15, 2016 at 07:36:44AM -0400, John Ferlan wrote:
Commit 'ef2ab8fd' moved just the virDomainConfNWFilterTeardown and left the logic to save/restore the current error essentially doing nothing in the error path for qemuBuildCommandLine. So move it to where it was meant to be.
Although the original code would reset the filter on command creation errors after building the network command portion and commit 'ef2ab8fd' altered that logic, the teardown is called during qemuProcessStop from virDomainConfVMNWFilterTeardown and that code has the save/restore last error logic, so just allow that code to handle the teardown rather than running it twice. The qemuProcessStop would be called in the failure path of qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2016-March/msg00566.html
Changes - just move the save/restore error logic from mainline to the BuildNetCommandLine function where it's used.
That sounds fine, but you should also move the originalError, otherwise how would this compile?
src/qemu/qemu_command.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b18e425..954f802 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8077,8 +8077,13 @@ qemuBuildNetCommandLine(virCommandPtr cmd, return 0;
error: + /* free up any resources in the network driver + * but don't overwrite the original error */ + originalError = virSaveLastError(); for (i = 0; last_good_net != -1 && i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); + virSetError(originalError); + virFreeError(originalError); return -1; }
@@ -9407,11 +9412,6 @@ qemuBuildCommandLine(virConnectPtr conn,
error: virObjectUnref(cfg); - /* free up any resources in the network driver - * but don't overwrite the original error */ - originalError = virSaveLastError(); - virSetError(originalError); - virFreeError(originalError); virCommandFree(cmd); return NULL; } -- 2.5.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
John Ferlan -
Martin Kletzander