[PATCH] Fix flapping/flakey notify test - V2
Jeremy Allison
jra at samba.org
Tue Sep 11 16:32:57 UTC 2018
On Tue, Sep 11, 2018 at 09:16:30AM +0200, Andreas Schneider wrote:
> On Tuesday, 11 September 2018 02:16:49 CEST Jeremy Allison via samba-technical
> wrote:
> > V2. Minor change - correctly check for NULL
> > returns on talloc_asprintf()'s for error
> > messages.
>
> Hi Jeremy,
>
> attached is the patch with a squash commit to use size_t. Something the
> optimizer might complain about.
>
> However what I would like to see in future is that we use:
>
> torture_assert_*_goto()
>
> to jump to a labal and do cleanup (free memory and close connections).
>
> In this case call smb_raw_exit() and smbcli_deltree().
Thanks, I'm good with that fixup.
Yes, that fixup would be better in future, but I'm going
to pish the combined for now.
>
>
> Andreas
>
> --
> Andreas Schneider asn at samba.org
> Samba Team www.samba.org
> GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
> From 1e9cff2d85130c93da91320da904c48060346347 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Mon, 10 Sep 2018 15:35:03 -0700
> Subject: [PATCH 1/2] s4: torture: Fix "flakey" notify test on slow cloud
> filesystems.
>
> Ensure we keep asking for change notifies until we get them all
> (or the request times out).
>
> Signed-off-by: Jeremy Allison <jra at samba.org>
> Reviewed-by: Andreas Schneider <asn at samba.org>
> ---
> source4/torture/raw/notify.c | 116 +++++++++++++++++++++++++++++++----
> 1 file changed, 104 insertions(+), 12 deletions(-)
>
> diff --git a/source4/torture/raw/notify.c b/source4/torture/raw/notify.c
> index 9a993f5bd88..7b20918d27f 100644
> --- a/source4/torture/raw/notify.c
> +++ b/source4/torture/raw/notify.c
> @@ -24,6 +24,7 @@
> #include "system/filesys.h"
> #include "torture/util.h"
> #include "torture/raw/proto.h"
> +#include "lib/events/events.h"
>
> #define BASEDIR "\\test_notify"
>
> @@ -2000,6 +2001,20 @@ done:
> return ret;
> }
>
> +struct cb_data {
> + struct smbcli_request *req;
> + bool timed_out;
> +};
> +
> +static void timeout_cb(struct tevent_context *ev,
> + struct tevent_timer *te,
> + struct timeval current_time,
> + void *private_data)
> +{
> + struct cb_data *cbp = (struct cb_data *)private_data;
> + cbp->req->state = SMBCLI_REQUEST_ERROR;
> + cbp->timed_out = true;
> +}
>
> /*
> testing alignment of multiple change notify infos
> @@ -2020,6 +2035,11 @@ static bool test_notify_alignment(struct torture_context *tctx,
> "ab",
> "abc",
> "abcd" };
> + bool fnames_received[] = {false,
> + false,
> + false,
> + false};
> + unsigned int total_names_received = 0;
> int num_names = ARRAY_SIZE(fnames);
> char *fpath = NULL;
>
> @@ -2079,19 +2099,91 @@ static bool test_notify_alignment(struct torture_context *tctx,
> talloc_free(fpath);
> }
>
> - /* We send a notify packet, and let smb_raw_changenotify_recv() do
> - * the alignment checking for us. */
> - req = smb_raw_changenotify_send(cli->tree, ¬ify);
> - status = smb_raw_changenotify_recv(req, tctx, ¬ify);
> - torture_assert_ntstatus_ok(tctx, status, "smb_raw_changenotify_recv");
> + /*
> + * Slow cloud filesystems mean we might
> + * not get everything in one go. Keep going
> + * until we get them all.
> + */
> + while (total_names_received < num_names) {
> + struct tevent_timer *te = NULL;
> + struct cb_data to_data = {0};
>
> - /* Do basic checking for correctness. */
> - torture_assert(tctx, notify.nttrans.out.num_changes == num_names, "");
> - for (i = 0; i < num_names; i++) {
> - torture_assert(tctx, notify.nttrans.out.changes[i].action ==
> - NOTIFY_ACTION_ADDED, "");
> - CHECK_WSTR(tctx, notify.nttrans.out.changes[i].name, fnames[i],
> - STR_UNICODE);
> + /*
> + * We send a notify packet, and let
> + * smb_raw_changenotify_recv() do
> + * the alignment checking for us.
> + */
> + req = smb_raw_changenotify_send(cli->tree, ¬ify);
> + torture_assert(tctx,
> + req != NULL,
> + "smb_raw_changenotify_send failed\n");
> +
> + /* Ensure we don't wait more than 30 seconds. */
> + to_data.req = req;
> + to_data.timed_out = false;
> +
> + te = tevent_add_timer(tctx->ev,
> + req,
> + tevent_timeval_current_ofs(30, 0),
> + timeout_cb,
> + &to_data);
> + if (te == NULL) {
> + torture_fail(tctx, "tevent_add_timer fail\n");
> + }
> +
> + status = smb_raw_changenotify_recv(req, tctx, ¬ify);
> + if (!NT_STATUS_IS_OK(status)) {
> + if (to_data.timed_out == true) {
> + torture_fail(tctx, "smb_raw_changenotify_recv "
> + "timed out\n");
> + }
> + }
> +
> + torture_assert_ntstatus_ok(tctx, status,
> + "smb_raw_changenotify_recv");
> +
> + for (i = 0; i < notify.nttrans.out.num_changes; i++) {
> + unsigned int j;
> +
> + /* Ensure it was an 'add'. */
> + torture_assert(tctx,
> + notify.nttrans.out.changes[i].action ==
> + NOTIFY_ACTION_ADDED,
> + "");
> +
> + for (j = 0; j < num_names; j++) {
> + if (strcmp(notify.nttrans.out.changes[i].name.s,
> + fnames[j]) == 0) {
> + if (fnames_received[j] == true) {
> + const char *err =
> + talloc_asprintf(tctx,
> + "Duplicate "
> + "name %s\n",
> + fnames[j]);
> + if (err == NULL) {
> + torture_fail(tctx,
> + "talloc "
> + "fail\n");
> + }
> + /* already got this. */
> + torture_fail(tctx, err);
> + }
> + fnames_received[j] = true;
> + break;
> + }
> + }
> + if (j == num_names) {
> + /* No name match. */
> + const char *err = talloc_asprintf(tctx,
> + "Unexpected name %s\n",
> + notify.nttrans.out.changes[i].name.s);
> + if (err == NULL) {
> + torture_fail(tctx, "talloc fail\n");
> + }
> + torture_fail(tctx, err);
> + }
> + total_names_received++;
> + }
> }
>
> smb_raw_exit(cli->session);
> --
> 2.18.0
>
>
> From dd147c15bb7a5eb7ea577cd5547d9c59eb2f7202 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 11 Sep 2018 09:08:49 +0200
> Subject: [PATCH 2/2] SQ
>
> ---
> source4/torture/raw/notify.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/source4/torture/raw/notify.c b/source4/torture/raw/notify.c
> index 7b20918d27f..ee6e92e81b3 100644
> --- a/source4/torture/raw/notify.c
> +++ b/source4/torture/raw/notify.c
> @@ -2028,7 +2028,7 @@ static bool test_notify_alignment(struct torture_context *tctx,
> NTSTATUS status;
> union smb_notify notify;
> union smb_open io;
> - int i, fnum, fnum2;
> + int fnum, fnum2;
> struct smbcli_request *req;
> const char *fname = BASEDIR_CN1_NALIGN "\\starter";
> const char *fnames[] = { "a",
> @@ -2039,8 +2039,9 @@ static bool test_notify_alignment(struct torture_context *tctx,
> false,
> false,
> false};
> - unsigned int total_names_received = 0;
> - int num_names = ARRAY_SIZE(fnames);
> + size_t total_names_received = 0;
> + size_t num_names = ARRAY_SIZE(fnames);
> + size_t i;
> char *fpath = NULL;
>
> torture_comment(tctx, "TESTING CHANGE NOTIFY REPLY ALIGNMENT\n");
> @@ -2143,7 +2144,7 @@ static bool test_notify_alignment(struct torture_context *tctx,
> "smb_raw_changenotify_recv");
>
> for (i = 0; i < notify.nttrans.out.num_changes; i++) {
> - unsigned int j;
> + size_t j;
>
> /* Ensure it was an 'add'. */
> torture_assert(tctx,
> --
> 2.18.0
>
More information about the samba-technical
mailing list