Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SDX 8900 platform #181

Open
wants to merge 6 commits into
base: release/xs8
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions answerfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ def parseCommon(self):
if bl not in ['' , 'grub2']:
raise AnswerfileException("Unsupported bootloader '%s'" % bl)

nodes = getElementsByTagName(self.top_node, ['target-platform'])
if len(nodes) > 0:
results['target-platform'] = getText(nodes[0])

return results

def parseExistingInstallation(self):
Expand Down
69 changes: 44 additions & 25 deletions backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def getPrepSequence(ans, interactive):
Task(util.getUUID, As(ans), ['installation-uuid']),
Task(util.getUUID, As(ans), ['control-domain-uuid']),
Task(util.randomLabelStr, As(ans), ['disk-label-suffix']),
Task(partitionTargetDisk, A(ans, 'primary-disk', 'installation-to-overwrite', 'preserve-first-partition','sr-on-primary'),
Task(partitionTargetDisk, A(ans, 'primary-disk', 'installation-to-overwrite', 'preserve-first-partition','sr-on-primary', 'target-platform'),
['target-boot-mode', 'primary-partnum', 'backup-partnum', 'storage-partnum', 'boot-partnum', 'logs-partnum', 'swap-partnum']),
]

Expand Down Expand Up @@ -531,25 +531,31 @@ def configureNTP(mounts, ntp_config_method, ntp_servers):
# based on options passed and status of disk (like partition to retain).
# This should be used for upgrade or install, not for restore.
# Returns 'target-boot-mode', 'primary-partnum', 'backup-partnum', 'storage-partnum', 'boot-partnum', 'logs-partnum', 'swap-partnum'
def partitionTargetDisk(disk, existing, preserve_first_partition, create_sr_part):
def partitionTargetDisk(disk, existing, preserve_first_partition, create_sr_part, target_platform):
logger.log("Installer booted in %s mode" % ("UEFI" if constants.UEFI_INSTALLER else "legacy"))

(PRIMARY, BACKUP, STORAGE, BOOT, LOGS, SWAP) = list(range(6))

if target_platform == 'sdx8900':
constants.boot_size = 2
constants.root_size = 4096 + 4096 - constants.boot_size - 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use constants.root_mbr_size_old and constants.backup_size_old here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I was thinking the same, then I forgot.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the 1 constant needed for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alignment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a comment


primary_part = 1
if existing:
# upgrade, use existing partitioning scheme
tool = PartitionTool(existing.primary_disk)

primary_part = tool.partitionNumber(existing.root_device)

part_nums = list(range(primary_part, primary_part+6))

# Determine target install's boot mode and boot partition number
target_boot_mode = TARGET_BOOT_MODE_LEGACY
if existing.boot_device:
boot_partnum = tool.partitionNumber(existing.boot_device)
boot_part = tool.getPartition(boot_partnum)
part_nums[BOOT] = tool.partitionNumber(existing.boot_device)
boot_part = tool.getPartition(part_nums[BOOT])
if 'id' in boot_part and boot_part['id'] == GPTPartitionTool.ID_EFI_BOOT:
target_boot_mode = TARGET_BOOT_MODE_UEFI
else:
boot_partnum = primary_part + 3

