Merge lp:~raharper/curtin/trunk.block-wipe-exclusive-optional into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

block: enable control over exclusive_open use when wiping volumes

The exclusive_open check on a device is very useful to determine if
curtin is removing all metadata from a device w.r.t storage
configuraiton. However, in some cases during creation of new storage
configations when curtin attempts to add additional partitions to a
device that has partitions already added to an raid device, the
exclusive open is blocked while we attempt to zero some data on the
underlying device. The Mirrorboot-UEFI vmtest exemplifies this
situation.

This patch exposes a flag to the zero_file_at_offset method to allow
callers (block_meta in this case) to disable the exclusive_open. This
lets curtin wipe data at specific offsets on a volume without
requiring exclusive ownership of the device.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:539
https://jenkins.ubuntu.com/server/job/curtin-ci/670/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/670
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/670
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/670
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/670

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/670/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

One nit, but looks good.

review: Approve

[lp:~raharper/curtin/trunk.block-wipe-exclusive-optional]

540. By Ryan Harper on 2017-10-30

Update function docstring; accept boolean-ish values for exclusive parameter

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:540
https://jenkins.ubuntu.com/server/job/curtin-ci/671/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/671
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/671
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/671
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/671

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/671/rebuild

review: Approve (continuous-integration)

=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py	2017-06-19 19:22:42 +0000
+++ curtin/block/__init__.py	2017-10-30 14:51:57 +0000
@@ -776,17 +776,21 @@
 
 
 @contextmanager
-def exclusive_open(path):
+def exclusive_open(path, exclusive=True):
     """
-    Obtain an exclusive file-handle to the file/device specified
+    Obtain an exclusive file-handle to the file/device specified unless
+    caller specifics exclusive=False.
     """
     mode = 'rb+'
     fd = None
     if not os.path.exists(path):
         raise ValueError("No such file at path: %s" % path)
 
+    flags = os.O_RDWR
+    if exclusive:
+        flags += os.O_EXCL
     try:
-        fd = os.open(path, os.O_RDWR | os.O_EXCL)
+        fd = os.open(path, flags)
         try:
             fd_needs_closing = True
             with os.fdopen(fd, mode) as fo:
@@ -875,7 +879,8 @@
     return zero_file_at_offsets(path, offsets, buflen=buflen, count=count)
 
 
-def zero_file_at_offsets(path, offsets, buflen=1024, count=1024, strict=False):
+def zero_file_at_offsets(path, offsets, buflen=1024, count=1024, strict=False,
+                         exclusive=True):
     """
     write zeros to file at specified offsets
     """
@@ -890,7 +895,8 @@
     tot = buflen * count
     msg_vals = {'path': path, 'tot': buflen * count}
 
-    with exclusive_open(path) as fp:
+    # allow caller to control if we require exclusive open
+    with exclusive_open(path, exclusive=exclusive) as fp:
         # get the size by seeking to end.
         fp.seek(0, 2)
         size = fp.tell()

=== modified file 'curtin/commands/block_meta.py'
--- curtin/commands/block_meta.py	2017-10-23 16:49:46 +0000
+++ curtin/commands/block_meta.py	2017-10-30 14:51:57 +0000
@@ -561,7 +561,10 @@
             # length of the previous partition
             wipe_offset = int(offset_sectors * logical_block_size_bytes)
             LOG.debug('Wiping 1M on %s at offset %s', disk, wipe_offset)
-            block.zero_file_at_offsets(disk, [wipe_offset])
+            # We don't require exclusive access as we're wiping data at an
+            # offset and the current holder maybe part of the current storage
+            # configuration.
+            block.zero_file_at_offsets(disk, [wipe_offset], exclusive=False)
 
     if disk_ptable == "msdos":
         if flag in ["extended", "logical", "primary"]:

=== modified file 'tests/unittests/test_commands_block_meta.py'
--- tests/unittests/test_commands_block_meta.py	2017-10-23 16:49:46 +0000
+++ tests/unittests/test_commands_block_meta.py	2017-10-30 14:51:57 +0000
@@ -131,7 +131,7 @@
         self.add_patch('curtin.block.clear_holders.assert_clear',
                        'mock_assert_clear')
         self.add_patch('curtin.block.zero_file_at_offsets',
-                       'mock_block_zero_file_at_offsets')
+                       'mock_block_zero_file')
 
         self.target = "my_target"
         self.config = {
@@ -197,8 +197,8 @@
         block_meta.partition_handler(part_info, self.storage_config)
 
         part_offset = 2048 * 512
-        self.mock_block_zero_file_at_offsets.assert_called_with(disk_kname,
-                                                                [part_offset])
+        self.mock_block_zero_file.assert_called_with(disk_kname, [part_offset],
+                                                     exclusive=False)
         self.mock_subp.assert_called_with(['parted', disk_kname, '--script',
                                            'mkpart', 'primary', '2048s',
                                            '1001471s'], capture=True)