Message ID | 20200218103454.296704-19-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
Series | block: Fix check_to_replace_node() | expand |
Am 18.02.2020 um 11:34 hat Max Reitz geschrieben: > Add two tests to see that you cannot replace a Quorum child with the > mirror job while the child is in use by a different parent. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/041 | 70 +++++++++++++++++++++++++++++++++++++- > tests/qemu-iotests/041.out | 4 +-- > 2 files changed, 71 insertions(+), 3 deletions(-) > > diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > index 1d9e64ff6d..53c8671969 100755 > --- a/tests/qemu-iotests/041 > +++ b/tests/qemu-iotests/041 > @@ -20,6 +20,7 @@ > > import time > import os > +import re > import iotests > from iotests import qemu_img, qemu_io > > @@ -34,6 +35,8 @@ quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img') > quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img') > quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img') > > +nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock') > + > class TestSingleDrive(iotests.QMPTestCase): > image_len = 1 * 1024 * 1024 # MB > qmp_cmd = 'drive-mirror' > @@ -892,7 +895,8 @@ class TestRepairQuorum(iotests.QMPTestCase): > > def tearDown(self): > self.vm.shutdown() > - for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]: > + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file, > + nbd_sock_path ]: > # Do a try/except because the test may have deleted some images > try: > os.remove(i) > @@ -1032,6 +1036,70 @@ class TestRepairQuorum(iotests.QMPTestCase): > self.assert_has_block_node("repair0", quorum_repair_img) > self.vm.assert_block_path('quorum0', '/children.1', 'repair0') > > + def test_with_other_parent(self): > + """ > + Check that we cannot replace a Quorum child when it has other > + parents. > + """ > + result = self.vm.qmp('nbd-server-start', > + addr={ > + 'type': 'unix', > + 'data': {'path': nbd_sock_path} > + }) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('nbd-server-add', device='img1') > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', > + sync='full', node_name='repair0', replaces='img1', > + target=quorum_repair_img, format=iotests.imgfmt) > + self.assert_qmp(result, 'error/desc', > + "Cannot replace 'img1' by a node mirrored from " > + "'quorum0', because it cannot be guaranteed that doing " > + "so would not lead to an abrupt change of visible data") > + > + def test_with_other_parents_after_mirror_start(self): > + """ > + The same as test_with_other_parent(), but add the NBD server > + only when the mirror job is already running. > + """ > + result = self.vm.qmp('nbd-server-start', > + addr={ > + 'type': 'unix', > + 'data': {'path': nbd_sock_path} > + }) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', > + sync='full', node_name='repair0', replaces='img1', > + target=quorum_repair_img, format=iotests.imgfmt) > + self.assert_qmp(result, 'return', {}) > + > + result = self.vm.qmp('nbd-server-add', device='img1') > + self.assert_qmp(result, 'return', {}) > + > + # The full error message goes to stderr, we will check it later > + self.complete_and_wait('mirror', > + completion_error='Operation not permitted') > + > + # Should not have been replaced > + self.vm.assert_block_path('quorum0', '/children.1', 'img1') > + > + # Check the full error message now > + self.vm.shutdown() > + log = self.vm.get_log() > + log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) > + log = re.sub(r'^Formatting.*\n', '', log) > + log = re.sub(r'\n\[I \+\d+\.\d+\] CLOSED\n?$', '', log) > + log = re.sub(r'^qemu-system-[^:]*: ', '', log) I would have applied the series, but: +.........................F.................................................................... +====================================================================== +FAIL: test_with_other_parents_after_mirror_start (__main__.TestRepairQuorum) +---------------------------------------------------------------------- +Traceback (most recent call last): + File "041", line 1099, in test_with_other_parents_after_mirror_start + "it can no longer be guaranteed that doing so would " + +AssertionError: "qemu: Can no longer replace 'img1' by 're[107 chars]data" != "Can no longer replace 'img1' by 'repair0'[101 chars]data" +- qemu: Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data +? ------ ++ Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data + + If you agree, I can just change this line while applying into: log = re.sub(r'^qemu[^:]*: ', '', log) Kevin
On 18.02.20 13:38, Kevin Wolf wrote: > Am 18.02.2020 um 11:34 hat Max Reitz geschrieben: >> Add two tests to see that you cannot replace a Quorum child with the >> mirror job while the child is in use by a different parent. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> tests/qemu-iotests/041 | 70 +++++++++++++++++++++++++++++++++++++- >> tests/qemu-iotests/041.out | 4 +-- >> 2 files changed, 71 insertions(+), 3 deletions(-) >> >> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 >> index 1d9e64ff6d..53c8671969 100755 >> --- a/tests/qemu-iotests/041 >> +++ b/tests/qemu-iotests/041 >> @@ -20,6 +20,7 @@ >> >> import time >> import os >> +import re >> import iotests >> from iotests import qemu_img, qemu_io >> >> @@ -34,6 +35,8 @@ quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img') >> quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img') >> quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img') >> >> +nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock') >> + >> class TestSingleDrive(iotests.QMPTestCase): >> image_len = 1 * 1024 * 1024 # MB >> qmp_cmd = 'drive-mirror' >> @@ -892,7 +895,8 @@ class TestRepairQuorum(iotests.QMPTestCase): >> >> def tearDown(self): >> self.vm.shutdown() >> - for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]: >> + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file, >> + nbd_sock_path ]: >> # Do a try/except because the test may have deleted some images >> try: >> os.remove(i) >> @@ -1032,6 +1036,70 @@ class TestRepairQuorum(iotests.QMPTestCase): >> self.assert_has_block_node("repair0", quorum_repair_img) >> self.vm.assert_block_path('quorum0', '/children.1', 'repair0') >> >> + def test_with_other_parent(self): >> + """ >> + Check that we cannot replace a Quorum child when it has other >> + parents. >> + """ >> + result = self.vm.qmp('nbd-server-start', >> + addr={ >> + 'type': 'unix', >> + 'data': {'path': nbd_sock_path} >> + }) >> + self.assert_qmp(result, 'return', {}) >> + >> + result = self.vm.qmp('nbd-server-add', device='img1') >> + self.assert_qmp(result, 'return', {}) >> + >> + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', >> + sync='full', node_name='repair0', replaces='img1', >> + target=quorum_repair_img, format=iotests.imgfmt) >> + self.assert_qmp(result, 'error/desc', >> + "Cannot replace 'img1' by a node mirrored from " >> + "'quorum0', because it cannot be guaranteed that doing " >> + "so would not lead to an abrupt change of visible data") >> + >> + def test_with_other_parents_after_mirror_start(self): >> + """ >> + The same as test_with_other_parent(), but add the NBD server >> + only when the mirror job is already running. >> + """ >> + result = self.vm.qmp('nbd-server-start', >> + addr={ >> + 'type': 'unix', >> + 'data': {'path': nbd_sock_path} >> + }) >> + self.assert_qmp(result, 'return', {}) >> + >> + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', >> + sync='full', node_name='repair0', replaces='img1', >> + target=quorum_repair_img, format=iotests.imgfmt) >> + self.assert_qmp(result, 'return', {}) >> + >> + result = self.vm.qmp('nbd-server-add', device='img1') >> + self.assert_qmp(result, 'return', {}) >> + >> + # The full error message goes to stderr, we will check it later >> + self.complete_and_wait('mirror', >> + completion_error='Operation not permitted') >> + >> + # Should not have been replaced >> + self.vm.assert_block_path('quorum0', '/children.1', 'img1') >> + >> + # Check the full error message now >> + self.vm.shutdown() >> + log = self.vm.get_log() >> + log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) >> + log = re.sub(r'^Formatting.*\n', '', log) >> + log = re.sub(r'\n\[I \+\d+\.\d+\] CLOSED\n?$', '', log) >> + log = re.sub(r'^qemu-system-[^:]*: ', '', log) > > I would have applied the series, but: > > +.........................F.................................................................... > +====================================================================== > +FAIL: test_with_other_parents_after_mirror_start (__main__.TestRepairQuorum) > +---------------------------------------------------------------------- > +Traceback (most recent call last): > + File "041", line 1099, in test_with_other_parents_after_mirror_start > + "it can no longer be guaranteed that doing so would " + > +AssertionError: "qemu: Can no longer replace 'img1' by 're[107 chars]data" != "Can no longer replace 'img1' by 'repair0'[101 chars]data" > +- qemu: Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data > +? ------ > ++ Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data > + > + > > If you agree, I can just change this line while applying into: > > log = re.sub(r'^qemu[^:]*: ', '', log) Sure, thanks! I just hope noone uses the QEMU environment variable to point to something that isn’t prefixed by “qemu”... Max
Am 18.02.2020 um 13:49 hat Max Reitz geschrieben: > On 18.02.20 13:38, Kevin Wolf wrote: > > Am 18.02.2020 um 11:34 hat Max Reitz geschrieben: > >> Add two tests to see that you cannot replace a Quorum child with the > >> mirror job while the child is in use by a different parent. > >> > >> Signed-off-by: Max Reitz <mreitz@redhat.com> > >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >> --- > >> tests/qemu-iotests/041 | 70 +++++++++++++++++++++++++++++++++++++- > >> tests/qemu-iotests/041.out | 4 +-- > >> 2 files changed, 71 insertions(+), 3 deletions(-) > >> > >> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 > >> index 1d9e64ff6d..53c8671969 100755 > >> --- a/tests/qemu-iotests/041 > >> +++ b/tests/qemu-iotests/041 > >> @@ -20,6 +20,7 @@ > >> > >> import time > >> import os > >> +import re > >> import iotests > >> from iotests import qemu_img, qemu_io > >> > >> @@ -34,6 +35,8 @@ quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img') > >> quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img') > >> quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img') > >> > >> +nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock') > >> + > >> class TestSingleDrive(iotests.QMPTestCase): > >> image_len = 1 * 1024 * 1024 # MB > >> qmp_cmd = 'drive-mirror' > >> @@ -892,7 +895,8 @@ class TestRepairQuorum(iotests.QMPTestCase): > >> > >> def tearDown(self): > >> self.vm.shutdown() > >> - for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]: > >> + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file, > >> + nbd_sock_path ]: > >> # Do a try/except because the test may have deleted some images > >> try: > >> os.remove(i) > >> @@ -1032,6 +1036,70 @@ class TestRepairQuorum(iotests.QMPTestCase): > >> self.assert_has_block_node("repair0", quorum_repair_img) > >> self.vm.assert_block_path('quorum0', '/children.1', 'repair0') > >> > >> + def test_with_other_parent(self): > >> + """ > >> + Check that we cannot replace a Quorum child when it has other > >> + parents. > >> + """ > >> + result = self.vm.qmp('nbd-server-start', > >> + addr={ > >> + 'type': 'unix', > >> + 'data': {'path': nbd_sock_path} > >> + }) > >> + self.assert_qmp(result, 'return', {}) > >> + > >> + result = self.vm.qmp('nbd-server-add', device='img1') > >> + self.assert_qmp(result, 'return', {}) > >> + > >> + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', > >> + sync='full', node_name='repair0', replaces='img1', > >> + target=quorum_repair_img, format=iotests.imgfmt) > >> + self.assert_qmp(result, 'error/desc', > >> + "Cannot replace 'img1' by a node mirrored from " > >> + "'quorum0', because it cannot be guaranteed that doing " > >> + "so would not lead to an abrupt change of visible data") > >> + > >> + def test_with_other_parents_after_mirror_start(self): > >> + """ > >> + The same as test_with_other_parent(), but add the NBD server > >> + only when the mirror job is already running. > >> + """ > >> + result = self.vm.qmp('nbd-server-start', > >> + addr={ > >> + 'type': 'unix', > >> + 'data': {'path': nbd_sock_path} > >> + }) > >> + self.assert_qmp(result, 'return', {}) > >> + > >> + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', > >> + sync='full', node_name='repair0', replaces='img1', > >> + target=quorum_repair_img, format=iotests.imgfmt) > >> + self.assert_qmp(result, 'return', {}) > >> + > >> + result = self.vm.qmp('nbd-server-add', device='img1') > >> + self.assert_qmp(result, 'return', {}) > >> + > >> + # The full error message goes to stderr, we will check it later > >> + self.complete_and_wait('mirror', > >> + completion_error='Operation not permitted') > >> + > >> + # Should not have been replaced > >> + self.vm.assert_block_path('quorum0', '/children.1', 'img1') > >> + > >> + # Check the full error message now > >> + self.vm.shutdown() > >> + log = self.vm.get_log() > >> + log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) > >> + log = re.sub(r'^Formatting.*\n', '', log) > >> + log = re.sub(r'\n\[I \+\d+\.\d+\] CLOSED\n?$', '', log) > >> + log = re.sub(r'^qemu-system-[^:]*: ', '', log) > > > > I would have applied the series, but: > > > > +.........................F.................................................................... > > +====================================================================== > > +FAIL: test_with_other_parents_after_mirror_start (__main__.TestRepairQuorum) > > +---------------------------------------------------------------------- > > +Traceback (most recent call last): > > + File "041", line 1099, in test_with_other_parents_after_mirror_start > > + "it can no longer be guaranteed that doing so would " + > > +AssertionError: "qemu: Can no longer replace 'img1' by 're[107 chars]data" != "Can no longer replace 'img1' by 'repair0'[101 chars]data" > > +- qemu: Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data > > +? ------ > > ++ Can no longer replace 'img1' by 'repair0', because it can no longer be guaranteed that doing so would not lead to an abrupt change of visible data > > + > > + > > > > If you agree, I can just change this line while applying into: > > > > log = re.sub(r'^qemu[^:]*: ', '', log) > > Sure, thanks! I just hope noone uses the QEMU environment variable to > point to something that isn’t prefixed by “qemu”... Ok, I'll use this instead: + log = re.sub(r'^%s[^:]*: ' % os.path.basename(iotests.qemu_prog), + '', log) Kevin
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 1d9e64ff6d..53c8671969 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -20,6 +20,7 @@ import time import os +import re import iotests from iotests import qemu_img, qemu_io @@ -34,6 +35,8 @@ quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img') quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img') quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img') +nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock') + class TestSingleDrive(iotests.QMPTestCase): image_len = 1 * 1024 * 1024 # MB qmp_cmd = 'drive-mirror' @@ -892,7 +895,8 @@ class TestRepairQuorum(iotests.QMPTestCase): def tearDown(self): self.vm.shutdown() - for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]: + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file, + nbd_sock_path ]: # Do a try/except because the test may have deleted some images try: os.remove(i) @@ -1032,6 +1036,70 @@ class TestRepairQuorum(iotests.QMPTestCase): self.assert_has_block_node("repair0", quorum_repair_img) self.vm.assert_block_path('quorum0', '/children.1', 'repair0') + def test_with_other_parent(self): + """ + Check that we cannot replace a Quorum child when it has other + parents. + """ + result = self.vm.qmp('nbd-server-start', + addr={ + 'type': 'unix', + 'data': {'path': nbd_sock_path} + }) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('nbd-server-add', device='img1') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', + sync='full', node_name='repair0', replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/desc', + "Cannot replace 'img1' by a node mirrored from " + "'quorum0', because it cannot be guaranteed that doing " + "so would not lead to an abrupt change of visible data") + + def test_with_other_parents_after_mirror_start(self): + """ + The same as test_with_other_parent(), but add the NBD server + only when the mirror job is already running. + """ + result = self.vm.qmp('nbd-server-start', + addr={ + 'type': 'unix', + 'data': {'path': nbd_sock_path} + }) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('drive-mirror', job_id='mirror', device='quorum0', + sync='full', node_name='repair0', replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('nbd-server-add', device='img1') + self.assert_qmp(result, 'return', {}) + + # The full error message goes to stderr, we will check it later + self.complete_and_wait('mirror', + completion_error='Operation not permitted') + + # Should not have been replaced + self.vm.assert_block_path('quorum0', '/children.1', 'img1') + + # Check the full error message now + self.vm.shutdown() + log = self.vm.get_log() + log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) + log = re.sub(r'^Formatting.*\n', '', log) + log = re.sub(r'\n\[I \+\d+\.\d+\] CLOSED\n?$', '', log) + log = re.sub(r'^qemu-system-[^:]*: ', '', log) + + self.assertEqual(log, + "Can no longer replace 'img1' by 'repair0', because " + + "it can no longer be guaranteed that doing so would " + + "not lead to an abrupt change of visible data") + + # Test mirroring with a source that does not have any parents (not even a # BlockBackend) class TestOrphanedSource(iotests.QMPTestCase): diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index f496be9197..ffc779b4d1 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -........................................................................................... +............................................................................................. ---------------------------------------------------------------------- -Ran 91 tests +Ran 93 tests OK