[libvirt] [PATCH] qemu: Check for existence of auto-generated socket path before removing

[John Ferlan]

On 09/16/2015 09:05 AM, Michal Privoznik wrote:
> On 15.09.2015 23:03, John Ferlan wrote:
>> Commit id 'f1f68ca33' added code to remove the directory paths for
>> auto-generated sockets, but that code could be called before the
>> paths were created resulting in generating error messages from
>> virFileDeleteTree indicating that the file doesn't exist. So just
>> add a check before attemping the directory delete for existence.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_process.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index ce2c70c..b55eb52 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -5269,13 +5269,13 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>>  
>>      ignore_value(virAsprintf(&tmppath, "%s/domain-%s",
>>                               cfg->libDir, vm->def->name));
>> -    if (tmppath)
>> +    if (tmppath && virFileExists(tmppath))
>>          virFileDeleteTree(tmppath);
>>      VIR_FREE(tmppath);
>>  
>>      ignore_value(virAsprintf(&tmppath, "%s/domain-%s",
>>                               cfg->channelTargetDir, vm->def->name));
>> -    if (tmppath)
>> +    if (tmppath && virFileExists(tmppath))
>>          virFileDeleteTree(tmppath);
>>      VIR_FREE(tmppath);
>>  
>>
> 
> Okay, so this is needed so that we don't taint logs with useless error
> messages. ACK to the idea. But what what about moving virFileExists into
> virFileDeleteTree? The reason for that would be that I like functions
> which deals with dummy arguments. In this case it would be:
> 
> virFileDeleteTree("/some/nonexistent/path");
> virFileDeleteTree(NULL);
> 
> But if you dislike the idea, ACK to this patch then. There's nothing
> wrong with it.
> 
Correct with your assertion - I saw the messages while debugging
something else (error validating something during qemu_command creation):

2015-09-15 14:18:40.197+0000: 28838: error : virFileDeleteTree:945 :
Cannot open dir '/var/lib/libvirt/qemu/domain-test': No such file or
directory
2015-09-15 14:18:40.197+0000: 28838: error : virFileDeleteTree:945 :
Cannot open dir '/var/lib/libvirt/qemu/channel/target/domain-test': No
such file or directory

and that was after the error I was expecting...  Since it was 'new' I
knew it was a recent change...

Anyway, I took the chisel approach rather than the sledgehammer, but I
see your point. Up to this point it seems the function was primarily
called from *test.c modules and those that weren't sure if 'path' could
be NULL prior to call would always check. I'll post a different patch
with the check in virFileDeleteTree() and of course remove the NULL
checks in the *test.c modules.

John