Re: [PATCH 3/3] Wire up virDomainOpenGraphicsFD in QEMU driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/25/2014 12:22 PM, Ján Tomko wrote:
> Should fix https://bugzilla.redhat.com/show_bug.cgi?id=999926
> ---
>  src/qemu/qemu_driver.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ad75bd9..932c638 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15806,6 +15806,85 @@ qemuDomainOpenGraphics(virDomainPtr dom,
>  }
>  
>  static int
> +qemuDomainOpenGraphicsFD(virDomainPtr dom,
> +                         unsigned int idx,
> +                         int *fd,
> +                         unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    int ret = -1;

-1 here...


> +    qemuDomainObjEnterMonitor(driver, vm);
> +    ret = qemuMonitorOpenGraphics(priv->mon, protocol, pair[1], "graphicsfd",
> +                                  (flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) != 0);

slammed into 0 or 1 here, but the contract of the API (prior to my
proposed change) is to return 0 on success, not 1.

> +    qemuDomainObjExitMonitor(driver, vm);
> +    if (!qemuDomainObjEndJob(driver, vm))
> +        vm = NULL;
> +
> +    *fd = pair[0];

And if qemuMonitorOpenGraphics failed, it means we are still assigning a
non-zero fd to *fd,...

> +
> + cleanup:
> +    if (ret < 0) {
> +        VIR_FORCE_CLOSE(pair[0]);
> +        VIR_FORCE_CLOSE(pair[1]);
> +    }

...but failing to close pair[1] (fd leak), and giving the user a useless
fd.  My proposed patch tries to fix that.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]