From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Gerd Hoffmann" <kraxel@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Bandan Das" <bsd@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: [Qemu-devel] [PULL 2/3] filemon: ensure watch IDs are unique to QFileMonitor scope
Date: Tue, 2 Apr 2019 13:54:16 +0100 [thread overview]
Message-ID: <20190402125417.21573-3-berrange@redhat.com> (raw)
In-Reply-To: <20190402125417.21573-1-berrange@redhat.com>
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 ea3715a8f4..71a7cf5de0 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 3a72be037f..3eb29f860b 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;
--
2.20.1
next prev parent reply other threads:[~2019-04-02 12:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-02 12:54 [Qemu-devel] [PULL 0/3] Filemon next patches Daniel P. Berrangé
2019-04-02 12:54 ` [Qemu-devel] [PULL 1/3] tests: refactor file monitor test to make it more understandable Daniel P. Berrangé
2019-04-02 12:54 ` Daniel P. Berrangé [this message]
2019-04-02 12:54 ` [Qemu-devel] [PULL 3/3] filemon: fix watch IDs to avoid potential wraparound issues Daniel P. Berrangé
2019-04-02 14:31 ` [Qemu-devel] [PULL 0/3] Filemon next patches Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190402125417.21573-3-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=bsd@redhat.com \
--cc=kraxel@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).