On Fri, 2010-08-20 at 16:33 -0700, Duane Sand wrote: > Bug 2779938: Many client jobs fail from fsck-induced double reboots. > Add 'touch /fastboot' to kernel rpm boot method used by client tests. > Also, factor out shared boot method for kernels installed from source > or from rpm, in client/bin/kernel and its unittest. > > Note that this patch overlaps with prior patch CL 4132496, which has not yet > been applied to SVN, so this patch seems to apply uncleanly by itself. > Applying > the former patch first should resolve that. Functionally, the second patch > totally replaces and extends the effect of the first patch. > > Using fastboot for boots into the target test kernel has greatly reduced > our rate of failed boots due to fsck's bringing back the default kernel.
I have reviewed the patch, looks good to me, I liked the refactoring you made with BootableKernel. I am testing it just to double check everything works correctly, and created an updated patch that merges your 2 patches. Once that is done, I am going to apply it upstream. > Signed-off-by: Duane Sand <[email protected]> > > --- autotest/client/bin/kernel.py 2010-07-12 16:31:48.000000000 -0700 > +++ autotest/client/bin/kernel.py 2010-07-15 18:23:26.000000000 -0700 > @@ -64,7 +64,49 @@ > root=root) > > > -class kernel(object): > +class BootableKernel(object): > + > + def __init__(self, job): > + self.job = job > + self.installed_as = None # kernel choice in bootloader menu > + self.image = None > + self.initrd = '' > + > + > + def _boot_kernel(self, args, ident_check, expected_ident, subdir, notes): > + """ Boot a kernel, with post-boot kernel id check > + > + @param args: kernel cmdline arguments > + @param ident_check: check kernel id after boot > + @param expected_ident: > + @param subdir: job-step qualifier in status log > + @param notes: additional comment in status log > + """ > + > + # If we can check the kernel identity do so. > + if ident_check: > + when = int(time.time()) > + args += " IDENT=%d" % when > + self.job.next_step_prepend(["job.end_reboot_and_verify", when, > + expected_ident, subdir, notes]) > + else: > + self.job.next_step_prepend(["job.end_reboot", > + subdir, expected_ident, notes]) > + > + # Point bootloader to the selected tag. > + _add_kernel_to_bootloader(self.job.bootloader, > + self.job.config_get('boot.default_args'), > + self.installed_as, args, self.image, > self.initrd) > + > + # defer fsck for next reboot, to avoid reboots back to default kernel > + utils.system('touch /fastboot') # this file is removed automatically > + > + # Boot it. > + self.job.start_reboot() > + self.job.reboot(tag=self.installed_as) > + > + > +class kernel(BootableKernel): > """ Class for compiling kernels. > > Data for the object includes the src files > @@ -109,7 +151,7 @@ > leave > Boolean, whether to leave existing tmpdir or not > """ > - self.job = job > + super(kernel, self).__init__(job) > self.autodir = job.autodir > > self.src_dir = os.path.join(tmp_dir, 'src') > @@ -120,8 +162,6 @@ > self.results_dir = os.path.join(subdir, 'results') > self.subdir = os.path.basename(subdir) > > - self.installed_as = None > - > if not leave: > if os.path.isdir(self.src_dir): > utils.system('rm -rf ' + self.src_dir) > @@ -450,16 +490,6 @@ > self.system_map, self.initrd) > > > - def add_to_bootloader(self, tag='autotest', args=''): > - """ add this kernel to bootloader, taking an > - optional parameter of space separated parameters > - e.g.: kernel.add_to_bootloader('mykernel', 'ro acpi=off') > - """ > - _add_kernel_to_bootloader(self.job.bootloader, > - self.job.config_get('boot.default_args'), > - tag, args, self.image, self.initrd) > - > - > def get_kernel_build_arch(self, arch=None): > """ > Work out the current kernel architecture (as a kernel arch) > @@ -526,32 +556,14 @@ > just make it happen. > """ > > - # If we can check the kernel identity do so. > - expected_ident = self.get_kernel_build_ident() > - if ident: > - when = int(time.time()) > - args += " IDENT=%d" % (when) > - self.job.next_step_prepend(["job.end_reboot_and_verify", when, > - expected_ident, self.subdir, > - self.applied_patches]) > - else: > - self.job.next_step_prepend(["job.end_reboot", self.subdir, > - expected_ident, > self.applied_patches]) > - > - # Check if the kernel has been installed, if not install > - # as the default tag and boot that. > + # If the kernel has not yet been installed, > + # install it now as default tag. > if not self.installed_as: > self.install() > > - # Boot the selected tag. > - self.add_to_bootloader(args=args, tag=self.installed_as) > - > - # defer fsck for next reboot, to avoid reboots back to default kernel > - utils.system('touch /fastboot') # this file is removed automatically > - > - # Boot it. > - self.job.start_reboot() > - self.job.reboot(tag=self.installed_as) > + expected_ident = self.get_kernel_build_ident() > + self._boot_kernel(args, ident, expected_ident, > + self.subdir, self.applied_patches) > > > def get_kernel_build_ver(self): > @@ -649,20 +661,19 @@ > pickle.dump(temp, open(filename, 'w')) > > > -class rpm_kernel(object): > +class rpm_kernel(BootableKernel): > """ > Class for installing a binary rpm kernel package > """ > > def __init__(self, job, rpm_package, subdir): > - self.job = job > + super(rpm_kernel, self).__init__(job) > self.rpm_package = rpm_package > self.log_dir = os.path.join(subdir, 'debug') > self.subdir = os.path.basename(subdir) > if os.path.exists(self.log_dir): > utils.system('rm -rf ' + self.log_dir) > os.mkdir(self.log_dir) > - self.installed_as = None > > > def build(self, *args, **dargs): > @@ -728,44 +739,23 @@ > % (vmlinux, rpm_pack)) > > > - def add_to_bootloader(self, tag='autotest', args=''): > - """ Add this kernel to bootloader > - """ > - _add_kernel_to_bootloader(self.job.bootloader, > - self.job.config_get('boot.default_args'), > - tag, args, self.image, self.initrd) > - > - > def boot(self, args='', ident=True): > """ install and boot this kernel > """ > > - # Check if the kernel has been installed, if not install > - # as the default tag and boot that. > + # If the kernel has not yet been installed, > + # install it now as default tag. > if not self.installed_as: > self.install() > > - # If we can check the kernel identity do so. > expected_ident = self.full_version > if not expected_ident: > expected_ident = '-'.join([self.version, > self.rpm_flavour, > self.release]) > - if ident: > - when = int(time.time()) > - args += " IDENT=%d" % (when) > - self.job.next_step_prepend(["job.end_reboot_and_verify", > - when, expected_ident, None, 'rpm']) > - else: > - self.job.next_step_prepend(["job.end_reboot", None, > - expected_ident, []]) > - > - # Boot the selected tag. > - self.add_to_bootloader(args=args, tag=self.installed_as) > > - # Boot it. > - self.job.start_reboot() > - self.job.reboot(tag=self.installed_as) > + self._boot_kernel(args, ident, expected_ident, > + None, 'rpm') > > > class rpm_kernel_suse(rpm_kernel): > --- autotest/client/bin/kernel_unittest.py 2010-07-12 16:31:48.000000000 > -0700 > +++ autotest/client/bin/kernel_unittest.py 2010-07-15 18:23:26.000000000 > -0700 > @@ -6,6 +6,99 @@ > from autotest_lib.client.bin import kernel, job, utils, kernelexpand > from autotest_lib.client.bin import kernel_config, boottool, os_dep > > + > +class TestAddKernelToBootLoader(unittest.TestCase): > + > + def add_to_bootloader(self, base_args, args, > + bootloader_args, bootloader_root, > + tag='image', > + image='image', initrd='initrd'): > + god = mock.mock_god() > + bootloader = god.create_mock_class( > + boottool.boottool, "boottool") > + > + # record > + bootloader.remove_kernel.expect_call(tag) > + bootloader.add_kernel.expect_call( > + image, tag, initrd=initrd, args=bootloader_args, > + root=bootloader_root) > + > + # run and check > + kernel._add_kernel_to_bootloader( > + bootloader, base_args, tag, > + args, image, initrd) > + god.check_playback() > + > + > + def test_add_kernel_to_bootloader(self): > + self.add_to_bootloader(base_args='baseargs', > + args='', > + bootloader_args='baseargs', > + bootloader_root=None) > + self.add_to_bootloader(base_args='arg1 root=/dev/oldroot arg2', > + args='root=/dev/newroot arg3', > + bootloader_args='arg1 arg2 arg3', > + bootloader_root='/dev/newroot') > + > + > +class TestBootableKernel(unittest.TestCase): > + > + def setUp(self): > + self.god = mock.mock_god() > + self.god.stub_function(time, "time") > + self.god.stub_function(utils, "system") > + self.god.stub_function(kernel, "_add_kernel_to_bootloader") > + job_ = self.god.create_mock_class(job.job, "job") > + self.kernel = kernel.BootableKernel(job_) > + self.kernel.job.bootloader = self.god.create_mock_class( > + boottool.boottool, "boottool") > + > + > + def tearDown(self): > + # note: time.time() can only be unstubbed via tearDown() > + self.god.unstub_all() > + > + > + def boot_kernel(self, ident_check): > + notes = "applied_patches" > + when = 1 > + args = '' > + base_args = 'base_args' > + tag = 'ident' > + subdir = 'subdir' > + self.kernel.image = 'image' > + self.kernel.initrd = 'initrd' > + self.kernel.installed_as = tag > + > + # record > + args_ = args > + if ident_check: > + time.time.expect_call().and_return(when) > + args_ += " IDENT=%d" % when > + status = ["job.end_reboot_and_verify", when, tag, subdir, notes] > + else: > + status = ["job.end_reboot", subdir, tag, notes] > + self.kernel.job.next_step_prepend.expect_call(status) > + self.kernel.job.config_get.expect_call( > + 'boot.default_args').and_return(base_args) > + kernel._add_kernel_to_bootloader.expect_call( > + self.kernel.job.bootloader, base_args, tag, > + args_, self.kernel.image, self.kernel.initrd) > + utils.system.expect_call('touch /fastboot') > + self.kernel.job.start_reboot.expect_call() > + self.kernel.job.reboot.expect_call(tag=tag) > + > + # run and check > + self.kernel._boot_kernel(args=args, ident_check=ident_check, > + expected_ident=tag, subdir=subdir, > notes=notes) > + self.god.check_playback() > + > + > + def test_boot_kernel(self): > + self.boot_kernel(ident_check=False) > + self.boot_kernel(ident_check=True) > + > + > class TestKernel(unittest.TestCase): > def setUp(self): > self.god = mock.mock_god() > @@ -473,47 +566,6 @@ > self.god.check_playback() > > > - def _setup_add_to_bootloader(self, tag='autotest', args='', > image='image', > - initrd='initrd', base_args='baseargs', > - bootloader_args='baseargs', > - bootloader_root=None): > - self.construct_kernel() > - > - # setup > - self.kernel.image = image > - self.kernel.initrd = initrd > - > - # record > - self.job.config_get.expect_call( > - 'boot.default_args').and_return(base_args) > - self.job.bootloader.remove_kernel.expect_call(tag) > - self.job.bootloader.add_kernel.expect_call( > - image, tag, initrd=initrd, args=bootloader_args, > - root=bootloader_root) > - > - > - def test_add_to_bootloader(self): > - # setup > - self._setup_add_to_bootloader() > - > - # run and check > - self.kernel.add_to_bootloader() > - self.god.check_playback() > - > - > - def test_add_to_bootloader_root_args(self): > - # setup > - args = 'root=/dev/newroot arg3' > - self._setup_add_to_bootloader(args=args, > - base_args='arg1 root=/dev/oldroot > arg2', > - bootloader_args='arg1 arg2 arg3', > - bootloader_root='/dev/newroot') > - > - # run and check > - self.kernel.add_to_bootloader(args=args) > - self.god.check_playback() > - > - > def test_get_kernel_build_arch1(self): > self.construct_kernel() > > @@ -573,53 +625,23 @@ > self.construct_kernel() > self.god.stub_function(self.kernel, "get_kernel_build_ident") > self.god.stub_function(self.kernel, "install") > - self.god.stub_function(self.kernel, "add_to_bootloader") > - self.kernel.applied_patches = "applied_patches" > - when = 1 > - args = '' > - self.kernel.installed_as = False > - > - # record > - self.kernel.get_kernel_build_ident.expect_call().and_return("ident") > - time.time.expect_call().and_return(when) > - args += " IDENT=%d" % (when) > - self.job.next_step_prepend.expect_call(["job.end_reboot_and_verify", > - when, "ident", self.subdir, self.kernel.applied_patches]) > - self.kernel.install.expect_call() > - self.kernel.add_to_bootloader.expect_call(args=args, > - tag=False) > - utils.system.expect_call('touch /fastboot') > - self.job.start_reboot.expect_call() > - self.job.reboot.expect_call(tag=False) > - > - # run and check > - self.kernel.boot() > - self.god.check_playback() > - > - > - def test_boot_without_ident(self): > - self.construct_kernel() > - self.god.stub_function(self.kernel, "get_kernel_build_ident") > - self.god.stub_function(self.kernel, "install") > - self.god.stub_function(self.kernel, "add_to_bootloader") > + self.god.stub_function(self.kernel, "_boot_kernel") > self.kernel.applied_patches = "applied_patches" > - when = 1 > + self.kernel.installed_as = None > args = '' > - self.kernel.installed_as = False > + expected_ident = 'ident' > + ident = True > > # record > - self.kernel.get_kernel_build_ident.expect_call().and_return("ident") > - self.job.next_step_prepend.expect_call(["job.end_reboot", > - self.subdir, "ident", self.kernel.applied_patches]) > self.kernel.install.expect_call() > - self.kernel.add_to_bootloader.expect_call(args=args, > - tag=False) > - utils.system.expect_call('touch /fastboot') > - self.job.start_reboot.expect_call() > - self.job.reboot.expect_call(tag=False) > + self.kernel.get_kernel_build_ident.expect_call( > + ).and_return(expected_ident) > + self.kernel._boot_kernel.expect_call( > + args, ident, expected_ident, > + self.subdir, self.kernel.applied_patches) > > # run and check > - self.kernel.boot(ident=False) > + self.kernel.boot(args=args, ident=ident) > self.god.check_playback() > _______________________________________________ > Autotest mailing list > [email protected] > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
