[libvirt] [PATCH] qemu: Reset error if we're not going to error label

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_conf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e91967..2a889381090f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -266,8 +266,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) { /* This however is not implemented on all platforms. */ virErrorPtr err = virGetLastError(); - if (err && err->code != VIR_ERR_NO_SUPPORT) - goto error; + if (err) { + if (err->code == VIR_ERR_NO_SUPPORT) + virResetError(err); + else + goto error; } if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0) -- 2.9.2

On Tue, Jul 19, 2016 at 11:00:56AM +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_conf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e91967..2a889381090f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -266,8 +266,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) { /* This however is not implemented on all platforms. */ virErrorPtr err = virGetLastError(); - if (err && err->code != VIR_ERR_NO_SUPPORT) - goto error; + if (err) { + if (err->code == VIR_ERR_NO_SUPPORT) + virResetError(err);
This does not unlog the error. If it's fatal, we should not be ignoring it. If it's not, we should not have logged it in the first place. Jan
+ else + goto error; }
if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0) -- 2.9.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2016-07-19 at 11:00 +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_conf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e91967..2a889381090f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -266,8 +266,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) { /* This however is not implemented on all platforms. */ virErrorPtr err = virGetLastError(); - if (err && err->code != VIR_ERR_NO_SUPPORT) - goto error; + if (err) { + if (err->code == VIR_ERR_NO_SUPPORT) + virResetError(err); + else + goto error;
You forgot the closing brace here.
} if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0)
Would virResetLastError() perhaps be more appropriate? ACK either way. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Jul 20, 2016 at 10:17:35AM +0200, Andrea Bolognani wrote:
On Tue, 2016-07-19 at 11:00 +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_conf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e91967..2a889381090f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -266,8 +266,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0) { /* This however is not implemented on all platforms. */ virErrorPtr err = virGetLastError(); - if (err && err->code != VIR_ERR_NO_SUPPORT) - goto error; + if (err) { + if (err->code == VIR_ERR_NO_SUPPORT) + virResetError(err); + else + goto error;
You forgot the closing brace here.
} if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0)
Would virResetLastError() perhaps be more appropriate?
ACK either way.
I actually agree with Jan here that the error should not be emitted at all if possible. Maybe I'll re-do that if I want at some point in the future. Martin P.S.: Your mail client does ugly things when replying to messages like converting spaces to non-braking spaces and adding all mail addresses into the 'To:' field. Just so you know ;)
-- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Ján Tomko
-
Martin Kletzander