Merge lp:~strikov-deactivatedaccount/curtin/multipath-2 into lp:~curtin-dev/curtin/trunk

Proposed by Scott Moser
Status: Merged
Merged at revision: 211
Proposed branch: lp:~strikov-deactivatedaccount/curtin/multipath-2
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 141 lines (+95/-0)
3 files modified
curtin/block/__init__.py (+53/-0)
curtin/commands/block_meta.py (+5/-0)
curtin/commands/curthooks.py (+37/-0)
To merge this branch: bzr merge lp:~strikov-deactivatedaccount/curtin/multipath-2
Reviewer Review Type Date Requested Status
curtin developers Pending
Review via email: mp+261121@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

So, some thoughts here.
a.) we're using 'strip' on the output of /lib/udev/scsi_id which is less than ideal. that was done to drop the trailing '\n'. the result is that we'd recognize ' DISK 1 ' and 'DISK 1' as the same device and take the "enable_multipath" path. Both not very likely and not very big deal, but odd. I guess probably we should:
  if len(scsi_id) and scsi_id[-1] == '\n':
     scsi_id = scsi_id[0:-1]

  not a big deal, but worth fixing

b.) the other hting i'm concerned about is if the image / environment that we're installing in has multipath-tools installed, it may see that /dev/sda (and /dev/sde) is used by /dev/dm-0. and then choose not to install to it, or not realize that and fail to install.

in that scenario, i think the first install possibly works, while subsequent ones fail.
we may need a way to un-setup the dm devices so we can install to the "raw" ones.

Revision history for this message
Scott Moser (smoser) wrote :

oh yeah, and 'c'
i'm not sure if this works or not in python3.
a test would be nice to make sure.

211. By Oleg Strikov

block: multipath: Do not strip scsi_id, just remove trailing newlines.

Two devices may have similar scsi_ids with the only difference in the
exact amount of spaces in the beginning or in the end. By using strip()
we treat these devices as multipath ones. That's wrong because multipath
assumes identical scsi_ids of the devices.

212. By Oleg Strikov

block_meta: Stop all multipath devices before installing the system.

Multipath devices might be automatically assembled if multipath-tools
package is available in the installation environment. We need to stop
all multipath devices to exclusively use one of paths as a target disk.

213. By Oleg Strikov

block, block_meta: Stylistic changes.

214. By Oleg Strikov

curthooks: multipath: Few changes to the way multipath.conf is generated.

