Skip to content

Enhancement for set_remote_log function in migration base_step.py and… #6784

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

Draft
nanli1 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Enhancement for set_remote_log function in migration base_step.py and… #6784

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
22 changes: 15 additions & 7 deletions provider/migration/base_steps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ def run_migration(self):
"dest_uri": dest_uri, "options": options, "virsh_options": virsh_options,
"extra": extra, "action_during_mig": action_during_mig, "extra_args": extra_args}
migration_base.do_migration(**do_mig_param)
if self.params.get_boolean("set_remote_libvirtd_log"):
utils_sys.display_remote_log(self.params, self.test)
Comment on lines +190 to +191
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Search for display_remote_log definition and imports
rg -n "display_remote_log" --type-list

Repository: autotest/tp-libvirt

Length of output: 452

🏁 Script executed:

# Find where display_remote_log is defined and used
rg -n "def display_remote_log|display_remote_log" -A 2

Repository: autotest/tp-libvirt

Length of output: 240

🏁 Script executed:

# Check the imports and context in base_steps.py around lines 190-191
head -200 provider/migration/base_steps.py | tail -30

Repository: autotest/tp-libvirt

Length of output: 1701

🏁 Script executed:

# Also check imports at the top of the file
head -30 provider/migration/base_steps.py

Repository: autotest/tp-libvirt

Length of output: 813

🏁 Script executed:

# Search for the definition of display_remote_log function
rg -n "def display_remote_log"

Repository: autotest/tp-libvirt

Length of output: 45

🏁 Script executed:

# Check if utils_sys module exists and search for display_remote_log there
find . -name "utils_sys.py" -o -name "utils_sys" -type d

Repository: autotest/tp-libvirt

Length of output: 45

🏁 Script executed:

# Search in virttest for display_remote_log
rg -n "display_remote_log" . --type py -B 2 -A 5

Repository: autotest/tp-libvirt

Length of output: 667

🏁 Script executed:

# Check git history for references to display_remote_log removal or changes
git log --all --oneline | head -20

Repository: autotest/tp-libvirt

Length of output: 1515

🏁 Script executed:

# Search for any comments or TODOs about display_remote_log
rg -n "display_remote_log|libvirtd_log" -B 3 -A 3 --type py

Repository: autotest/tp-libvirt

Length of output: 50375

🏁 Script executed:

# Check if there's a CHANGES file or documentation about API changes
find . -name "CHANGES*" -o -name "NEWS*" -o -name "README*" | head -5

Repository: autotest/tp-libvirt

Length of output: 97

Remove the call to the revoked display_remote_log function at line 191.

The commit 7059b4f51 explicitly revoked utils_sys.display_remote_log, yet line 191 still calls this non-existent function. This will cause an AttributeError at runtime. The enhancement introduced set_remote_log() as the replacement mechanism—if remote log display is needed, use that pattern instead or remove this call entirely.

🤖 Prompt for AI Agents 
In `@provider/migration/base_steps.py` around lines 190 - 191, The code calls the
removed function utils_sys.display_remote_log inside the if-check
self.params.get_boolean("set_remote_libvirtd_log"); remove that call or replace
it with the new pattern using set_remote_log() — locate the conditional in
base_steps.py and either delete the utils_sys.display_remote_log(...) invocation
or invoke utils_sys.set_remote_log(self.params, self.test) (or the appropriate
set_remote_log signature) so the deprecated function is not referenced.



def run_migration_again(self):
"""
Expand Down Expand Up @@ -384,14 +387,15 @@ def set_remote_log(self):
Set remote libvirtd log file

"""
log_level = self.params.get("libvirtd_debug_level")
log_file = self.params.get("libvirtd_debug_file")
log_filters = self.params.get("libvirtd_debug_filters")
remote_file_type = self.params.get("remote_file_type")
server_ip = self.params.get("server_ip")
server_user = self.params.get("server_user")
server_pwd = self.params.get("server_pwd")
log_level = self.params.get("libvirtd_debug_level", "1")
log_file = self.params.get("libvirtd_debug_file", "/var/log/libvirt/virtqemud.log")
log_filters = self.params.get("libvirtd_debug_filters", "1:*")
remote_file_type = self.params.get("remote_file_type", "virtqemud")
server_ip = self.params.get("server_ip", self.params.get("migrate_dest_host)"))
server_user = self.params.get("server_user", "root")
server_pwd = self.params.get("server_pwd", self.params.get("migrate_dest_pwd"))

self.test.log.debug(f"Start setting {remote_file_type} log on remote host")
Comment on lines +390 to +398
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix the server_ip default syntax error and propagate resolved defaults.

Line 394 has a mismatched quote/paren, which is a SyntaxError. Also, remote.run_remote_cmd and update_remote_file read from self.params, so defaults won’t apply if those keys are missing unless you write them back.

🛠️ Proposed fix 
-        server_ip = self.params.get("server_ip", self.params.get("migrate_dest_host)"))
-        server_user = self.params.get("server_user", "root")
-        server_pwd = self.params.get("server_pwd", self.params.get("migrate_dest_pwd"))
+        server_ip = self.params.get("server_ip", self.params.get("migrate_dest_host"))
+        server_user = self.params.get("server_user", "root")
+        server_pwd = self.params.get("server_pwd", self.params.get("migrate_dest_pwd"))
+        # Ensure downstream helpers that read params see the resolved defaults
+        self.params.update({
+            "server_ip": server_ip,
+            "server_user": server_user,
+            "server_pwd": server_pwd,
+        })
🤖 Prompt for AI Agents 
In `@provider/migration/base_steps.py` around lines 390 - 398, There's a syntax
error in the server_ip default: change self.params.get("server_ip",
self.params.get("migrate_dest_host)")) to self.params.get("server_ip",
self.params.get("migrate_dest_host")) and then propagate defaults back into
self.params so subsequent calls use them; e.g. after computing
server_ip/server_pwd/server_user/remote_file_type/log_level/log_file/log_filters
assign them into self.params (self.params["server_ip"]=server_ip etc.) so
remote.run_remote_cmd and update_remote_file read the resolved values. Ensure
the corrected keys are used where referenced (server_ip, server_pwd,
server_user, remote_file_type) to avoid missing-key behavior.

service_name = utils_libvirtd.Libvirtd(remote_file_type).service_name
file_path = utils_config.get_conf_obj(service_name).conf_path
self.test.log.debug("Config file path: %s" % file_path)
Expand All @@ -405,6 +409,10 @@ def set_remote_log(self):
username=server_user,
password=server_pwd)
utils_libvirtd.Libvirtd(remote_file_type, session=remote_runner.session).restart()
self.params.update({
"remote_session": remote_runner.session
})


def check_local_and_remote_log(self, local_str_in_log=True, remote_str_in_log=True):
"""
Expand Down
Loading