Skip to content

fix server session unbound error #4295

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

Closed
hholoubk wants to merge 2 commits into from
Closed

fix server session unbound error #4295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c50755d
utils-test:Add running htx-cmdline stress_type
Aug 20, 2024
2bb3117
utils_sys: Fix UnboundLocalError for server_session variable
hholoubk Jan 2, 2026
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
1 change: 1 addition & 0 deletions virttest/utils_sys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def get_qemu_log(vms, type="local", params=None, log_lines=10):
:return: list, like [{"vm_name": "vm1", "local": xxx, "remote": xxx}, {"vm_name": "vm2", "local": xxx}]
"""
logs = []
server_session = None
if params is not None and type != "local":
server_ip = params.get("migrate_dest_host", params.get("remote_ip"))
server_user = params.get("server_user", params.get("remote_user"))
Expand Down
117 changes: 91 additions & 26 deletions virttest/utils_test/__init__.py
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,10 @@ def get_time(session, time_command, time_filter_re, time_format):
msg += "'%s', output: %s" % (time_command, output)
raise exceptions.TestError(msg)
if re.search("PM|AM", guest_time):
hour = re.findall("\d+ (\d+):", guest_time)[0]
hour = re.findall(r"\d+ (\d+):", guest_time)[0]
fix = 12 if re.search("PM", guest_time) else 0
hour = str(int(hour) % 12 + fix)
guest_time = re.sub("\s\d+:", " %s:" % hour, guest_time)[:-3]
guest_time = re.sub(r"\s\d+:", " %s:" % hour, guest_time)[:-3]
else:
guest_time = guest_time[:-3]
guest_time = time.mktime(time.strptime(guest_time, time_format))
Expand Down Expand Up @@ -873,7 +873,7 @@ def transfer_data(session, host_cmd, guest_cmd, n_time, timeout, md5_check, acti
else:
LOG.warning(err)
else:
md5_re = "md5_sum = (\w{32})"
md5_re = r"md5_sum = (\w{32})"
try:
md5_guest = re.findall(md5_re, g_output)[0]
except Exception:
Expand All @@ -893,7 +893,7 @@ def transfer_data(session, host_cmd, guest_cmd, n_time, timeout, md5_check, acti
else:
LOG.warning(err)
else:
md5_re = "md5_sum = (\w{32})"
md5_re = r"md5_sum = (\w{32})"
try:
md5_host = re.findall(md5_re, output)[0]
except Exception:
Expand Down Expand Up @@ -1504,10 +1504,10 @@ def extract(vm, remote_path, dest_dir):
def get_last_guest_results_index():
res_index = 0
for subpath in os.listdir(outputdir):
if re.search("guest_autotest_results\d+", subpath):
if re.search(r"guest_autotest_results\d+", subpath):
res_index = max(
res_index,
int(re.search("guest_autotest_results(\d+)", subpath).group(1)),
int(re.search(r"guest_autotest_results(\d+)", subpath).group(1)),
)
return res_index

Expand Down Expand Up @@ -2063,7 +2063,7 @@ def summary_up_result(result_file, ignore, row_head, column_mark):
if len(re.findall(column_mark, eachLine)) != 0 and not head_flag:
column = 0
_, row, eachLine = re.split(row_head, eachLine)
for i in re.split("\s+", eachLine):
for i in re.split(r"\s+", eachLine):
if i:
result_dict[i] = {}
column_list[column] = i
Expand All @@ -2080,7 +2080,7 @@ def summary_up_result(result_file, ignore, row_head, column_mark):
row_list.append(row)
for i in result_dict:
result_dict[i][row] = []
for i in re.split("\s+", eachLine):
for i in re.split(r"\s+", eachLine):
if i:
result_dict[column_list[column]][row].append(i)
column += 1
Expand Down Expand Up @@ -2327,6 +2327,7 @@ def __init__(
self.stress_wait_for_timeout = int(
self.params.get("stress_wait_for_timeout", 60)
)
stress_type = stress_type.split('_')[0]
Copy link

@coderabbitai coderabbitai bot Jan 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Pipeline failure: Black formatting required.

The pre-commit hook indicates this file needs reformatting. The single quotes here may need to be double quotes for consistency with the codebase style.

-        stress_type = stress_type.split('_')[0]
+        stress_type = stress_type.split("_")[0]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
stress_type = stress_type.split('_')[0]
stress_type = stress_type.split("_")[0]
🤖 Prompt for AI Agents 
In virttest/utils_test/__init__.py around line 2330, the string literal uses
single quotes which fails the project's Black/formatting/pre-commit checks;
replace single quotes with double quotes for that literal (change 'stress_type'
usage to use double-quoted string) and re-run Black or pre-commit formatting to
ensure the file is properly reformatted.

self.stress_type = stress_type
stress_cmds = stress_cmds or stress_type
self.stress_cmds = self.params.get("stress_cmds_%s" % stress_type, stress_cmds)
Expand Down Expand Up @@ -2401,6 +2402,45 @@ def load_stress_tool(self):
):
raise exceptions.TestError("Stress app does not " "running as expected")

@session_handler
def load_htxstress_tool(self):
"""
load stress tool in guest
"""
# install htx strss tool
remove_old_package = "cd " + self.dst_path + " && rm -rf " + self.base_name
get_stress = "cd " + self.dst_path + " && wget " + self.download_url
if "rpm" in self.base_name:
install_stress = "cd " + self.dst_path + " && rpm -i " + self.base_name
elif "deb" in self.base_name:
install_stress = "cd " + self.dst_path + " && dpkg -i " + self.base_name
else:
raise exceptions.TestError("Could not install htx stress tool")
LOG.info("Installing htx rpm/deb in the guest")
self.cmd(remove_old_package)
self.cmd(get_stress)
self.cmd(install_stress)
Comment on lines +2419 to +2422
Copy link

@coderabbitai coderabbitai bot Jan 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing error handling for critical installation steps.

The wget and package installation commands are executed without checking their return status. Since self.cmd uses ignore_status=True / ignore_all_errors=True, failures will be silently ignored. If the download or installation fails, the function will proceed to start a stress tool that wasn't installed, resulting in a confusing failure later.

🔎 Proposed fix with error handling 
         LOG.info("Installing htx rpm/deb in the guest")
-        self.cmd(remove_old_package)
-        self.cmd(get_stress)
-        self.cmd(install_stress)
+        self.cmd(remove_old_package)
+        
+        status, output = self.cmd_status_output(get_stress, timeout=self.stress_shell_timeout)
+        if status != 0:
+            raise exceptions.TestError("Failed to download HTX stress tool: %s" % output)
+        
+        status, output = self.cmd_status_output(install_stress, timeout=self.stress_shell_timeout)
+        if status != 0:
+            raise exceptions.TestError("Failed to install HTX stress tool: %s" % output)

Committable suggestion skipped: line range outside the PR's diff.


# start htx stress
launch_cmds = "nohup %s %s > /dev/null &" % (self.stress_cmds, self.stress_args)
LOG.info("Launch stress with command: %s", launch_cmds)
try:
self.cmd_launch(launch_cmds)
# The background process sometimes does not return to
# terminate, if timeout, send a blank line afterward
except aexpect.ShellTimeoutError:
self.cmd_launch("")

# wait for stress to start and then check, if not raise TestError
if not utils_misc.wait_for(
self.app_running,
self.stress_wait_for_timeout,
first=2.0,
text="wait for stress app to start",
step=1.0,
):
raise exceptions.TestError("Stress app does not " "running as expected")

@session_handler
def unload_stress(self):
"""
Expand Down Expand Up @@ -2791,25 +2831,50 @@ def load_stress(
:param download_type: currently support "git" or "file" download
"""
fail_info = []
# Add stress/iozone tool in vms
if stress_type in ["stress_in_vms", "iozone_in_vms"]:
default_stress_type = stress_type
if stress_type in ["stress_in_vms", "iozone_in_vms", "htxcmdline_in_vms"]:
for vm in vms:
try:
vstress = VMStress(
vm,
stress_type.split("_")[0],
params,
download_url,
make_cmds,
stress_cmds,
stress_args,
work_path,
uninstall_cmds,
download_type=download_type,
)
vstress.load_stress_tool()
except StressError as detail:
fail_info.append("Launch stress in %s failed: %s" % (vm.name, detail))
stress_type = params.get("stress_type_%s" % vm.name, default_stress_type)
if stress_type == "htxcmdline_in_vms":
# Add htx stress tool in vms
try:
vstress = VMStress(
vm,
stress_type.split("_")[0],
params,
download_url,
make_cmds,
stress_cmds,
stress_args,
work_path,
uninstall_cmds,
download_type=download_type,
)
vstress.load_htxstress_tool()
except StressError as detail:
fail_info.append(
"Launch htxstress in %s failed: %s" % (vm.name, detail)
)
else:
# Add stress/iozone tool in vms
try:
vstress = VMStress(
vm,
stress_type.split("_")[0],
params,
download_url,
make_cmds,
stress_cmds,
stress_args,
work_path,
uninstall_cmds,
download_type=download_type,
)
vstress.load_stress_tool()
except StressError as detail:
fail_info.append(
"Launch stress in %s failed: %s" % (vm.name, detail)
)
# Add stress for host
elif stress_type == "stress_on_host" or stress_type == "stress_on_remote_host":
try:
Expand Down
Loading