Review: Needs Fixing
Diff comments: > diff --git a/curtin/commands/block_meta_v2.py > b/curtin/commands/block_meta_v2.py > index c1e3630..fd87acc 100644 > --- a/curtin/commands/block_meta_v2.py > +++ b/curtin/commands/block_meta_v2.py > @@ -351,18 +356,7 @@ def disk_handler_v2(info, storage_config, handlers): > if needs_resize(storage_config, action, part_info): It feels like needs_resize and _prepare_resize are kinda duplicating each other now, would it make sense to have this more like wipes where _wipe_for_action just returns None if no wipe is needed? Doesn't have to be in this branch of course. > resizes[entry.start] = _prepare_resize(entry, part_info, > table) > preserved_offsets.add(entry.start) > - wipe = wipes[entry.start] = _wipe_for_action(action) > - if wipe is not None: > - # We do a quick wipe of where any new partitions will be, > - # because if there is bcache or other metadata there, this > - # can cause the partition to be used by a storage > - # subsystem and preventing the exclusive open done by the > - # wipe_volume call below. See > - # https://bugs.launchpad.net/curtin/+bug/1718699 for all > - # the gory details. > - wipe_offset = table.sectors2bytes(entry.start) > - LOG.debug('Wiping 1M on %s at offset %s', disk, wipe_offset) > - block.zero_file_at_offsets(disk, [wipe_offset], exclusive=False) > + wipes[entry.start] = _wipe_for_action(action) > > for kname, nr, offset, size in block.sysfs_partition_data(disk): > offset_sectors = table.bytes2sectors(offset) > @@ -371,19 +365,28 @@ def disk_handler_v2(info, storage_config, handlers): > block.wipe_volume(block.kname_to_path(kname), 'superblock') > resize = resizes.get(offset_sectors) > if resize and size > resize['end']: > - perform_resize(kname, resize['end'], 'down') > + perform_resize(kname, resize, 'down') > So I think the pre-sfdisk wipe needs to go in a new loop here. Adding it to the loop above will probably work _most_ of the time but I think sysfs_partition_data returns the partitions in essentially the textual sort order of the partition nodes so that's not always the same as offset order. > table.apply(disk) > > for kname, number, offset, size in block.sysfs_partition_data(disk): > offset_sectors = table.bytes2sectors(offset) > - mode = wipes[offset_sectors] > - if mode is not None: > + wipe = wipes[offset_sectors] > + if wipe is not None: > + # We do a quick wipe of where any new partitions will be, > + # because if there is bcache or other metadata there, this > + # can cause the partition to be used by a storage > + # subsystem and preventing the exclusive open done by the > + # wipe_volume call below. See > + # https://bugs.launchpad.net/curtin/+bug/1718699 for all > + # the gory details. > + LOG.debug('Wiping 1M on %s at offset %s', disk, offset) > + block.zero_file_at_offsets(disk, [offset], exclusive=False) This is too late, it has to happen before the table.apply() call. > # Wipe the new partitions as needed. > - block.wipe_volume(block.kname_to_path(kname), mode) > + block.wipe_volume(block.kname_to_path(kname), wipe) > resize = resizes.get(offset_sectors) > if resize and resize['start'] < size: > - perform_resize(kname, resize['end'], 'up') > + perform_resize(kname, resize, 'up') > > # Make the names if needed > if 'name' in info: -- https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/419640 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : curtin-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~curtin-dev More help : https://help.launchpad.net/ListHelp
