[libvirt] [PATCH v2] virfile: Check for existence of dir in virFileDeleteTree

[Martin Kletzander]

On Wed, Sep 16, 2015 at 10:01:46AM -0400, 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.
>
>Rather than "enforce" all callers to make the non-NULL and existence
>checks, modify the virFileDeleteTree API to silently ignore NULL on
>input and non-existent directory trees.
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
>
>Difference over v1: Add checks in virFileDeleteTree
>

I like this version better as well, and even more when you fixed all
the callers.

ACK

> src/qemu/qemu_process.c | 6 ++----
> src/util/virfile.c      | 8 ++++++--
> tests/virhostdevtest.c  | 2 +-
> tests/virscsitest.c     | 2 +-
> 4 files changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index ce2c70c..155846e 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -5269,14 +5269,12 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>
>     ignore_value(virAsprintf(&tmppath, "%s/domain-%s",
>                              cfg->libDir, vm->def->name));
>-    if (tmppath)
>-        virFileDeleteTree(tmppath);
>+    virFileDeleteTree(tmppath);
>     VIR_FREE(tmppath);
>
>     ignore_value(virAsprintf(&tmppath, "%s/domain-%s",
>                              cfg->channelTargetDir, vm->def->name));
>-    if (tmppath)
>-        virFileDeleteTree(tmppath);
>+    virFileDeleteTree(tmppath);
>     VIR_FREE(tmppath);
>
>     ignore_value(virDomainChrDefForeach(vm->def,
>diff --git a/src/util/virfile.c b/src/util/virfile.c
>index 75819d9..fe9f8d4 100644
>--- a/src/util/virfile.c
>+++ b/src/util/virfile.c
>@@ -934,13 +934,17 @@ int virFileNBDDeviceAssociate(const char *file,
>  */
> int virFileDeleteTree(const char *dir)
> {
>-    DIR *dh = opendir(dir);
>+    DIR *dh;
>     struct dirent *de;
>     char *filepath = NULL;
>     int ret = -1;
>     int direrr;
>
>-    if (!dh) {
>+    /* Silently return 0 if passed NULL or directory doesn't exist */
>+    if (!dir || !virFileExists(dir))
>+        return 0;
>+
>+    if (!(dh = opendir(dir))) {
>         virReportSystemError(errno, _("Cannot open dir '%s'"),
>                              dir);
>         return -1;
>diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
>index 1e93819..065b825 100644
>--- a/tests/virhostdevtest.c
>+++ b/tests/virhostdevtest.c
>@@ -65,7 +65,7 @@ myCleanup(void)
>     }
>
>     if (mgr) {
>-        if (mgr->stateDir && !getenv("LIBVIRT_SKIP_CLEANUP"))
>+        if (!getenv("LIBVIRT_SKIP_CLEANUP"))
>             virFileDeleteTree(mgr->stateDir);
>
>         virObjectUnref(mgr->activePCIHostdevs);
>diff --git a/tests/virscsitest.c b/tests/virscsitest.c
>index a86ca28..88286f1 100644
>--- a/tests/virscsitest.c
>+++ b/tests/virscsitest.c
>@@ -241,7 +241,7 @@ mymain(void)
>         ret = -1;
>
>  cleanup:
>-    if (tmpdir && getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
>+    if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
>         virFileDeleteTree(tmpdir);
>     VIR_FREE(virscsi_prefix);
>     return ret;
>--
>2.1.0
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150916/4a8b9898/attachment-0001.sig>