[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

RE: [PATCH V2] add multiboot support for tboot



Thanks for the precious comments from both David & Chris. I will revise and send out a third version soon.

BTW, can anyone tell me where to find the current compose package list for F16? I need to make sure tboot package is included.

Jimmy

David Lehman wrote onĀ 2011-08-12:
> I have some inline comments.
> 
> On Mon, 2011-08-01 at 17:54 +0800, Wei, Gang wrote:
>> diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
>> index 6df0107..dc11f90 100644
>> --- a/pyanaconda/bootloader.py
>> +++ b/pyanaconda/bootloader.py
>> @@ -114,6 +114,25 @@ class LinuxBootLoaderImage(BootLoaderImage):
>>              filename = "initramfs-%s.img" % self.version
>>          return filename
>> +class MultibootLinuxBootLoaderImage(LinuxBootLoaderImage): +    def
>> __init__(self, device=None, label=None, short=None, version=None, +    
>>             multiboot=None, mbargs=None, args=None): +       
>> super(MultibootLinuxBootLoaderImage, self).__init__( + device=device,
>> label=label, + short=short, version=version) +        self._multiboot =
>> multiboot      # filename string +        self._mbargs = mbargs
> 
> It seems like these two attributes above could just be hardcoded in
> here if they cannot be changed in any case.
> 
>> +       self._args = args
>> +
>> +    @property
>> +    def multiboot(self):
>> +        return self._multiboot
>> +
>> +    def mbargs(self):
>> +        return self._mbargs
>> +
>> +    def args(self):
>> +        return self._args
>> 
>>  class BootLoader(object):
>>      """TODO:
>> @@ -1123,15 +1142,30 @@ class GRUB(BootLoader):
>>                  grub_root = self.grub_device_name(self.stage2_device)
>>                  args.update(["ro", "root=%s" %
>>                  image.device.fstabSpec]) args.update(self.boot_args)
>> -                stanza = ("title %(label)s (%(version)s)\n"
>> -                          "\troot %(grub_root)s\n"
> 
> 
>> -                          "\tkernel %(prefix)s/%(kernel)s %(args)s\n"
>> -                          "\tinitrd %(prefix)s/%(initrd)s\n"
> 
> Just by abstracting the above section you could prevent duplication of
> the entire stanza template, eg:
> 
> if isinstance(image, MultibootLinuxBootLoaderImage):
>   snippet = ("\tkernel %(prefix)s/%(multiboot)s %(mbargs)s\n"
>              "\tmodule %(prefix)s/%(kernel)s %(args)s\n"
>              "\tmodule %(prefix)s/%(initrd)s\n"
>              % {"prefix": self.boot_prefix,
>                 "multiboot": image.multiboot,
>                 "mbargs": image.mbargs,
>                 "kernel": image.kernel, "initrd": image.initrd,
>                 "args": args})
> else:
>   snippet = ("\tkernel %(prefix)s/%(kernel)s %(args)s\n"
>              "\tinitrd %(prefix)s/%(initrd)s\n"
>              % {"prefix": self.boot_prefix,
>                 "kernel": image.kernel, "initrd": image.initrd,
>                 "args": args})
> and then
> 
> stanza = ("title %(label)s (%(version)s)\n"
>           "\troot %(grub_root)s\n"
>           "%(snippet)s"
>           % {"label": image.label, "version": image.version,
> 	     "snippet": snippet
>              "grub_root": grub_root})
> 
> 
>> -                          % {"label": image.label, "version":
>> image.version,
>> -                             "grub_root": grub_root,
>> -                             "kernel": image.kernel, "initrd":
>> image.initrd,
>> -                             "args": args,
>> -                             "prefix": self.boot_prefix})
>> +                if isinstance(image, MultibootLinuxBootLoaderImage):
>> +                    args.update(image.args)
>> +                    stanza = ("title %(label)s (%(version)s)\n"
>> +                              "\troot %(grub_root)s\n"
>> +                              "\tkernel %(prefix)s/%(multiboot)s
>> %(mbargs)s\n"
>> +                              "\tmodule %(prefix)s/%(kernel)s
>> %(args)s\n"
>> +                              "\tmodule %(prefix)s/%(initrd)s\n"
>> +                              % {"label": image.label, "version":
>> image.version,
>> +                                 "grub_root": grub_root,
>> +                                 "multiboot": image.multiboot,
>> +                                 "mbargs": image.mbargs,
>> +                                 "kernel": image.kernel, "initrd":
>> image.initrd,
>> +                                 "args": args,
>> +                                 "prefix": self.boot_prefix})
>> +                else:
>> +                    stanza = ("title %(label)s (%(version)s)\n"
>> +                              "\troot %(grub_root)s\n"
>> +                              "\tkernel %(prefix)s/%(kernel)s
>> %(args)s\n"
>> +                              "\tinitrd %(prefix)s/%(initrd)s\n"
>> +                              % {"label": image.label, "version":
>> image.version,
>> +                                 "grub_root": grub_root,
>> +                                 "kernel": image.kernel, "initrd":
>> image.initrd,
>> +                                 "args": args,
>> +                                 "prefix": self.boot_prefix})
>>              else:
>>                  stanza = ("title %(label)s\n"
>>                            "\trootnoverify %(grub_root)s\n"
>> @@ -1905,6 +1939,8 @@ def writeSysconfigKernel(anaconda,
>> default_kernel):
>>      f.write("DEFAULTKERNEL=%s\n" % default_kernel)
>>      f.close()
>> +def is_trusted_boot(anaconda):
>> +    return anaconda.backend.ayum.isPackageInstalled(name="tboot")
> 
> Perhaps this could be written to an attribute of the BootLoader
> instance from inside YumBackend immediately after package installation
> so we don't have yum references in the bootloader module. Then you can
> just check anaconda.bootloader.trusted_boot in writeBootloader.
> 
>> 
>>  def writeBootloader(anaconda):
>>      """ Write bootloader configuration to disk.
>> @@ -1971,9 +2007,18 @@ def writeBootloader(anaconda):
>>          used.append(nick)
>>          label = "%s-%s" % (base_label, nick)
>>          short = "%s-%s" % (base_short, nick)
>> -        image =
>> LinuxBootLoaderImage(device=anaconda.storage.rootDevice,
>> -                                     version=version,
>> -                                     label=label, short=short)
>> +        if is_trusted_boot(anaconda):
>> +            image = MultibootLinuxBootLoaderImage(
>> +
>> device=anaconda.storage.rootDevice,
>> +                                         version=version,
>> +                                         label=label, short=short,
>> +                                         multiboot="tboot.gz",
>> +
>> mbargs=["logging=vga,serial,memory"],
>> +
> args=["intel_iommu=on"])
> 
> If you're going to hardcode the multiboot and mbargs strings, why not
> do it in the MultibootLinuxBootLoaderImage class instead of here?
> 
>> +        else:
>> +            image =
>> LinuxBootLoaderImage(device=anaconda.storage.rootDevice,
>> +                                         version=version,
>> +                                         label=label, short=short)
>>          anaconda.bootloader.add_image(image)
>>      # write out /etc/sysconfig/kernel
> 
> Dave
> 
>> --
>> 1.7.5.1
>> 
>> 
>> 
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]