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

Re: [libvirt] [PATCH v2] qemu: Unlink temporary file on failure



On 08/02/2011 09:59 AM, Eric Blake wrote:
On 08/02/2011 09:50 AM, Michal Privoznik wrote:
Although virFDStreamOpenFile will unlink it once opened, when we hit
error path, we must unlink it by hand.
---
src/qemu/qemu_driver.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 09b2791..c78cdb7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2931,7 +2931,10 @@ qemuDomainScreenshot(virDomainPtr dom,

endjob:
VIR_FORCE_CLOSE(tmp_fd);
- VIR_FREE(tmp);
+ if (tmp) {

Wonky spacing.

+ unlink(tmp);
+ VIR_FREE(tmp);
+ }

ACK.

I went ahead and pushed this after fixing the spacing, so that it will be in 0.9.4.

Meanwhile, I wonder if we have a bigger bug - that is, should virFDStreamOpenInternal guarantee that the file is deleted when requested, even on failure paths, so that callers need not do the unlink()? That is, on the success path, we end up unlink()ing the same name twice, which is racy if the name is predictable (in the case of qemuDomainScreenshot, the name is temporary and supposedly safe).

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


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