if (target_boot_mode == TARGET_BOOT_MODE_UEFI and not constants.UEFI_INSTALLER) or \
(target_boot_mode == TARGET_BOOT_MODE_LEGACY and constants.UEFI_INSTALLER):
Expand All @@ -560,10 +566,15 @@ def partitionTargetDisk(disk, existing, preserve_first_partition, create_sr_part

# Return install mode and numbers of primary, backup, SR, boot, log and swap partitions
storage_partition = tool.getPartition(primary_part+2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be changed to ... tool.getPartition(part_nums[STORAGE]) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would make it more readable, indeed.

if storage_partition:
return (target_boot_mode, primary_part, primary_part+1, primary_part+2, boot_partnum, primary_part+4, primary_part+5)
else:
return (target_boot_mode, primary_part, primary_part+1, 0, boot_partnum, primary_part+4, primary_part+5)
if not storage_partition:
part_nums[STORAGE] = 0

if target_platform == 'sdx8900':
part_nums[BACKUP] = 0
part_nums[LOGS] = 0
part_nums[SWAP] = 0

return tuple([target_boot_mode] + part_nums)

tool = PartitionTool(disk)

Expand All @@ -582,17 +593,23 @@ def partitionTargetDisk(disk, existing, preserve_first_partition, create_sr_part
if primary_part > 2:
raise RuntimeError("Installer only supports a single Utility Partition at partition 1, but found Utility Partitions at %s" % str(utilparts))

sr_part = -1
if create_sr_part:
sr_part = primary_part+2
part_nums = list(range(primary_part, primary_part+6))

boot_part = max(primary_part + 1, sr_part) + 1
if not create_sr_part:
part_nums[STORAGE] = -1

part_nums[BOOT] = max(primary_part + 1, part_nums[STORAGE]) + 1

target_boot_mode = TARGET_BOOT_MODE_UEFI if constants.UEFI_INSTALLER else TARGET_BOOT_MODE_LEGACY

logger.log("Fresh install, target_boot_mode: %s" % target_boot_mode)

return (target_boot_mode, primary_part, primary_part + 1, sr_part, boot_part, primary_part + 4, primary_part + 5)
if target_platform == 'sdx8900':
part_nums[BACKUP] = 0
part_nums[LOGS] = 0
part_nums[SWAP] = 0

return tuple([target_boot_mode] + part_nums)

def removeBlockingVGs(disks):
for vg in diskutil.findProblematicVGs(disks):
Expand Down Expand Up @@ -636,15 +653,16 @@ def writeDom0DiskPartitions(disk, target_boot_mode, boot_partnum, primary_partnu
# Create logs partition
# Start the first partition at 1 MiB if there are no other partitions.
# Otherwise start the partition following the utility partition.
if order == 1:
tool.createPartition(tool.ID_LINUX, sizeBytes=logs_size * 2**20, startBytes=2**20, number=logs_partnum, order=order)
else:
tool.createPartition(tool.ID_LINUX, sizeBytes=logs_size * 2**20, number=logs_partnum, order=order)
order += 1
if logs_partnum > 0:
if order == 1:
tool.createPartition(tool.ID_LINUX, sizeBytes=constants.logs_size * 2**20, startBytes=2**20, number=logs_partnum, order=order)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps remove the from constants import * line at the top of the file to avoid any accidental reference to the wrong size?

Even still, this is quite nasty since the constant changes above won't be visible in other modules that import from constants rather than referencing constants. To avoid this, can the partition sizes be made part of the mutable installation data, much like the partition numbers are?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check all usage of constants (and I'm not sure how to do it safely to avoid crashes in unexpected code paths).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid using mutable installation data unless we decide to support more of these alternated layout. And that would take surely more time to test it (for regressions). Also, the way we use these data would create functions with ton of parameters and specifiers which honestly is already not that readable now.

else:
tool.createPartition(tool.ID_LINUX, sizeBytes=constants.logs_size * 2**20, number=logs_partnum, order=order)
order += 1

# Create backup partition
if backup_partnum > 0:
tool.createPartition(tool.ID_LINUX, sizeBytes=backup_size * 2**20, number=backup_partnum, order=order)
tool.createPartition(tool.ID_LINUX, sizeBytes=constants.backup_size * 2**20, number=backup_partnum, order=order)
order += 1

# Create dom0 partition
Expand All @@ -653,14 +671,15 @@ def writeDom0DiskPartitions(disk, target_boot_mode, boot_partnum, primary_partnu

# Create Boot partition
if target_boot_mode == TARGET_BOOT_MODE_UEFI:
tool.createPartition(tool.ID_EFI_BOOT, sizeBytes=boot_size * 2**20, number=boot_partnum, order=order)
tool.createPartition(tool.ID_EFI_BOOT, sizeBytes=constants.boot_size * 2**20, number=boot_partnum, order=order)
else:
tool.createPartition(tool.ID_BIOS_BOOT, sizeBytes=boot_size * 2**20, number=boot_partnum, order=order)
tool.createPartition(tool.ID_BIOS_BOOT, sizeBytes=constants.boot_size * 2**20, number=boot_partnum, order=order)
order += 1

# Create swap partition
tool.createPartition(tool.ID_LINUX_SWAP, sizeBytes=swap_size * 2**20, number=swap_partnum, order=order)
order += 1
if swap_partnum > 0:
tool.createPartition(tool.ID_LINUX_SWAP, sizeBytes=constants.swap_size * 2**20, number=swap_partnum, order=order)
order += 1

# Create LVM partition
if storage_partnum > 0:
Expand Down
2 changes: 2 additions & 0 deletions disktools.py
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ class GPTPartitionTool(PartitionToolBase):
# These are partition type GUIDs
ID_LINUX_SWAP = "0657FD6D-A4AB-43C4-84E5-0933C84B4F4F"
ID_LINUX = "EBD0A0A2-B9E5-4433-87C0-68B6B72699C7"
ID_LINUX2 = "0FC63DAF-8483-4772-8E79-3D69D8477DE4"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases would the host-installer need to handle a partition of this type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you load the MBR using GPT tool for doing the conversion, it's the type gdisk will use.

ID_LINUX_LVM = "E6D6D379-F507-44C2-A23C-238F2A3DF928"
ID_EFI_BOOT = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
ID_BIOS_BOOT = "21686148-6449-6E6F-744E-656564454649"
Expand All @@ -948,6 +949,7 @@ class GPTPartitionTool(PartitionToolBase):
GUID_to_type_code = {
ID_LINUX_SWAP: '8200',
ID_LINUX: '0700',
ID_LINUX2: '0700',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0FC63DAF-8483-4772-8E79-3D69D8477DE4 has a type code of 0x8300 according to gptfdisk, not 0x700.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but we want to end up with EBD0A0A2-B9E5-4433-87C0-68B6B72699C7 for compatibility reasons.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than calling this ID_LINUX2, I think it would be better to rename ID_LINUX to reflect what gptfdisk calls it (at least since 0.7.2 from 2011).

0x700 == EBD0A0A2-B9E5-4433-87C0-68B6B72699C7 == "Microsoft basic data"
0x8300 == 0FC63DAF-8483-4772-8E79-3D69D8477DE4 == "Linux filesystem"

(Although it is still not clear to me why this new constant is needed.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using Microsoft basic type type for Linux partitions. So, we want to tell sgdisk 0700 for both. Maybe use ID_LINUX for 0FC63DAF-8483-4772-8E79-3D69D8477DE4 and ID_MS_BASIC for EBD0A0A2-B9E5-4433-87C0-68B6B72699C7 but maps both to 0700 ?

ID_LINUX_LVM: '8e00',
ID_EFI_BOOT: 'ef00',
ID_BIOS_BOOT: 'ef02',
Expand Down
128 changes: 97 additions & 31 deletions upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,27 @@ def buildRestoreList(self, src_base):
completeUpgrade(). """
return []

def handleRestoreList(self, src_base, restore_list, restore_file):
for f in restore_list:
if isinstance(f, str):
restore_file(src_base, f, None)
elif isinstance(f, dict):
if 'src' in f:
assert 'dst' in f
restore_file(src_base, f['src'], f['dst'])
elif 'dir' in f:
pat = f.get('re', None)
src_dir = os.path.join(src_base, f['dir'])
if os.path.exists(src_dir):
if pat is not None:
# filter here (though should ideally let restore_file do it)
for ff in os.listdir(src_dir):
fn = os.path.join(f['dir'], ff)
if pat.match(fn):
restore_file(src_base, fn, None)
else:
restore_file(src_base, f['dir'], None)

completeUpgradeArgs = ['mounts', 'primary-disk', 'backup-partnum']
def completeUpgrade(self, mounts, target_disk, backup_partnum):
""" Write any data back into the new filesystem as needed to follow
Expand Down Expand Up @@ -166,34 +187,23 @@ def restore_file(src_base, f, d=None):
else:
logger.log("WARNING: /%s did not exist in the backup image." % f)

backup_volume = partitionDevice(target_disk, backup_partnum)
tds = util.TempMount(backup_volume, 'upgrade-src-', options=['ro'])
tds = None
if backup_partnum <= 0:
src_base = "/tmp/fs_backup"
else:
backup_volume = partitionDevice(target_disk, backup_partnum)
tds = util.TempMount(backup_volume, 'upgrade-src-', options=['ro'])
src_base = tds.mount_point

try:
restore_list = self.buildRestoreList(tds.mount_point)
init_id_maps(tds.mount_point, mounts['root'])
restore_list = self.buildRestoreList(src_base)
init_id_maps(src_base, mounts['root'])

logger.log("Restoring preserved files")
for f in restore_list:
if isinstance(f, str):
restore_file(tds.mount_point, f)
elif isinstance(f, dict):
if 'src' in f:
assert 'dst' in f
restore_file(tds.mount_point, f['src'], f['dst'])
elif 'dir' in f:
pat = f.get('re', None)
src_dir = os.path.join(tds.mount_point, f['dir'])
if os.path.exists(src_dir):
if pat is not None:
# filter here (though should ideally let restore_file do it)
for ff in os.listdir(src_dir):
fn = os.path.join(f['dir'], ff)
if pat.match(fn):
restore_file(tds.mount_point, fn)
else:
restore_file(tds.mount_point, f['dir'])
self.handleRestoreList(src_base, restore_list, restore_file)
finally:
tds.unmount()
if tds is not None:
tds.unmount()


class ThirdGenUpgrader(Upgrader):
Expand Down Expand Up @@ -228,11 +238,15 @@ def testUpgradeForbidden(self, tool):
raise RuntimeError("Util partition detected on DOS partition type, upgrade forbidden.")

convertTargetStateChanges = []
convertTargetArgs = ['primary-disk', 'target-boot-mode', 'boot-partnum', 'primary-partnum', 'logs-partnum', 'swap-partnum', 'storage-partnum', 'backup-partnum']
def convertTarget(self, progress_callback, primary_disk, target_boot_mode, boot_partnum, primary_partnum, logs_partnum, swap_partnum, storage_partnum, backup_partnum):
convertTargetArgs = ['primary-disk', 'target-boot-mode', 'boot-partnum', 'primary-partnum', 'logs-partnum', 'swap-partnum', 'storage-partnum', 'backup-partnum', 'target-platform']
def convertTarget(self, progress_callback, primary_disk, target_boot_mode, boot_partnum, primary_partnum, logs_partnum, swap_partnum, storage_partnum, backup_partnum, target_platform):
if not self.safe2mbrupgrade:
return

# SDX 8900 we keep old data
if target_platform == 'sdx8900':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you still need to convert from MBR to GPT, even if the partition resizing is not necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is done automatically, that's why the additional partition GUID.

return

tool = PartitionTool(primary_disk)
if tool.partTableType != constants.PARTITION_DOS:
return
Expand Down Expand Up @@ -320,8 +334,8 @@ def convertTarget(self, progress_callback, primary_disk, target_boot_mode, boot_


prepTargetStateChanges = []
prepTargetArgs = ['primary-disk', 'target-boot-mode', 'boot-partnum', 'primary-partnum', 'logs-partnum', 'swap-partnum', 'storage-partnum', 'backup-partnum']
def prepareTarget(self, progress_callback, primary_disk, target_boot_mode, boot_partnum, primary_partnum, logs_partnum, swap_partnum, storage_partnum, backup_partnum):
prepTargetArgs = ['primary-disk', 'target-boot-mode', 'boot-partnum', 'primary-partnum', 'logs-partnum', 'swap-partnum', 'storage-partnum', 'backup-partnum', 'target-platform']
def prepareTarget(self, progress_callback, primary_disk, target_boot_mode, boot_partnum, primary_partnum, logs_partnum, swap_partnum, storage_partnum, backup_partnum, target_platform):
""" Modify partition layout prior to installation. """

tool = PartitionTool(primary_disk, constants.PARTITION_GPT)
Expand All @@ -337,7 +351,24 @@ def prepareTarget(self, progress_callback, primary_disk, target_boot_mode, boot_

self.testUpgradeForbidden(tool)

if self.safe2upgrade and logs_partition is None:
if target_platform == 'sdx8900':
# Create new root partition using old root
tool.deletePartition(primary_partnum)
if backup_partnum <= 0:
tool.deletePartitionIfPresent(primary_partnum + 1)
tool.deletePartitionIfPresent(boot_partnum)

order = primary_partnum

# Create new root partition
tool.createPartition(tool.ID_LINUX, sizeBytes=constants.root_size * 2**20, order=order, number=primary_partnum)

# Create boot partition
start = tool.partitionStart(primary_partnum) + constants.root_size * 2**20
tool.createPartition(tool.ID_BIOS_BOOT, sizeBytes=constants.boot_size * 2**20, startBytes=start, number=boot_partnum)

tool.commit(log=True)
elif self.safe2upgrade and logs_partition is None:
# Rename old dom0 and Boot (if any) partitions (10 and 11 are temporary number which let us create
# dom0 and Boot partitions using the same numbers)
tool.renamePartition(srcNumber=primary_partnum, destNumber=10, overwrite=False)
Expand Down Expand Up @@ -415,15 +446,50 @@ def prepareTarget(self, progress_callback, primary_disk, target_boot_mode, boot_
return
raise RuntimeError("Old partition layout is unsupported, run prepare_host_upgrade plugin and try again")

doBackupArgs = ['primary-disk', 'backup-partnum', 'boot-partnum', 'storage-partnum', 'logs-partnum', 'primary-partnum']
def backupFilesToTemp(self, progress_callback, target_disk, boot_device):

progress_callback(10)

# copy the files across:
primary_fs = util.TempMount(self.source.root_device, 'primary-', options=['ro'], boot_device=boot_device)
backup_dir = "/tmp/fs_backup"

files_list = ['etc/passwd', 'etc/group']
def restore_file(src_base, f, d):
src = os.path.join(src_base, f)
if os.path.exists(src):
files_list.append(f)

try:
os.mkdir(backup_dir, 0o755)

restore_list = self.buildRestoreList(primary_fs.mount_point)

self.handleRestoreList(primary_fs.mount_point, restore_list, restore_file)
logger.log("File list is:\n\t%s" % '\n\t'.join(files_list))
if util.runCmd2("tar -C '%s' -c -T - | tar -C '%s' -x" % (primary_fs.mount_point, backup_dir), inputtext='\n'.join(files_list)) != 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are etc/passwd and etc/group handled specially with a double invocation of tar?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why specially? Normally they are not restored, but they are used in the restore process, so we need to save them. The double invocation is to copy all files and directories. Maybe I didn't get your point.

raise RuntimeError("Backup failed")
progress_callback(100)

# save the GPT table
rc, err = util.runCmd2(["sgdisk", "-b", os.path.join(backup_dir, '.xen-gpt.bin'), target_disk], with_stderr=True)
if rc != 0:
raise RuntimeError("Failed to save partition layout: %s" % err)
finally:
primary_fs.unmount()

doBackupArgs = ['primary-disk', 'backup-partnum', 'boot-partnum', 'storage-partnum', 'logs-partnum', 'primary-partnum', 'target-platform']
doBackupStateChanges = []
def doBackup(self, progress_callback, target_disk, backup_partnum, boot_partnum, storage_partnum, logs_partnum, primary_partnum):
def doBackup(self, progress_callback, target_disk, backup_partnum, boot_partnum, storage_partnum, logs_partnum, primary_partnum, target_platform):

tool = PartitionTool(target_disk)
boot_part = tool.getPartition(boot_partnum)
boot_device = partitionDevice(target_disk, boot_partnum) if boot_part else None
logs_partition = tool.getPartition(logs_partnum)

if target_platform == 'sdx8900':
return self.backupFilesToTemp(progress_callback, target_disk, boot_device)

self.testUpgradeForbidden(tool)

# Check if possible to create new partition layout, increasing the size, using plugin result
Expand Down