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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---

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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]