Skip to content

avocado-vt: add support for shim parameter #4355

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

Open
XueqiangWei wants to merge 1 commit into
base: master
Choose a base branch
from
Open

avocado-vt: add support for shim parameter #4355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions virttest/qemu_vm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1438,6 +1438,9 @@ def add_kernel(filename):
def add_initrd(filename):
return " -initrd '%s'" % filename

def add_shim(filename):
return " -shim '%s'" % filename

def add_rtc(devices):
# Pay attention that rtc-td-hack is for early version
# if "rtc " in help:
Expand Down Expand Up @@ -2933,6 +2936,11 @@ def __iothread_conflict_check(params):
initrd = utils_misc.get_path(data_dir.get_data_dir(), initrd)
devices.insert(StrDev("initrd", cmdline=add_initrd(initrd)))

shim = params.get("shim")
if shim:
shim = utils_misc.get_path(data_dir.get_data_dir(), shim)
devices.insert(StrDev("shim", cmdline=add_shim(shim)))

for host_port, guest_port in redirs:
cmd = add_tcp_redir(devices, host_port, guest_port)
devices.insert(StrDev("tcp-redir", cmdline=cmd))
Expand Down
1 change: 1 addition & 0 deletions virttest/shared/cfg/guest-os/Linux/RHEL.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# kernel_params += ":sr1:/ks.cfg"
kernel_params += " nicdelay=60 "
boot_path = images/pxeboot
shim_target_efi_path = EFI/BOOT
aarch64:
kernel_params += " earlyprintk=pl011,0x9000000 console=ttyAMA0 debug ignore_loglevel rootwait"
ppc64, ppc64le:
Expand Down
38 changes: 38 additions & 0 deletions virttest/tests/unattended_install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ def __init__(self, test, params, vm):
"driver_in_floppy",
"vga",
"unattended_file_kernel_param_name",
"shim_target_efi_path",
"shim",
]

for a in self.attributes:
Expand Down Expand Up @@ -266,6 +268,8 @@ def get_unattended_file(backend):
self.kernel = os.path.join(root_dir, self.kernel)
if getattr(self, "initrd"):
self.initrd = os.path.join(root_dir, self.initrd)
if getattr(self, "shim"):
self.shim = os.path.join(root_dir, self.shim)

if self.medium == "nfs":
self.nfs_mount = tempfile.mkdtemp(prefix="nfs_", dir=self.tmpdir)
Expand Down Expand Up @@ -984,6 +988,11 @@ def setup_cdrom(self):
assert os.path.getsize(self.kernel) > 0
i.copy(os.path.join(self.boot_path, os.path.basename(self.initrd)), self.initrd)
assert os.path.getsize(self.initrd) > 0
i.copy(
os.path.join(self.shim_target_efi_path, os.path.basename(self.shim)),
self.shim,
)
assert os.path.getsize(self.shim) > 0

if self.unattended_file.endswith(".preseed"):
self.preseed_initrd()
Expand Down Expand Up @@ -1063,6 +1072,7 @@ def setup_url(self):

kernel_basename = os.path.basename(self.kernel)
initrd_basename = os.path.basename(self.initrd)
shim_basename = os.path.basename(self.shim)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 6, 2026

Choose a reason for hiding this comment

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

medium

Accessing self.shim here without checking if it's set can lead to issues if the parameter is missing or empty. It should be guarded to avoid errors in subsequent logic.

Suggested change
shim_basename = os.path.basename(self.shim)
shim_basename = os.path.basename(self.shim) if self.shim else ""

sha1sum_kernel_cmd = "sha1sum %s" % kernel_basename
sha1sum_kernel_output = process.run(
sha1sum_kernel_cmd, ignore_status=True, verbose=DEBUG
Expand All @@ -1081,12 +1091,24 @@ def setup_url(self):
except IndexError:
sha1sum_initrd = ""

sha1sum_shim_cmd = "sha1sum %s" % shim_basename
sha1sum_shim_output = process.run(
sha1sum_shim_cmd, ignore_status=True, verbose=DEBUG
).stdout_text
try:
sha1sum_shim = sha1sum_shim_output.split()[0]
except IndexError:
sha1sum_shim = ""
Comment on lines +1094 to +1101
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 6, 2026

Choose a reason for hiding this comment

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

high

The sha1sum calculation for the shim should only occur if self.shim is defined. If the calculation fails, the test should fail explicitly rather than falling back to an empty string, as per the repository guidelines to avoid masking configuration issues.

Suggested change
sha1sum_shim_cmd = "sha1sum %s" % shim_basename
sha1sum_shim_output = process.run(
sha1sum_shim_cmd, ignore_status=True, verbose=DEBUG
).stdout_text
try:
sha1sum_shim = sha1sum_shim_output.split()[0]
except IndexError:
sha1sum_shim = ""
sha1sum_shim = ""
if self.shim:
sha1sum_shim_cmd = "sha1sum %s" % shim_basename
sha1sum_shim_output = process.run(
sha1sum_shim_cmd, verbose=DEBUG
).stdout_text
sha1sum_shim = sha1sum_shim_output.split()[0]
References
  1. When a test's intent is to use a specific feature (e.g., an OVS bridge), it should fail explicitly if that feature or its dependencies are unavailable. Do not silently ignore the error or fall back to a different behavior, as this can mask configuration issues and lead to misleading results.


url_kernel = os.path.join(
self.url, self.boot_path, os.path.basename(self.kernel)
)
url_initrd = os.path.join(
self.url, self.boot_path, os.path.basename(self.initrd)
)
url_shim = os.path.join(
self.url, self.shim_target_efi_path, os.path.basename(self.shim)
)
Comment on lines +1109 to +1111
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 6, 2026

Choose a reason for hiding this comment

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

high

The url_shim construction should be guarded by a check for self.shim and initialized to an empty string to prevent errors in the download block.

Suggested change
url_shim = os.path.join(
self.url, self.shim_target_efi_path, os.path.basename(self.shim)
)
url_shim = ""
if self.shim:
url_shim = os.path.join(
self.url, self.shim_target_efi_path, os.path.basename(self.shim)
)


if not sha1sum_kernel == self.params.get("sha1sum_vmlinuz", None):
if os.path.isfile(self.kernel):
Expand All @@ -1106,6 +1128,15 @@ def setup_url(self):
os.path.join(self.image_path, os.path.basename(self.initrd)),
)

if not sha1sum_shim == self.params.get("sha1sum_shim", None):
if os.path.isfile(self.shim):
os.remove(self.shim)
LOG.info("Downloading %s -> %s", url_shim, self.image_path)
download.get_file(
url_shim,
os.path.join(self.image_path, os.path.basename(self.shim)),
)
Comment on lines +1131 to +1138
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 6, 2026

Choose a reason for hiding this comment

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

high

The download logic for the shim must be guarded to avoid errors when self.shim is not used. Without this guard, the code will attempt to download from an invalid URL for every installation.

Suggested change
if not sha1sum_shim == self.params.get("sha1sum_shim", None):
if os.path.isfile(self.shim):
os.remove(self.shim)
LOG.info("Downloading %s -> %s", url_shim, self.image_path)
download.get_file(
url_shim,
os.path.join(self.image_path, os.path.basename(self.shim)),
)
if self.shim and not sha1sum_shim == self.params.get("sha1sum_shim", None):
if os.path.isfile(self.shim):
os.remove(self.shim)
LOG.info("Downloading %s -> %s", url_shim, self.image_path)
download.get_file(
url_shim,
os.path.join(self.image_path, os.path.basename(self.shim)),
)


if "repo=cdrom" in self.kernel_params:
# Red Hat
self.kernel_params = re.sub(
Expand Down Expand Up @@ -1154,6 +1185,13 @@ def setup_nfs(self):
self.image_path,
)
process.run(initrd_fetch_cmd, verbose=DEBUG)
shim_fetch_cmd = "cp %s/%s/%s %s" % (
self.nfs_mount,
self.shim_target_efi_path,
os.path.basename(self.shim),
self.image_path,
)
process.run(shim_fetch_cmd, verbose=DEBUG)
Comment on lines +1188 to +1194
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 6, 2026

Choose a reason for hiding this comment

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

high

Fetching the shim via NFS should be conditional on self.shim being set. Unconditional execution will cause the cp command to fail for installations that do not provide a shim.

Suggested change
shim_fetch_cmd = "cp %s/%s/%s %s" % (
self.nfs_mount,
self.shim_target_efi_path,
os.path.basename(self.shim),
self.image_path,
)
process.run(shim_fetch_cmd, verbose=DEBUG)
if self.shim:
shim_fetch_cmd = "cp %s/%s/%s %s" % (
self.nfs_mount,
self.shim_target_efi_path,
os.path.basename(self.shim),
self.image_path,
)
process.run(shim_fetch_cmd, verbose=DEBUG)

finally:
utils_disk.cleanup(self.nfs_mount)

Expand Down
Loading