I think the other five parts of the patch look good. As for the patch to
boottool itself; I don't see any problems with it, but my Perl is pretty
weak so much review of it doesn't say much.

-- John

On Mon, Oct 18, 2010 at 7:10 AM, Lucas Meneghel Rodrigues 
<[email protected]>wrote:

> On Mon, 2010-10-18 at 06:45 -0700, John Admanski wrote:
> > If you would like to replace boottool I'd be happy to see it go...I
> > seem to remember having a discussions about replacing it as far back
> > as 2008. It's just hard to find a significant amount of time to
> > allocate to replacing something that mostly-works, at least well
> > enough to get by...
>
> Absolutely. I'll put this on my TODO list, anyway I think we'll want to
> support grub2 eventually. Grubby is a candidate, but it's missing the
> --boot-once functionality, so we'd need to patch it and write support
> for grub2. Also, it needs to be compiled on the client machines, so it's
> important to check how to do this specially in the cross compiling mode
> the Chrome OS folks use. So, maybe it is still worthy to develop our own
> python replacement for boottool...
>
> In the meantime, the 6 patches I sent should be enough to get the kernel
> build/install/boot funtctionality working for newer Fedora/RHEL. Please
> take a look at them and let me know what you think.
>
> Lucas
>
> >
> > -- John
> >
> > On Sat, Oct 16, 2010 at 10:38 AM, Lucas Meneghel Rodrigues
> > <[email protected]> wrote:
> >         Instead of resorting to the horrible 2 hacks I had submitted
> >         before, take a little more time and write support in boottool
> >         to handle OS specific exceptions. This way we supersede the
> >         2 previous patches and provide a better and more integrated
> >         approach to solve the problem:
> >
> >         Geez, why isn't boottool --boot-once working for Fedora/RHEL ?
> >
> >         Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
> >         ---
> >          client/tools/boottool |   53
> >         ++++++++++++++++++++++++++++++++++++++++++++----
> >          1 files changed, 48 insertions(+), 5 deletions(-)
> >
> >         diff --git a/client/tools/boottool b/client/tools/boottool
> >         index 4dafbab..728e4ef 100755
> >         --- a/client/tools/boottool
> >         +++ b/client/tools/boottool
> >         @@ -791,6 +791,38 @@ sub detect_architecture {
> >             return $arch;
> >          }
> >
> >         +=head3 detect_os_vendor()
> >         +
> >         +Input:
> >         +Output: string
> >         +
> >         +This function determines the OS vendor (linux distribution
> >         breed).
> >         +
> >         +Return values: "Red Hat", "Fedora", "SUSE", "Ubuntu",
> >         "Debian", or
> >         +"Unknown" if none of the predefined patterns could be found
> >         on the
> >         +issue file.
> >         +
> >         +=cut
> >         +
> >         +sub detect_os_vendor {
> >         +    my $vendor = "";
> >         +    my $issue_file = '/etc/issue';
> >         +    if ( not system("egrep 'Red Hat' $issue_file") ){
> >         +       $vendor = 'Red Hat';
> >         +    } elsif ( not system("egrep 'Fedora' $issue_file") ){
> >         +       $vendor = 'Fedora';
> >         +    } elsif ( not system("egrep 'SUSE' $issue_file") ){
> >         +       $vendor = 'SUSE';
> >         +    } elsif ( not system("egrep 'Ubuntu' $issue_file") ){
> >         +       $vendor = 'Ubuntu';
> >         +    } elsif ( not system("egrep 'Debian' $issue_file") ){
> >         +       $vendor = 'Debian';
> >         +    } else {
> >         +       $vendor = 'Unknown';
> >         +    }
> >         +    return $vendor;
> >         +}
> >         +
> >          =head3 detect_bootloader(['device1', 'device2', ...])
> >
> >          Input:  devices to detect against (optional)
> >         @@ -1595,20 +1627,31 @@ sub update_main_options{
> >          sub boot_once {
> >           my $self=shift;
> >           my $entry_to_boot_once = shift;
> >         +  my $detected_os_vendor =
> >         Linux::Bootloader::Detect::detect_os_vendor();
> >
> >           unless ( $entry_to_boot_once ) { print "No kernel\n"; return
> >         undef;}
> >           $self->read();
> >           my $default=$self->get_default();
> >
> >         -  if ( $default == $self->_lookup($entry_to_boot_once)){
> >         -     warn "The default and once-boot kernels are the same.
> >          No action taken.  \nSet default to something else, then
> >         re-try.\n";
> >         -     return undef;
> >         -  }
> >           if ( $self->_get_bootloader_version() < 0.97 ){
> >              warn "This function works for grub version 0.97 and up.
> >          No action taken.  \nUpgrade, then re-try.\n";
> >              return undef;
> >           }
> >
> >         +  if ( $detected_os_vendor eq "Red Hat" or
> >         $detected_os_vendor eq "Fedora" ) {
> >         +    # if not a number, do title lookup
> >         +    if ( $entry_to_boot_once !~ /^\d+$/ ) {
> >         +      $entry_to_boot_once =
> >         $self->_lookup($entry_to_boot_once);
> >         +      return undef unless ( defined $entry_to_boot_once );
> >         +    }
> >         +
> >         +    return `echo "savedefault --default=$entry_to_boot_once"
> >         --once | grub --batch`;
> >         +  } else {
> >         +  if ( $default == $self->_lookup($entry_to_boot_once)){
> >         +     warn "The default and once-boot kernels are the same.
> >          No action taken.  \nSet default to something else, then
> >         re-try.\n";
> >         +     return undef;
> >         +  }
> >         +
> >           $self->set_default('saved');
> >           if ( ! -f '/boot/grub/default' ){
> >              open FH, '>/boot/grub/default';
> >         @@ -1635,7 +1678,7 @@ sub boot_once {
> >
> >         $self->update(
> 'update-kernel'=>"$entry_to_boot_once",'option'=>'','savedefault' =>
> 'fallback' );
> >           $self->update( 'update-kernel'=>"$default",'option'=>'',
> >         'savedefault' => '' );
> >           $self->write();
> >         -
> >         +  }
> >          }
> >
> >          sub _get_bootloader_version {
> >         --
> >         1.7.2.3
> >
> >
> >
>
>
>
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to