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.

Sign up for GitHub

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

Jump to bottom

Fix boot from cdrom for svirt/libvirt using qemu #7936

Merged
merged 2 commits into from Aug 6, 2019
Merged

Fix boot from cdrom for svirt/libvirt using qemu #7936

merged 2 commits into from Aug 6, 2019

Conversation

foursixnine
Copy link
Member

@foursixnine foursixnine commented Jul 18, 2019 •
edited

When starting a job that will boot from cdrom using svirt but with qemu instead of xen or hyperv, the SUT will try to still boot from HDD, somehow removing the ternary operator fixes the problem... I guess that it has to do somehow with the hdd being a virtio device and not a the more traditional ide device.

Before: http://phobos.suse.de/tests/1747939#step/bootloader/2

Passing (booting from ISO): http://phobos.suse.de/tests/1747959
Failing (Neither HDD nor ISO defined): http://phobos.suse.de/tests/1747961

Progress ticket: https://progress.opensuse.org/issues/45326

@foursixnine
Copy link
Member Author

@SergioAtSUSE You will need this PR

SergioAtSUSE
SergioAtSUSE reviewed Jul 19, 2019
View reviewed changes
tests/installation/bootloader_svirt.pm Outdated
@@ -82,11 +82,13 @@ sub run {
if (check_var('BOOTFROM', 'c')) {
$boot_device = 'hd';
}
elsif (check_var('BOOTFROM', 'd')) {
elsif (check_var('BOOTFROM', 'd') || (get_var('ISO') && !get_var('BOOT_HDD_IMAGE'))) {
Copy link
Member

Choose a reason for hiding this comment

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

If BOOT_HDD_IMAGE was set, BOOTFROM would be set to 'c', so the first if would take place, and the else if shouldn't. Why do we need to explicitely check for BOOT_HDD_IMAGE not being present?

@mnowaksuse
Copy link

Somehow it's set to boot from HDD in the original fail:

  'args' => [
              'os',
              'boot',
              {
                'dev' => 'hd'
              }
            ],

Looking at HEAD I don't see, how could $boot_device be set to hd given the variables in the job.

mnowaksuse
mnowaksuse reviewed Jul 19, 2019
View reviewed changes
tests/installation/bootloader_svirt.pm
@@ -82,11 +82,13 @@ sub run {
if (check_var('BOOTFROM', 'c')) {
$boot_device = 'hd';
}
elsif (check_var('BOOTFROM', 'd')) {
elsif (check_var('BOOTFROM', 'd') || get_var('ISO')) {
Copy link

Choose a reason for hiding this comment

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

It may look reasonable, but given that our variables like BOOTFROM were used "freely", one needs to run test suites from OSD test suites which are in place to actually verify it. I suggest various test suites from Virtualization groups (VMware & Hyper-V) and Functional (Xen) are run in OSD with this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

hvm - https://openqa.suse.de/tests/3138871
Xen - https://openqa.suse.de/tests/3138914
VMWare - https://openqa.suse.de/tests/3138936

@mnowaksuse can you point to some jobs?

@asdil12 asdil12 merged commit 657a779 into os-autoinst:master Aug 6, 2019
@foursixnine foursixnine deleted the fix_bootfrom branch August 26, 2019 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnowaksuse mnowaksuse mnowaksuse left review comments

@SergioAtSUSE SergioAtSUSE SergioAtSUSE left review comments

@asdil12 asdil12 asdil12 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@foursixnine @mnowaksuse @asdil12 @SergioAtSUSE