ceph-disk: don't change the journal partition uuid

We observe that the new /dev/disk/by-partuuid/<journal_uuid>
symlink is not always created by udev when reusing a journal
partition. Fix by not changing the uuid of a journal partition
in this case -- instead we can reuse the existing uuid (and
journal_symlink) instead. We also now assert that the symlink
exists before further preparing the OSD.

Fixes: ceph#10146
Signed-off-by: Dan van der Ster <[email protected]>
Tested-by: Dan van der Ster <[email protected]>

src/ceph-disk

@@ -1056,30 +1056,47 @@ def prepare_journal_dev(
         if get_partition_type(journal) == JOURNAL_UUID:
             LOG.debug('Journal %s was previously prepared with ceph-disk. Reusing it.', journal)
             reusing_partition = True
-            base = get_partition_base(journal)
-            part = journal.replace(base,'')
-            journal = base # needed for later
+            # Read and reuse the partition uuid from this journal's previous life.
+            # We reuse the uuid instead of changing it because udev does not reliably
+            # notice changes to an existing partition's GUID.
+            # See http://tracker.ceph.com/issues/10146
+            journal_uuid = get_partition_uuid(journal)
+            LOG.debug('Reusing journal with uuid %s', journal_uuid)
         else:
             LOG.warning('Journal %s was not prepared with ceph-disk. Symlinking directly.', journal)
             return (journal, None, None)
 
+    journal_symlink = '/dev/disk/by-partuuid/{journal_uuid}'.format(
+        journal_uuid=journal_uuid,
+        )
+
+    journal_dmcrypt = None
+    if journal_dm_keypath:
+        journal_dmcrypt = journal_symlink
+        journal_symlink = '/dev/mapper/{uuid}'.format(uuid=journal_uuid)
+
+    if reusing_partition:
+        # confirm that the journal_symlink exists. It should since this was an active journal
+        # in the past. Continuing otherwise would be futile.
+        assert os.path.exists(journal_symlink)
+        return (journal_symlink, journal_dmcrypt, journal_uuid)
+
+    # From here on we are creating a new journal device, not reusing.
+
     ptype = JOURNAL_UUID
     if journal_dm_keypath:
         ptype = DMCRYPT_JOURNAL_UUID
 
     # it is a whole disk.  create a partition!
     num = None
-    if journal == data and not reusing_partition:
+    if journal == data:
         # we're sharing the disk between osd data and journal;
         # make journal be partition number 2, so it's pretty
         num = 2
         journal_part = '{num}:0:{size}M'.format(
             num=num,
             size=journal_size,
             )
-    elif reusing_partition:
-        num = int(part)
-        journal_part = '' # not used in this case
     else:
         # sgdisk has no way for me to say "whatever is the next
         # free index number" when setting type guids etc, so we
@@ -1094,10 +1111,7 @@ def prepare_journal_dev(
             )
         LOG.warning('OSD will not be hot-swappable if journal is not the same device as the osd data')
 
-    if reusing_partition:
-        dev_size = get_dev_size(base+part)
-    else:
-        dev_size = get_dev_size(journal)
+    dev_size = get_dev_size(journal)
 
     if journal_size > dev_size:
         LOG.error('refusing to create journal on %s' % journal)
@@ -1107,7 +1121,9 @@ def prepare_journal_dev(
         )
 
     try:
-        sgdisk_call = [
+        LOG.debug('Creating journal partition num %d size %d on %s', num, journal_size, journal)
+        command_check_call(
+            [
                 'sgdisk',
                 '--new={part}'.format(part=journal_part),
                 '--change-name={num}:ceph journal'.format(num=num),
@@ -1122,14 +1138,8 @@ def prepare_journal_dev(
                 '--mbrtogpt',
                 '--',
                 journal,
-            ]
-        if reusing_partition:
-            action= 'Reusing'
-            del sgdisk_call[1] # don't add --new when reusing
-        else:
-            action = 'Creating'
-        LOG.debug('%s journal partition num %d size %d on %s', action, num, journal_size, journal)
-        command_check_call(sgdisk_call)
+                ]
+            )
 
         update_partition('-a', journal, 'prepared')
 
@@ -1141,16 +1151,11 @@ def prepare_journal_dev(
                 ],
             )
 
-        journal_symlink = '/dev/disk/by-partuuid/{journal_uuid}'.format(
-            journal_uuid=journal_uuid,
-            )
+        LOG.debug('Journal is GPT partition %s', journal_symlink)
 
-        journal_dmcrypt = None
-        if journal_dm_keypath:
-            journal_dmcrypt = journal_symlink
-            journal_symlink = '/dev/mapper/{uuid}'.format(uuid=journal_uuid)
+        # udev should have created the symlink by now. If not, abort.
+        assert os.path.exists(journal_symlink)
 
-        LOG.debug('Journal is GPT partition %s', journal_symlink)
         return (journal_symlink, journal_dmcrypt, journal_uuid)
 
     except subprocess.CalledProcessError as e: