Merge pull request #188 from marineam/syslinux

feat(disk_util): Switch from SYSLINUX's gptmbr.bin to mbr.bin

build_library/base_image_util.sh

@@ -7,12 +7,11 @@ create_base_image() {
   local disk_layout=$2
 
   local disk_img="${BUILD_DIR}/${image_name}"
-  local mbr_img="/usr/share/syslinux/gptmbr.bin"
   local root_fs_dir="${BUILD_DIR}/rootfs"
 
   info "Using image type ${disk_layout}"
   "${BUILD_LIBRARY_DIR}/disk_util" --disk_layout="${disk_layout}" \
-      format --mbr_boot_code="${mbr_img}" "${disk_img}"
+      format "${disk_img}"
 
   "${BUILD_LIBRARY_DIR}/disk_util" --disk_layout="${disk_layout}" \
       mount "${disk_img}" "${root_fs_dir}"

build_library/disk_util

@@ -15,6 +15,10 @@ import uuid
 # First sector we can use.
 GPT_RESERVED_SECTORS = 34
 
+# Default MBR boot code for hybrid MBRs
+DEFAULT_MBR_BOOT_CODE = '/usr/share/syslinux/mbr.bin'
+
+
 class ConfigNotFound(Exception):
   pass
 class PartitionNotFound(Exception):
@@ -23,6 +27,8 @@ class InvalidLayout(Exception):
   pass
 class InvalidAdjustment(Exception):
   pass
+class InvalidBootCode(Exception):
+  pass
 
 
 def LoadPartitionConfig(options):
@@ -254,7 +260,7 @@ def WritePartitionTable(options, config=None, partitions=None):
 
   Cgpt('create', '-c', '-s', config['metadata']['blocks'], options.disk_image)
 
-  esp_number = None
+  syslinux = None
   for partition in partitions.itervalues():
     if partition['type'] != 'blank':
       Cgpt('add', '-i', partition['num'],
@@ -265,17 +271,28 @@ def WritePartitionTable(options, config=None, partitions=None):
                   '-u', partition['uuid'],
                   options.disk_image)
 
-    if partition['type'] == 'efi':
-      esp_number = partition['num']
+      features = partition.get('features', [])
+      if not syslinux and 'syslinux' in features:
+        syslinux = partition['num']
 
-  if esp_number is None:
-    raise InvalidLayout('Table does not include an EFI partition.')
+  if syslinux and options.mbr_boot_code:
+    # Enable legacy boot flag and generate a hybrid MBR partition table
+    Cgpt('add', '-i', syslinux, '-B1', options.disk_image)
 
-  if options.mbr_boot_code:
-    Cgpt('boot', '-p',
-                 '-b', options.mbr_boot_code,
-                 '-i', esp_number,
-                 options.disk_image)
+    try:
+      with open(options.mbr_boot_code, 'r') as mbr_fd:
+        mbr_code = mbr_fd.read()
+    except IOError, ex:
+      raise InvalidBootCode("Failed to read %s: %s",
+                            options.mbr_boot_code, ex)
+
+    # The boot code must fit in the first 440 bytes of disk
+    if len(mbr_code) != 440:
+      raise InvalidBootCode("Invalid %s size %s, must be 440 bytes",
+                            options.mbr_boot_code, len(mbr_code))
+
+    with open(options.disk_image, 'r+') as image_fd:
+      image_fd.write(mbr_code)
 
   Cgpt('show', options.disk_image)
 
@@ -847,19 +864,19 @@ def main(argv):
           help='initialize new partition table')
   a.add_argument('--update', action='store_false', dest='create',
           help='update existing partition table')
