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