* [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering
@ 2019-08-21 15:53 Daniel P. Berrangé
2019-08-22 0:35 ` Wei Yang
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2019-08-21 15:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Wei Yang, Peter Xu
The ordering of events that are emitted during the rmdir
test have changed with kernel >= 5.3. Semantically both
new & old orderings are correct, so we must be able to
cope with either.
To cope with this, when we see an unexpected event, we
push it back onto the queue and look and the subsequent
event to see if that matches instead.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/test-util-filemonitor.c | 43 +++++++++++++++++++++++++++--------
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 46e781c022..301cd2db61 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -45,6 +45,11 @@ typedef struct {
const char *filedst;
int64_t *watchid;
int eventid;
+ /*
+ * Only valid with OP_EVENT - this event might be
+ * swapped with the next OP_EVENT
+ */
+ bool swapnext;
} QFileMonitorTestOp;
typedef struct {
@@ -98,6 +103,10 @@ qemu_file_monitor_test_handler(int64_t id,
QFileMonitorTestData *data = opaque;
QFileMonitorTestRecord *rec = g_new0(QFileMonitorTestRecord, 1);
+ if (debug) {
+ g_printerr("Queue event id %" PRIx64 " event %d file %s\n",
+ id, event, filename);
+ }
rec->id = id;
rec->event = event;
rec->filename = g_strdup(filename);
@@ -125,7 +134,8 @@ qemu_file_monitor_test_record_free(QFileMonitorTestRecord *rec)
* to wait for the event to be queued for us.
*/
static QFileMonitorTestRecord *
-qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
+qemu_file_monitor_test_next_record(QFileMonitorTestData *data,
+ QFileMonitorTestRecord *pushback)
{
GTimer *timer = g_timer_new();
QFileMonitorTestRecord *record = NULL;
@@ -139,9 +149,15 @@ qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
}
if (data->records) {
record = data->records->data;
- tmp = data->records;
- data->records = g_list_remove_link(data->records, tmp);
- g_list_free(tmp);
+ if (pushback) {
+ data->records->data = pushback;
+ } else {
+ tmp = data->records;
+ data->records = g_list_remove_link(data->records, tmp);
+ g_list_free(tmp);
+ }
+ } else if (pushback) {
+ qemu_file_monitor_test_record_free(pushback);
}
qemu_mutex_unlock(&data->lock);
@@ -158,13 +174,15 @@ static bool
qemu_file_monitor_test_expect(QFileMonitorTestData *data,
int64_t id,
QFileMonitorEvent event,
- const char *filename)
+ const char *filename,
+ bool swapnext)
{
QFileMonitorTestRecord *rec;
bool ret = false;
- rec = qemu_file_monitor_test_next_record(data);
+ rec = qemu_file_monitor_test_next_record(data, NULL);
+ retry:
if (!rec) {
g_printerr("Missing event watch id %" PRIx64 " event %d file %s\n",
id, event, filename);
@@ -172,6 +190,11 @@ qemu_file_monitor_test_expect(QFileMonitorTestData *data,
}
if (id != rec->id) {
+ if (swapnext) {
+ rec = qemu_file_monitor_test_next_record(data, rec);
+ swapnext = false;
+ goto retry;
+ }
g_printerr("Expected watch id %" PRIx64 " but got %" PRIx64 "\n",
id, rec->id);
goto cleanup;
@@ -347,7 +370,8 @@ test_file_monitor_events(void)
.filesrc = "fish", },
{ .type = QFILE_MONITOR_TEST_OP_EVENT,
.filesrc = "", .watchid = &watch4,
- .eventid = QFILE_MONITOR_EVENT_IGNORED },
+ .eventid = QFILE_MONITOR_EVENT_IGNORED,
+ .swapnext = true },
{ .type = QFILE_MONITOR_TEST_OP_EVENT,
.filesrc = "fish", .watchid = &watch0,
.eventid = QFILE_MONITOR_EVENT_DELETED },
@@ -493,8 +517,9 @@ test_file_monitor_events(void)
g_printerr("Event id=%" PRIx64 " event=%d file=%s\n",
*op->watchid, op->eventid, op->filesrc);
}
- if (!qemu_file_monitor_test_expect(
- &data, *op->watchid, op->eventid, op->filesrc))
+ if (!qemu_file_monitor_test_expect(&data, *op->watchid,
+ op->eventid, op->filesrc,
+ op->swapnext))
goto cleanup;
break;
case QFILE_MONITOR_TEST_OP_CREATE:
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering
2019-08-21 15:53 [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering Daniel P. Berrangé
@ 2019-08-22 0:35 ` Wei Yang
2019-08-22 3:37 ` Peter Xu
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Wei Yang @ 2019-08-22 0:35 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Peter Xu, Wei Yang
On Wed, Aug 21, 2019 at 04:53:27PM +0100, Daniel P. Berrangé wrote:
>The ordering of events that are emitted during the rmdir
>test have changed with kernel >= 5.3. Semantically both
>new & old orderings are correct, so we must be able to
>cope with either.
>
>To cope with this, when we see an unexpected event, we
>push it back onto the queue and look and the subsequent
>event to see if that matches instead.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Wei Yang <richardw.yang@linux.intel.com>
>---
> tests/test-util-filemonitor.c | 43 +++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 9 deletions(-)
>
>diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
>index 46e781c022..301cd2db61 100644
>--- a/tests/test-util-filemonitor.c
>+++ b/tests/test-util-filemonitor.c
>@@ -45,6 +45,11 @@ typedef struct {
> const char *filedst;
> int64_t *watchid;
> int eventid;
>+ /*
>+ * Only valid with OP_EVENT - this event might be
>+ * swapped with the next OP_EVENT
>+ */
>+ bool swapnext;
> } QFileMonitorTestOp;
>
> typedef struct {
>@@ -98,6 +103,10 @@ qemu_file_monitor_test_handler(int64_t id,
> QFileMonitorTestData *data = opaque;
> QFileMonitorTestRecord *rec = g_new0(QFileMonitorTestRecord, 1);
>
>+ if (debug) {
>+ g_printerr("Queue event id %" PRIx64 " event %d file %s\n",
>+ id, event, filename);
>+ }
> rec->id = id;
> rec->event = event;
> rec->filename = g_strdup(filename);
>@@ -125,7 +134,8 @@ qemu_file_monitor_test_record_free(QFileMonitorTestRecord *rec)
> * to wait for the event to be queued for us.
> */
> static QFileMonitorTestRecord *
>-qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
>+qemu_file_monitor_test_next_record(QFileMonitorTestData *data,
>+ QFileMonitorTestRecord *pushback)
> {
> GTimer *timer = g_timer_new();
> QFileMonitorTestRecord *record = NULL;
>@@ -139,9 +149,15 @@ qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
> }
> if (data->records) {
> record = data->records->data;
>- tmp = data->records;
>- data->records = g_list_remove_link(data->records, tmp);
>- g_list_free(tmp);
>+ if (pushback) {
>+ data->records->data = pushback;
>+ } else {
>+ tmp = data->records;
>+ data->records = g_list_remove_link(data->records, tmp);
>+ g_list_free(tmp);
>+ }
>+ } else if (pushback) {
>+ qemu_file_monitor_test_record_free(pushback);
> }
> qemu_mutex_unlock(&data->lock);
>
>@@ -158,13 +174,15 @@ static bool
> qemu_file_monitor_test_expect(QFileMonitorTestData *data,
> int64_t id,
> QFileMonitorEvent event,
>- const char *filename)
>+ const char *filename,
>+ bool swapnext)
> {
> QFileMonitorTestRecord *rec;
> bool ret = false;
>
>- rec = qemu_file_monitor_test_next_record(data);
>+ rec = qemu_file_monitor_test_next_record(data, NULL);
>
>+ retry:
> if (!rec) {
> g_printerr("Missing event watch id %" PRIx64 " event %d file %s\n",
> id, event, filename);
>@@ -172,6 +190,11 @@ qemu_file_monitor_test_expect(QFileMonitorTestData *data,
> }
>
> if (id != rec->id) {
>+ if (swapnext) {
>+ rec = qemu_file_monitor_test_next_record(data, rec);
>+ swapnext = false;
>+ goto retry;
>+ }
> g_printerr("Expected watch id %" PRIx64 " but got %" PRIx64 "\n",
> id, rec->id);
> goto cleanup;
>@@ -347,7 +370,8 @@ test_file_monitor_events(void)
> .filesrc = "fish", },
> { .type = QFILE_MONITOR_TEST_OP_EVENT,
> .filesrc = "", .watchid = &watch4,
>- .eventid = QFILE_MONITOR_EVENT_IGNORED },
>+ .eventid = QFILE_MONITOR_EVENT_IGNORED,
>+ .swapnext = true },
> { .type = QFILE_MONITOR_TEST_OP_EVENT,
> .filesrc = "fish", .watchid = &watch0,
> .eventid = QFILE_MONITOR_EVENT_DELETED },
>@@ -493,8 +517,9 @@ test_file_monitor_events(void)
> g_printerr("Event id=%" PRIx64 " event=%d file=%s\n",
> *op->watchid, op->eventid, op->filesrc);
> }
>- if (!qemu_file_monitor_test_expect(
>- &data, *op->watchid, op->eventid, op->filesrc))
>+ if (!qemu_file_monitor_test_expect(&data, *op->watchid,
>+ op->eventid, op->filesrc,
>+ op->swapnext))
> goto cleanup;
> break;
> case QFILE_MONITOR_TEST_OP_CREATE:
>--
>2.21.0
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering
2019-08-21 15:53 [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering Daniel P. Berrangé
2019-08-22 0:35 ` Wei Yang
@ 2019-08-22 3:37 ` Peter Xu
2019-08-22 16:21 ` Cornelia Huck
2019-09-04 10:10 ` Thomas Huth
3 siblings, 0 replies; 5+ messages in thread
From: Peter Xu @ 2019-08-22 3:37 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Wei Yang
On Wed, Aug 21, 2019 at 04:53:27PM +0100, Daniel P. Berrangé wrote:
> The ordering of events that are emitted during the rmdir
> test have changed with kernel >= 5.3. Semantically both
> new & old orderings are correct, so we must be able to
> cope with either.
>
> To cope with this, when we see an unexpected event, we
> push it back onto the queue and look and the subsequent
> event to see if that matches instead.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Thanks for fixing it!
Tested-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering
2019-08-21 15:53 [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering Daniel P. Berrangé
2019-08-22 0:35 ` Wei Yang
2019-08-22 3:37 ` Peter Xu
@ 2019-08-22 16:21 ` Cornelia Huck
2019-09-04 10:10 ` Thomas Huth
3 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2019-08-22 16:21 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, Peter Xu, Wei Yang
On Wed, 21 Aug 2019 16:53:27 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:
> The ordering of events that are emitted during the rmdir
> test have changed with kernel >= 5.3. Semantically both
> new & old orderings are correct, so we must be able to
> cope with either.
>
> To cope with this, when we see an unexpected event, we
> push it back onto the queue and look and the subsequent
> event to see if that matches instead.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/test-util-filemonitor.c | 43 +++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 9 deletions(-)
Tested-by: Cornelia Huck <cohuck@redhat.com>
I tried to review this patch, but I fear I'm out of my depth here :(
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering
2019-08-21 15:53 [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering Daniel P. Berrangé
` (2 preceding siblings ...)
2019-08-22 16:21 ` Cornelia Huck
@ 2019-09-04 10:10 ` Thomas Huth
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2019-09-04 10:10 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel; +Cc: Wei Yang, Peter Xu
On 21/08/2019 17.53, Daniel P. Berrangé wrote:
> The ordering of events that are emitted during the rmdir
> test have changed with kernel >= 5.3. Semantically both
> new & old orderings are correct, so we must be able to
> cope with either.
>
> To cope with this, when we see an unexpected event, we
> push it back onto the queue and look and the subsequent
> event to see if that matches instead.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/test-util-filemonitor.c | 43 +++++++++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
> index 46e781c022..301cd2db61 100644
> --- a/tests/test-util-filemonitor.c
> +++ b/tests/test-util-filemonitor.c
> @@ -45,6 +45,11 @@ typedef struct {
> const char *filedst;
> int64_t *watchid;
> int eventid;
> + /*
> + * Only valid with OP_EVENT - this event might be
> + * swapped with the next OP_EVENT
> + */
> + bool swapnext;
> } QFileMonitorTestOp;
>
> typedef struct {
> @@ -98,6 +103,10 @@ qemu_file_monitor_test_handler(int64_t id,
> QFileMonitorTestData *data = opaque;
> QFileMonitorTestRecord *rec = g_new0(QFileMonitorTestRecord, 1);
>
> + if (debug) {
> + g_printerr("Queue event id %" PRIx64 " event %d file %s\n",
> + id, event, filename);
> + }
> rec->id = id;
> rec->event = event;
> rec->filename = g_strdup(filename);
> @@ -125,7 +134,8 @@ qemu_file_monitor_test_record_free(QFileMonitorTestRecord *rec)
> * to wait for the event to be queued for us.
> */
> static QFileMonitorTestRecord *
> -qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
> +qemu_file_monitor_test_next_record(QFileMonitorTestData *data,
> + QFileMonitorTestRecord *pushback)
> {
> GTimer *timer = g_timer_new();
> QFileMonitorTestRecord *record = NULL;
> @@ -139,9 +149,15 @@ qemu_file_monitor_test_next_record(QFileMonitorTestData *data)
> }
> if (data->records) {
> record = data->records->data;
> - tmp = data->records;
> - data->records = g_list_remove_link(data->records, tmp);
> - g_list_free(tmp);
> + if (pushback) {
> + data->records->data = pushback;
> + } else {
> + tmp = data->records;
> + data->records = g_list_remove_link(data->records, tmp);
> + g_list_free(tmp);
> + }
> + } else if (pushback) {
> + qemu_file_monitor_test_record_free(pushback);
> }
> qemu_mutex_unlock(&data->lock);
>
> @@ -158,13 +174,15 @@ static bool
> qemu_file_monitor_test_expect(QFileMonitorTestData *data,
> int64_t id,
> QFileMonitorEvent event,
> - const char *filename)
> + const char *filename,
> + bool swapnext)
> {
> QFileMonitorTestRecord *rec;
> bool ret = false;
>
> - rec = qemu_file_monitor_test_next_record(data);
> + rec = qemu_file_monitor_test_next_record(data, NULL);
>
> + retry:
> if (!rec) {
> g_printerr("Missing event watch id %" PRIx64 " event %d file %s\n",
> id, event, filename);
> @@ -172,6 +190,11 @@ qemu_file_monitor_test_expect(QFileMonitorTestData *data,
> }
>
> if (id != rec->id) {
> + if (swapnext) {
> + rec = qemu_file_monitor_test_next_record(data, rec);
> + swapnext = false;
> + goto retry;
> + }
> g_printerr("Expected watch id %" PRIx64 " but got %" PRIx64 "\n",
> id, rec->id);
> goto cleanup;
> @@ -347,7 +370,8 @@ test_file_monitor_events(void)
> .filesrc = "fish", },
> { .type = QFILE_MONITOR_TEST_OP_EVENT,
> .filesrc = "", .watchid = &watch4,
> - .eventid = QFILE_MONITOR_EVENT_IGNORED },
> + .eventid = QFILE_MONITOR_EVENT_IGNORED,
> + .swapnext = true },
> { .type = QFILE_MONITOR_TEST_OP_EVENT,
> .filesrc = "fish", .watchid = &watch0,
> .eventid = QFILE_MONITOR_EVENT_DELETED },
> @@ -493,8 +517,9 @@ test_file_monitor_events(void)
> g_printerr("Event id=%" PRIx64 " event=%d file=%s\n",
> *op->watchid, op->eventid, op->filesrc);
> }
> - if (!qemu_file_monitor_test_expect(
> - &data, *op->watchid, op->eventid, op->filesrc))
> + if (!qemu_file_monitor_test_expect(&data, *op->watchid,
> + op->eventid, op->filesrc,
> + op->swapnext))
> goto cleanup;
> break;
> case QFILE_MONITOR_TEST_OP_CREATE:
>
I don't know that part of the code, but this looks ok to me at a quick
glance. So FWIW, from my side a light
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-04 10:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 15:53 [Qemu-devel] [PATCH] tests: make filemonitor test more robust to event ordering Daniel P. Berrangé
2019-08-22 0:35 ` Wei Yang
2019-08-22 3:37 ` Peter Xu
2019-08-22 16:21 ` Cornelia Huck
2019-09-04 10:10 ` Thomas Huth
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).