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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list