a) Don't overwrite multipath.conf provided by the installation image.
b) Add header to the file which state that file was generated by curtin.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py 2015-05-13 13:30:56 +0000
+++ curtin/block/__init__.py 2015-06-05 16:58:55 +0000
@@ -22,6 +22,7 @@
22import tempfile22import tempfile
2323
24from curtin import util24from curtin import util
25from curtin.log import LOG
2526
2627
27def get_dev_name_entry(devname):28def get_dev_name_entry(devname):
@@ -179,6 +180,58 @@
179 return ret180 return ret
180181
181182
183def stop_all_unused_multipath_devices():
184 """
185 Stop all unused multipath devices.
186 """
187 multipath = util.which('multipath')
188
189 # Command multipath is not available only when multipath-tools package
190 # is not installed. Nothing needs to be done in this case because system
191 # doesn't create multipath devices without this package installed and we
192 # have nothing to stop.
193 if not multipath:
194 return
195
196 # Command multipath -F flushes all unused multipath device maps
197 cmd = [multipath, '-F']
198 try:
199 util.subp(cmd)
200 except util.ProcessExecutionError as e:
201 LOG.warn("Failed to stop multipath devices: %s", e)
202
203
204def detect_multipath():
205 """
206 Figure out if target machine has any multipath devices.
207 """
208 # Multipath-tools are not available in the ephemeral image.
209 # This workaround detects multipath by looking for multiple
210 # devices with the same scsi id (serial number).
211 disk_ids = []
212 bdinfo = _lsblock(['--nodeps'])
213 for devname, data in bdinfo.items():
214 # Command scsi_id returns error while running against cdrom devices.
215 # To prevent getting unexpected errors for some other types of devices
216 # we ignore everything except hard disks.
217 if data['TYPE'] != 'disk':
218 continue
219
220 cmd = ['/lib/udev/scsi_id', '--replace-whitespace',
221 '--whitelisted', '--device=%s' % data['device_path']]
222 try:
223 (out, err) = util.subp(cmd, capture=True)
224 scsi_id = out.rstrip('\n')
225 # ignore empty ids because they are meaningless
226 if scsi_id:
227 disk_ids.append(scsi_id)
228 except util.ProcessExecutionError as e:
229 LOG.warn("Failed to get disk id: %s", e)
230
231 duplicates_found = (len(disk_ids) != len(set(disk_ids)))
232 return duplicates_found
233
234
182def get_root_device(dev, fpath="curtin"):235def get_root_device(dev, fpath="curtin"):
183 """236 """
184 Get root partition for specified device, based on presence of /curtin.237 Get root partition for specified device, based on presence of /curtin.
185238
=== modified file 'curtin/commands/block_meta.py'
--- curtin/commands/block_meta.py 2015-05-07 03:58:10 +0000
+++ curtin/commands/block_meta.py 2015-06-05 16:58:55 +0000
@@ -146,6 +146,11 @@
146 # Remove duplicates but maintain ordering.146 # Remove duplicates but maintain ordering.
147 devices = list(OrderedDict.fromkeys(devices))147 devices = list(OrderedDict.fromkeys(devices))
148148
149 # Multipath devices might be automatically assembled if multipath-tools
150 # package is available in the installation environment. We need to stop
151 # all multipath devices to exclusively use one of paths as a target disk.
152 block.stop_all_unused_multipath_devices()
153
149 if len(devices) == 0:154 if len(devices) == 0:
150 devices = block.get_installable_blockdevs()155 devices = block.get_installable_blockdevs()
151 LOG.warn("'%s' mode, no devices given. unused list: %s",156 LOG.warn("'%s' mode, no devices given. unused list: %s",
152157
=== modified file 'curtin/commands/curthooks.py'
--- curtin/commands/curthooks.py 2015-03-11 17:03:33 +0000
+++ curtin/commands/curthooks.py 2015-06-05 16:58:55 +0000
@@ -419,6 +419,41 @@
419 maxsize=maxsize)419 maxsize=maxsize)
420420
421421
422def detect_and_handle_multipath(cfg, target):
423 DEFAULT_MULTIPATH_PACKAGES = ['multipath-tools-boot']
424 mpcfg = cfg.get('multipath', {})
425 mpmode = mpcfg.get('mode', 'auto')
426 mppkgs = mpcfg.get('packages', DEFAULT_MULTIPATH_PACKAGES)
427 if isinstance(mppkgs, str):
428 mppkgs = [mppkgs]
429
430 if mpmode == 'disabled':
431 return
432
433 if mpmode == 'auto' and not block.detect_multipath():
434 return
435
436 util.install_packages(mppkgs, target=target)
437
438 multipath_cfg_path = os.path.sep.join([target, '/etc/multipath.conf'])
439 # We don't want to overwrite multipath.conf file provided by the image.
440 if os.path.isfile(multipath_cfg_path):
441 return
442
443 # Without user_friendly_names option enabled system fails to boot
444 # if any of the disks has spaces in its name. Package multipath-tools
445 # has bug opened for this issue (LP: 1432062) but it was not fixed yet.
446 multipath_cfg_content = '\n'.join(
447 ['# This file was generated by curtin while installing the system.',
448 'defaults {',
449 ' user_friendly_names yes',
450 '}',
451 ''])
452 util.write_file(multipath_cfg_path, content=multipath_cfg_content)
453 # Initrams needs to be regenerated to include updated multipath.cfg
454 update_initramfs(target)
455
456
422def curthooks(args):457def curthooks(args):
423 state = util.load_command_environment()458 state = util.load_command_environment()
424459
@@ -452,6 +487,8 @@
452 copy_interfaces(state.get('interfaces'), target)487 copy_interfaces(state.get('interfaces'), target)
453 copy_fstab(state.get('fstab'), target)488 copy_fstab(state.get('fstab'), target)
454489
490 detect_and_handle_multipath(cfg, target)
491
455 # As a rule, ARMv7 systems don't use grub. This may change some492 # As a rule, ARMv7 systems don't use grub. This may change some
456 # day, but for now, assume no. They do require the initramfs493 # day, but for now, assume no. They do require the initramfs
457 # to be updated, and this also triggers boot loader setup via494 # to be updated, and this also triggers boot loader setup via

Subscribers

People subscribed via source and target branches