<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I have an alternative patch which does this, I'll submit that instead.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Simon</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Michal Prívozník <mprivozn@redhat.com><br>
<b>Sent:</b> 20 August 2021 16:09<br>
<b>To:</b> Simon Rowe <simon.rowe@nutanix.com>; libvir-list@redhat.com <libvir-list@redhat.com><br>
<b>Subject:</b> Re: [PATCH 2/2] qemu: never unlink() the core dump output file</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On 8/20/21 10:39 AM, Simon Rowe wrote:<br>
> The comment above virQEMUFileOpenAs() implies any result should be<br>
> left intact.<br>
> <br>
> Signed-off-by: Simon Rowe <simon.rowe@nutanix.com><br>
> ---<br>
>  src/qemu/qemu_driver.c | 2 --<br>
>  1 file changed, 2 deletions(-)<br>
> <br>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c<br>
> index f31e13889e..b1ac1cb73b 100644<br>
> --- a/src/qemu/qemu_driver.c<br>
> +++ b/src/qemu/qemu_driver.c<br>
> @@ -3282,8 +3282,6 @@ doCoreDump(virQEMUDriver *driver,<br>
>      if (qemuDomainFileWrapperFDClose(vm, wrapperFd) < 0)<br>
>          ret = -1;<br>
>      virFileWrapperFdFree(wrapperFd);<br>
> -    if (ret != 0)<br>
> -        unlink(path);<br>
>      return ret;<br>
>  }<br>
>  <br>
> <br>
<br>
So the @path is opened using virQEMUFileOpenAs() which as the last<br>
argument has @needUnlink which tells caller whether the file was created<br>
by this call (true) or was already existing (false). But for some reason<br>
doCoreDump() passes NULL and then unlinks the file unconditionally. I<br>
think the proper fix would be to introduce a boolean, pass it instead of<br>
NULL and then have:<br>
<br>
 if (ret != 0 && needUnlink)<br>
   unlink(path);<br>
<br>
Michal<br>
<br>
</div>
</span></font></div>
</body>
</html>