[PATCH] Test for bug 12903

[Jeremy Allison]

On Fri, Aug 25, 2017 at 04:46:04PM +0200, Ralph Böhme via samba-technical wrote:
> Hi!
> 
> Attached is a testcase that prooves that bug 12903 "File change notification for
> renames not working" got fixed by a bunch of messaging patches from Volker
> (5eccc2fd0072409f166c63e6876266f926411423~10..5eccc2fd0072409f166c63e6876266f926411423).
> 
> The fixes are in master and 4.7, but we need backports for 4.6 and 4.5. The
> patches apply cleanly and work for 4.6 as well, still have to check 4.5.
> 
> Please review & push if ok. Once this patces are in, I'll roll up backports of
> the fix alongside the testcase.

LGTM. Nice work Ralph !

> From 1bc4bb4794ed39bb9ecd8a9e24b2e722a73e663d Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 13 Jul 2017 16:01:53 +0200
> Subject: [PATCH 1/3] selftest: enable kernel change notifications in the
>  fileserver environment
> 
> This environment is currently not used for any test in the smb2
> testsuite, so this change doesn't affect any existing test.
> 
> A subsequent commit will add a test for change notifications with kernel
> change notify enabled. It verifies a bug (this one) that only crops up
> in such a setup and causes rename events to get lost.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12903
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  selftest/target/Samba3.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 54da52b..45c00ba 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -884,6 +884,8 @@ sub setup_fileserver($$)
>  	push(@dirs,$usershare_sharedir);
>  
>  	my $fileserver_options = "
> +	kernel change notify = yes
> +
>  	usershare path = $usershare_dir
>  	usershare max shares = 10
>  	usershare allow guests = yes
> -- 
> 2.9.4
> 
> 
> From dc4750110994047bc5657c0bc1dc95542923f4a0 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 13 Jul 2017 16:04:50 +0200
> Subject: [PATCH 2/3] selftest: run smb2.notify-inotify testsuite against
>  fileserver
> 
> Next commit adds the suite and a test.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12903
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/selftest/tests.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index a624f85..17d34cc 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -56,6 +56,7 @@ finally:
>  
>  have_libarchive = ("HAVE_LIBARCHIVE" in config_hash)
>  have_linux_kernel_oplocks = ("HAVE_KERNEL_OPLOCKS_LINUX" in config_hash)
> +have_inotify = ("HAVE_INOTIFY" in config_hash)
>  
>  plantestsuite("samba3.blackbox.success", "nt4_dc:local", [os.path.join(samba3srcdir, "script/tests/test_success.sh")])
>  plantestsuite("samba3.blackbox.failure", "nt4_dc:local", [os.path.join(samba3srcdir, "script/tests/test_failure.sh")])
> @@ -502,6 +503,9 @@ for t in tests:
>      elif t == "smb2.kernel-oplocks":
>          if have_linux_kernel_oplocks:
>              plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER/kernel_oplocks -U$USERNAME%$PASSWORD')
> +    elif t == "smb2.notify-inotify":
> +        if have_inotify:
> +            plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>      elif t == "vfs.acl_xattr":
>          plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>      elif t == "smb2.compound_find":
> -- 
> 2.9.4
> 
> 
> From 414d2a3b375c9ca9cb458e0b50d1b50bf572a419 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 13 Jul 2017 16:05:49 +0200
> Subject: [PATCH 3/3] s4/torture: add a test for rename change notification
>  with inotify enabled
> 
> This is already fixed in master by
> 5eccc2fd0072409f166c63e6876266f926411423~10..5eccc2fd0072409f166c63e6876266f926411423.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12903
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source4/torture/smb2/notify.c | 158 ++++++++++++++++++++++++++++++++++++++++++
>  source4/torture/smb2/smb2.c   |   1 +
>  2 files changed, 159 insertions(+)
> 
> diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
> index 9fc856c..936a4d2 100644
> --- a/source4/torture/smb2/notify.c
> +++ b/source4/torture/smb2/notify.c
> @@ -2354,6 +2354,151 @@ static bool torture_smb2_notify_rmdir4(struct torture_context *torture,
>  	return torture_smb2_notify_rmdir(torture, tree1, tree2, true);
>  }
>  
> +static void notify_timeout(struct tevent_context *ev,
> +			   struct tevent_timer *te,
> +			   struct timeval current_time,
> +			   void *private_data)
> +{
> +	struct smb2_request *req = talloc_get_type_abort(
> +		private_data, struct smb2_request);
> +
> +	smb2_cancel(req);
> +}
> +
> +static bool torture_smb2_inotify_rename(struct torture_context *torture,
> +					struct smb2_tree *tree1,
> +					struct smb2_tree *tree2)
> +{
> +	NTSTATUS status;
> +	struct smb2_notify notify;
> +	struct notify_changes change1 = {0};
> +	struct notify_changes change2 = {0};
> + 	struct smb2_create create;
> +	union smb_setfileinfo sinfo;
> +	struct smb2_handle h1 = {{0}};
> +	struct smb2_handle h2 = {{0}};
> +	struct smb2_request *req;
> +	struct tevent_timer *te = NULL;
> +	bool ok = false;
> +
> +	smb2_deltree(tree1, BASEDIR);
> +
> +	torture_comment(torture, "Testing change notify of a rename with inotify\n");
> +
> +	status = torture_smb2_testdir(tree1, BASEDIR, &h1);
> +	torture_assert_ntstatus_ok_goto(torture, status, ok, done, "torture_smb2_testdir failed");
> +
> +	ZERO_STRUCT(create);
> +	create.in.desired_access = SEC_RIGHTS_FILE_READ |
> +		SEC_RIGHTS_FILE_WRITE|
> +		SEC_RIGHTS_FILE_ALL;
> +	create.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
> +	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
> +	create.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
> +		NTCREATEX_SHARE_ACCESS_WRITE |
> +		NTCREATEX_SHARE_ACCESS_DELETE;
> +	create.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
> +	create.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
> +	create.in.fname = BASEDIR "\\subdir-name";
> +
> +	status = smb2_create(tree2, torture, &create);
> +	torture_assert_ntstatus_ok_goto(torture, status, ok, done, "smb2_create failed\n");
> +	h2 = create.out.file.handle;
> +
> +	ZERO_STRUCT(notify);
> +	notify.level = RAW_NOTIFY_SMB2;
> +	notify.in.buffer_size = 4096;
> +	notify.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
> +	notify.in.file.handle = h1;
> +	notify.in.recursive = true;
> +	req = smb2_notify_send(tree1, &notify);
> +	torture_assert_not_null_goto(torture, req, ok, done, "smb2_notify_send failed\n");
> +
> +	while (!NT_STATUS_EQUAL(req->status, STATUS_PENDING)) {
> +		if (tevent_loop_once(torture->ev) != 0) {
> +			return false;
> +		}
> +	}
> +
> +	ZERO_STRUCT(sinfo);
> +	sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
> +	sinfo.rename_information.in.file.handle = h2;
> +	sinfo.rename_information.in.new_name = BASEDIR "\\subdir-name-r";
> +
> +	status = smb2_setinfo_file(tree2, &sinfo);
> +	torture_assert_ntstatus_ok_goto(torture, status, ok, done, "smb2_setinfo_file failed\n");
> +
> +	smb2_util_close(tree2, h2);
> +
> +	te = tevent_add_timer(torture->ev,
> +			      tree1,
> +			      tevent_timeval_current_ofs(1, 0),
> +			      notify_timeout,
> +			      req);
> +	torture_assert_not_null_goto(torture, te, ok, done, "tevent_add_timer failed\n");
> +
> +	status = smb2_notify_recv(req, torture, &notify);
> +	torture_assert_ntstatus_ok_goto(torture, status, ok, done, "smb2_notify_recv failed\n");
> +
> +	torture_assert_goto(torture, notify.out.num_changes == 1 || notify.out.num_changes == 2,
> +			    ok, done, "bad notify\n");
> +
> +	change1 = notify.out.changes[0];
> +	if (notify.out.num_changes == 2) {
> +		change2 = notify.out.changes[1];
> +	} else {
> +		/*
> +		 * We may only get one event at a time, so check for the
> +		 * matching second event for the oldname/newname or
> +		 * removed/added pair.
> +		 */
> +		ZERO_STRUCT(notify);
> +		notify.level = RAW_NOTIFY_SMB2;
> +		notify.in.buffer_size = 4096;
> +		notify.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
> +		notify.in.file.handle = h1;
> +		notify.in.recursive = true;
> +		req = smb2_notify_send(tree1, &notify);
> +		torture_assert_not_null_goto(torture, req, ok, done, "smb2_notify_send failed\n");
> +
> +		status = smb2_notify_recv(req, torture, &notify);
> +		torture_assert_ntstatus_ok_goto(torture, status, ok, done, "smb2_notify_recv failed\n");
> +
> +		torture_assert_goto(torture, notify.out.num_changes == 1, ok, done,
> +				    "bad notify\n");
> +
> +		change2 = notify.out.changes[0];
> +	}
> +
> +	if ((change1.action != NOTIFY_ACTION_OLD_NAME) &&
> +	    (change1.action != NOTIFY_ACTION_REMOVED))
> +	{
> +		torture_fail_goto(torture, done, "bad change notification\n");
> +	}
> +	torture_assert_str_equal_goto(torture, change1.name.s, "subdir-name",
> +			    ok, done, "bad change notification\n");
> +
> +	if ((change2.action != NOTIFY_ACTION_NEW_NAME) &&
> +	    (change2.action != NOTIFY_ACTION_ADDED))
> +	{
> +		torture_fail_goto(torture, done, "bad change notification\n");
> +	}
> +	torture_assert_str_equal_goto(torture, change2.name.s, "subdir-name-r",
> +			    ok, done, "bad change notification\n");
> +
> +	ok = true;
> +done:
> +	if (!smb2_util_handle_empty(h1)) {
> +		smb2_util_close(tree2, h1);
> +	}
> +	if (!smb2_util_handle_empty(h2)) {
> +		smb2_util_close(tree2, h2);
> +	}
> +
> +	smb2_deltree(tree1, BASEDIR);
> +	return ok;
> +}
> +
>  /*
>     basic testing of SMB2 change notify
>  */
> @@ -2393,3 +2538,16 @@ struct torture_suite *torture_smb2_notify_init(TALLOC_CTX *ctx)
>  	return suite;
>  }
>  
> +/*
> +   basic testing of SMB2 change notify
> +*/
> +struct torture_suite *torture_smb2_notify_inotify_init(TALLOC_CTX *ctx)
> +{
> +	struct torture_suite *suite = torture_suite_create(ctx, "notify-inotify");
> +
> +	suite->description = talloc_strdup(suite, "SMB2-NOTIFY tests that use inotify");
> +
> +	torture_suite_add_2smb2_test(suite, "inotify-rename", torture_smb2_inotify_rename);
> +
> +	return suite;
> +}
> diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
> index 352a36e..344cf5a 100644
> --- a/source4/torture/smb2/smb2.c
> +++ b/source4/torture/smb2/smb2.c
> @@ -155,6 +155,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx)
>  	torture_suite_add_suite(suite, torture_smb2_create_init(suite));
>  	torture_suite_add_suite(suite, torture_smb2_acls_init(suite));
>  	torture_suite_add_suite(suite, torture_smb2_notify_init(suite));
> +	torture_suite_add_suite(suite, torture_smb2_notify_inotify_init(suite));
>  	torture_suite_add_suite(suite,
>  		torture_smb2_notify_disabled_init(suite));
>  	torture_suite_add_suite(suite, torture_smb2_durable_open_init(suite));
> -- 
> 2.9.4
>