[libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110713/94633d5f/attachment-0001.sig>