From ff3dc8fefe953fd3650279e064bf63b212c5699a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Wed, 13 Mar 2019 17:36:18 +0000
Subject: [PATCH] filemon: ensure watch IDs are unique to QFileMonitor scope
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The watch IDs are mistakenly only unique within the scope of the
directory being monitored. This is not useful for clients which are
monitoring multiple directories. They require watch IDs to be unique
globally within the QFileMonitor scope.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Bandan Das <bsd@redhat.com>
Reviewed-by: Bandan Das <bsd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/test-util-filemonitor.c | 116 +++++++++++++++++++++++++++++++---
 util/filemonitor-inotify.c    |   5 +-
 2 files changed, 110 insertions(+), 11 deletions(-)

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index ea3715a8f44..71a7cf5de05 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -35,6 +35,8 @@ enum {
     QFILE_MONITOR_TEST_OP_RENAME,
     QFILE_MONITOR_TEST_OP_TOUCH,
     QFILE_MONITOR_TEST_OP_UNLINK,
+    QFILE_MONITOR_TEST_OP_MKDIR,
+    QFILE_MONITOR_TEST_OP_RMDIR,
 };
 
 typedef struct {
@@ -298,6 +300,54 @@ test_file_monitor_events(void)
           .eventid = QFILE_MONITOR_EVENT_DELETED },
 
 