-  a.add_argument('--mbr_boot_code',
+  a.add_argument('--mbr_boot_code', default=DEFAULT_MBR_BOOT_CODE,
           help='path to mbr boot block, such as syslinux/gptmbr.bin')
   a.add_argument('disk_image', help='path to disk image file')
   a.set_defaults(func=WritePartitionTable)
 
   a = actions.add_parser('format', help='write gpt and filesystems to image')
-  a.add_argument('--mbr_boot_code',
+  a.add_argument('--mbr_boot_code', default=DEFAULT_MBR_BOOT_CODE,
           help='path to mbr boot block, such as syslinux/gptmbr.bin')
   a.add_argument('disk_image', help='path to disk image file')
   a.set_defaults(func=Format, create=True)
 
   a = actions.add_parser('resize', help='write gpt and resize filesystems')
-  a.add_argument('--mbr_boot_code',
+  a.add_argument('--mbr_boot_code', default=DEFAULT_MBR_BOOT_CODE,
           help='path to mbr boot block, such as syslinux/gptmbr.bin')
   a.add_argument('disk_image', help='path to disk image file')
   a.set_defaults(func=Resize, create=False)

build_library/vm_image_util.sh

@@ -37,9 +37,6 @@ VM_UUID=
 VM_GENERATED_FILES=()
 
 ## DEFAULT
-# If set to 1 use a hybrid GPT/MBR format instead of plain GPT
-IMG_DEFAULT_HYBRID_MBR=0
-
 # If set to 0 then a partition skeleton won't be laid out on VM_TMP_IMG
 IMG_DEFAULT_PARTITIONED_IMG=1
 
@@ -68,8 +65,6 @@ IMG_qemu_DISK_LAYOUT=vm
 IMG_qemu_CONF_FORMAT=qemu
 
 ## xen
-# Hybrid is required by pvgrub (pygrub supports GPT but we support both)
-IMG_xen_HYBRID_MBR=1
 IMG_xen_BOOT_KERNEL=0
 IMG_xen_CONF_FORMAT=xl
 
@@ -102,7 +97,6 @@ IMG_vmware_insecure_CONF_FORMAT=vmware_zip
 IMG_vmware_insecure_OEM_PACKAGE=oem-vagrant
 
 ## ami
-IMG_ami_HYBRID_MBR=1
 IMG_ami_BOOT_KERNEL=0
 IMG_ami_OEM_PACKAGE=oem-ami
 
@@ -213,10 +207,8 @@ setup_disk_image() {
     cp --sparse=always "${VM_SRC_IMG}" "${VM_TMP_IMG}"
 
     if [[ $(_get_vm_opt PARTITIONED_IMG) -eq 1 ]]; then
-      # TODO(marineam): Fix this so --mbr_boot_code isn't required
-      local mbr_img="/usr/share/syslinux/gptmbr.bin"
       "${BUILD_LIBRARY_DIR}/disk_util" --disk_layout="${disk_layout}" \
-          resize --mbr_boot_code="${mbr_img}" "${VM_TMP_IMG}"
+          resize "${VM_TMP_IMG}"
     fi
 
     info "Mounting image to $(relpath "${VM_TMP_ROOT}")"
@@ -250,33 +242,12 @@ write_vm_disk() {
         cleanup_mounts "${VM_TMP_ROOT}"
     fi
 
-    if [[ $(_get_vm_opt HYBRID_MBR) -eq 1 ]]; then
-        info "Creating hybrid MBR"
-        _write_hybrid_mbr "${VM_TMP_IMG}"
-    fi
-
     local disk_format=$(_get_vm_opt DISK_FORMAT)
     info "Writing $disk_format image $(basename "${VM_DST_IMG}")"
     _write_${disk_format}_disk "${VM_TMP_IMG}" "${VM_DST_IMG}"
     VM_GENERATED_FILES+=( "${VM_DST_IMG}" )
 }
 
-_write_hybrid_mbr() {
-    # TODO(marineam): Switch to sgdisk
-    /usr/sbin/gdisk "$1" <<EOF
-r
-h
-1
-N
-c
-Y
-N
-w
-Y
-Y
-EOF
-}
-
 _write_raw_disk() {
     mv "$1" "$2"
 }