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

RE: [PATCH] add multiboot support for tboot



> > @@ -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"
> > -                          % {"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"
> 
> I'm sure it made sense to do it this way when we only had two
> possibilities, but now with a third, I'd much prefer that the code to
> generate a stanza be a method on an Image subclass.

Think it again. Before generating stanza in Image subclass method we will also need to check the bootloader type first. It just defer the condition checks into next level. Unless we redefine all Image subclasses as GrubLinuxBootLoaderImage, Grub2LinuxBootLoaderImage, GrubMultibootLinuxBootLoaderImage, etc. I still think it is suitable to keep Image classes simple to just provide mechanism, and put more logic in the calling classes.

Jimmy


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