avocado-vt: add support for shim parameter

[XueqiangWei]

Signed-off-by: Xueqiang Wei xuwei@redhat.com
ID: 5099

[gemini-code-assist]

Code Review

This pull request introduces support for a "shim" bootloader component within unattended installations and QEMU VM configurations, including updates to RHEL configuration and setup logic for CDROM, URL, and NFS. The review feedback identifies several instances where the new shim-related logic lacks necessary conditional guards, which could lead to failures in installations where a shim is not utilized. Suggestions are provided to ensure these operations only occur when a shim is explicitly defined.

[gemini-code-assist]

The shim file is copied and its size is asserted without checking if the shim attribute is actually set. This will cause failures in unattended installations that do not use a shim (where self.shim is an empty string).

Suggested change

	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.shim:
	i.copy(os.path.join(self.shim_target_efi_path, os.path.basename(self.shim)), self.shim)
	assert os.path.getsize(self.shim) > 0

[gemini-code-assist]

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.

[gemini-code-assist]

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)
	)

[gemini-code-assist]

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)),
	)

[gemini-code-assist]

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)

[gemini-code-assist]

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 ""