+        { .type = QFILE_MONITOR_TEST_OP_MKDIR,
+          .filesrc = "fish", },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "fish", .watchid = 0,
+          .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+        { .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+          .filesrc = "fish/", .watchid = 4 },
+        { .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+          .filesrc = "fish/one.txt", .watchid = 5 },
+        { .type = QFILE_MONITOR_TEST_OP_CREATE,
+          .filesrc = "fish/one.txt", },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "one.txt", .watchid = 4,
+          .eventid = QFILE_MONITOR_EVENT_CREATED },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "one.txt", .watchid = 5,
+          .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+        { .type = QFILE_MONITOR_TEST_OP_DEL_WATCH,
+          .filesrc = "fish/one.txt", .watchid = 5 },
+        { .type = QFILE_MONITOR_TEST_OP_RENAME,
+          .filesrc = "fish/one.txt", .filedst = "two.txt", },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "one.txt", .watchid = 4,
+          .eventid = QFILE_MONITOR_EVENT_DELETED },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "two.txt", .watchid = 0,
+          .eventid = QFILE_MONITOR_EVENT_CREATED },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "two.txt", .watchid = 2,
+          .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+        { .type = QFILE_MONITOR_TEST_OP_RMDIR,
+          .filesrc = "fish", },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "", .watchid = 4,
+          .eventid = QFILE_MONITOR_EVENT_IGNORED },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "fish", .watchid = 0,
+          .eventid = QFILE_MONITOR_EVENT_DELETED },
+        { .type = QFILE_MONITOR_TEST_OP_DEL_WATCH,
+          .filesrc = "fish", .watchid = 4 },
+
+
         { .type = QFILE_MONITOR_TEST_OP_UNLINK,
           .filesrc = "two.txt", },
         { .type = QFILE_MONITOR_TEST_OP_EVENT,
@@ -366,6 +416,8 @@ test_file_monitor_events(void)
         int fd;
         int watchid;
         struct utimbuf ubuf;
+        char *watchdir;
+        const char *watchfile;
 
         pathsrc = g_strdup_printf("%s/%s", dir, op->filesrc);
         if (op->filedst) {
@@ -378,13 +430,26 @@ test_file_monitor_events(void)
                 g_printerr("Add watch %s %s %d\n",
                            dir, op->filesrc, op->watchid);
             }
+            if (op->filesrc && strchr(op->filesrc, '/')) {
+                watchdir = g_strdup_printf("%s/%s", dir, op->filesrc);
+                watchfile = strrchr(watchdir, '/');
+                *(char *)watchfile = '\0';
+                watchfile++;
+                if (*watchfile == '\0') {
+                    watchfile = NULL;
+                }
+            } else {
+                watchdir = g_strdup(dir);
+                watchfile = op->filesrc;
+            }
             watchid =
                 qemu_file_monitor_add_watch(mon,
-                                            dir,
-                                            op->filesrc,
+                                            watchdir,
+                                            watchfile,
                                             qemu_file_monitor_test_handler,
                                             &data,
                                             &local_err);
+            g_free(watchdir);
             if (watchid < 0) {
                 g_printerr("Unable to add watch %s",
                            error_get_pretty(local_err));
@@ -400,9 +465,17 @@ test_file_monitor_events(void)
             if (debug) {
                 g_printerr("Del watch %s %d\n", dir, op->watchid);
             }
+            if (op->filesrc && strchr(op->filesrc, '/')) {
+                watchdir = g_strdup_printf("%s/%s", dir, op->filesrc);
+                watchfile = strrchr(watchdir, '/');
+                *(char *)watchfile = '\0';
+            } else {
+                watchdir = g_strdup(dir);
+            }
             qemu_file_monitor_remove_watch(mon,
-                                           dir,
+                                           watchdir,
                                            op->watchid);
+            g_free(watchdir);
             break;
         case QFILE_MONITOR_TEST_OP_EVENT:
             if (debug) {
@@ -492,6 +565,28 @@ test_file_monitor_events(void)
             }
             break;
 
+        case QFILE_MONITOR_TEST_OP_MKDIR:
+            if (debug) {
+                g_printerr("Mkdir %s\n", pathsrc);
+            }
+            if (mkdir(pathsrc, 0700) < 0) {
+                g_printerr("Unable to mkdir %s: %s",
+                           pathsrc, strerror(errno));
+                goto cleanup;
+            }
+            break;
+
+        case QFILE_MONITOR_TEST_OP_RMDIR:
+            if (debug) {
+                g_printerr("Rmdir %s\n", pathsrc);
+            }
+            if (rmdir(pathsrc) < 0) {
+                g_printerr("Unable to rmdir %s: %s",
+                           pathsrc, strerror(errno));
+                goto cleanup;
+            }
+            break;
+
         default:
             g_assert_not_reached();
         }
@@ -532,13 +627,18 @@ test_file_monitor_events(void)
             const QFileMonitorTestOp *op = &(ops[i]);
             char *path = g_strdup_printf("%s/%s",
                                          dir, op->filesrc);
-            unlink(path);
-            g_free(path);
-            if (op->filedst) {
-                path = g_strdup_printf("%s/%s",
-                                       dir, op->filedst);
+            if (op->type == QFILE_MONITOR_TEST_OP_MKDIR) {
+                rmdir(path);
+                g_free(path);
+            } else {
                 unlink(path);
                 g_free(path);
+                if (op->filedst) {
+                    path = g_strdup_printf("%s/%s",
+                                           dir, op->filedst);
+                    unlink(path);
+                    g_free(path);
+                }
             }
         }
         if (rmdir(dir) < 0) {
diff --git a/util/filemonitor-inotify.c b/util/filemonitor-inotify.c
index 3a72be037f1..3eb29f860be 100644
--- a/util/filemonitor-inotify.c
+++ b/util/filemonitor-inotify.c
@@ -29,7 +29,7 @@
 
 struct QFileMonitor {
     int fd;
-
+    int nextid; /* watch ID counter */
     QemuMutex lock; /* protects dirs & idmap */
     GHashTable *dirs; /* dirname => QFileMonitorDir */
     GHashTable *idmap; /* inotify ID => dirname */
@@ -47,7 +47,6 @@ typedef struct {
 typedef struct {
     char *path;
     int id; /* inotify ID */
-    int nextid; /* watch ID counter */
     GArray *watches; /* QFileMonitorWatch elements */
 } QFileMonitorDir;
 
@@ -277,7 +276,7 @@ qemu_file_monitor_add_watch(QFileMonitor *mon,
         }
     }
 
-    watch.id = dir->nextid++;
+    watch.id = mon->nextid++;
     watch.filename = g_strdup(filename);
     watch.cb = cb;
     watch.opaque = opaque;
-- 
GitLab