Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to tty1 multi-times in ro snapshot #5629

Merged
merged 1 commit into from Aug 24, 2018

Conversation

mitiao
Copy link
Contributor

@mitiao mitiao commented Aug 23, 2018

This MAY fix some sporadic failures during booting read-only snapshot at snapper rollback stage.
The failure is hard to reproduce, sometimes it would not appear if we give another run.

@mitiao mitiao force-pushed the poo#40154 branch 2 times, most recently from 6036501 to a0b98ab Compare August 23, 2018 08:34
lib/migration.pm Outdated
else {
record_info('not in tty1', 'switch to tty1 failed', result => 'softfail');
}
die "Boot into read-only snapshot failed over 5 minutes, considering a product issue" if $i == $retries;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simply put this out of the loop unconditionally to avoid check on every iteration and return once tty1-selected matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i have optimized a little

lib/migration.pm Outdated
record_info('not in tty1', 'switch to tty1 failed', result => 'softfail');
}
die "Boot into read-only snapshot failed over 5 minutes, considering a product issue" if $i == $retries;
sleep 10;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also put this in checkscreen, and have timeout there, so we have chance to match it earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use checkscreen here is no sense, tty switch by sending key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusion, that's not what I mean, there is already check-screen above, instead of sleep we could wait longer to match tty1 instead of idle time.
Imagine following, tty1 appear in 5 seconds, with your current implementation we will wait 12 seconds before we match it, if we put 10 seconds, we will match it immediately.
In case it doesn't appear we will still wait 10 seconds before retry. Hope the point is clear now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, hate checkscreen :)

This may fix some sporadic failures during booting
read-only snapshot at snapper rollback stage.

See: https://progress.opensuse.org/issues/40154
@rwx788 rwx788 merged commit 8906d9a into os-autoinst:master Aug 24, 2018
@mitiao mitiao deleted the poo#40154 branch August 24, 2018 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants