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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list