Bug 746075

Summary:	screenshot leaks pipe fd
Product:	Red Hat Enterprise Linux 6	Reporter:	Eric Blake <eblake>
Component:	libvirt	Assignee:	Eric Blake <eblake>
Status:	CLOSED ERRATA	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	6.2	CC:	acathrow, ajia, crobinso, dallan, eblake, marcandre.lureau, mzhan, rwu, weizhan, xen-maint, yupzhang
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	libvirt-0.9.4-18.el6	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:	745761	Environment:
Last Closed:	2011-12-06 11:37:24 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	745761
Bug Blocks:	748554

Description Eric Blake 2011-10-13 20:05:52 UTC

Cloning to RHEL.

+++ This bug was initially created as a clone of Bug #745761 +++

Description of problem:

It seems the screenshot API leaks FDs.

Version-Release number of selected component (if applicable):

v0.9.6-139-g9bc9999 (current git master)

How reproducible:

10/10

Steps to Reproduce:
1. lsof -p `pidof libvirtd`
2. virsh screenshot a-running-vm
3. lsof -p `pidof libvirtd`
  
Actual results:

The number of open files grows, and a new pipe fd is leaked.

Expected results:

The number of open files should remain constant after taking screenshots.

--- Additional comment from marcandre.lureau on 2011-10-13 05:24:04 MDT ---

It seems *errfd = pipeerr[0]; in command.c is not closed.

--- Additional comment from marcandre.lureau on 2011-10-13 05:45:16 MDT ---

I'll send a patch to libvirt ML.

Comment 1 Eric Blake 2011-10-13 20:07:46 UTC

Needs at least these two upstream commits:

commit 72851bb9ef0a6aef20141d0a13c36e5a1907a43a
Author: Marc-André Lureau <marcandre.lureau>
Date:   Thu Oct 13 13:47:58 2011 +0200

    virFDStream: close also given errfd (fd leak)
    
    In virFDStreamOpenFileInternal(), a errfd pipe is opened by
    virCommandRunAsync() and given to virFDStreamOpenInternal().
    
    It seems virFDStream should close errfd, just like the other
    fd it is given.
    
    This fixes screenshots leaking FDs:
    http://bugzilla.redhat.com/show_bug.cgi?id=745761

commit be5ec76630c4af83404e8fbf2b5a6b7cb46656e7
Author: Marc-André Lureau <marcandre.lureau>
Date:   Sat Oct 1 03:39:14 2011 +0200

    Set to NULL members that have been freed to prevent crashes
    
    Do not crash if virStreamFinish is called after error.

Comment 2 Eric Blake 2011-10-13 20:22:14 UTC

In POST:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2011-October/msg00417.html

Comment 6 yuping zhang 2011-10-18 06:12:22 UTC

Reproduce this issue on libvirt-0.9.4-16.el6.x86_64,after make screenshot for a running guest,there is one more open file:
libvirtd 8906 root   37r  FIFO                0,8      0t0  194216788 pipe

Verified this issue on:
libvirt-0.9.4-18.el6.x86_64
libvirt-client-0.9.4-18.el6.x86_64
libvirt-python-0.9.4-18.el6.x86_64

1. # lsof -p `pidof libvirtd` > 746075.1
2. # virsh screenshot rhel62
Screenshot saved to rhel62-2011-10-19-04:18:42.ppm, with type of image/x-portable-pixmap
3. # lsof -p `pidof libvirtd` >746072.2
4. # vimdiff 746075.1 746075.2

The two files are same.
So change the bug status to VERIFIED.

Comment 7 errata-xmlrpc 2011-12-06 11:37:24 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1513.html