Skip to content

Add and fix hyperv modules test #3853

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.

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kanchansenlaskar
Copy link
Collaborator

This PR contains the changes:

  1. Addition of other hyperv modules e.g. (hv_vmbus hv_storvsc hv_utils hv_balloon hid_hyperv hyperv_keyboard hyperv_fb)
  2. Also since the test is about reloading the modules, only looking for non-built in modules is not enough, hence added a small change to look specifically for reloadable modules in the config file.
  3. Fixes around the existing hv_netvsc failure where the timout of 10min for the command execution was not honored. And that is because the LocalProcess in the spur lib gets stops running withing the first 50 sec. Hence a workaround for the time being is put to reduce the dmesg log level to 4 if the existing log level is more than 4.

@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch 2 times, most recently from 1cdd16f to 6b9ac2e Compare June 11, 2025 19:37
@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch from 6b9ac2e to 14d92fc Compare June 12, 2025 15:06
@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch from 14d92fc to 747ca1b Compare June 16, 2025 18:09
@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch 2 times, most recently from bdc9e3e to df80e7a Compare June 17, 2025 19:56
[
# Modules which dont have "=y" in the kernel config
# and therefore are not built into the kernel.
"NOT_BUILT_IN",
Copy link
Member

Choose a reason for hiding this comment

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

It should be

  • BUILT_IN =y
  • MODULE =m
  • NOT_BUILD =n

@@ -42,6 +42,15 @@ def _freebsd_tool(cls) -> Optional[Type[Tool]]:
def can_install(self) -> bool:
return False

def generate_renew_command(self, interface: str = "eth0") -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Please also reuse it inside of the dhclient to reduce duplicate code.

@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch from df80e7a to ee2c759 Compare June 30, 2025 09:47
@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch from ee2c759 to d0fefcc Compare July 11, 2025 17:03
).stdout.strip()
)

# self.node.execute(
Copy link
Member

Choose a reason for hiding this comment

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

Remove unused code, instead of comment out.

Path(__file__).parent.joinpath("scripts"), ["modprobe_reloader.sh"]
)

parameters = f'{nohup_output_log_file_name} {loop_process_pid_file_name} {mod_name} {times} {verbose_flag} "{dhclient_renew_command}"'
Copy link
Member

Choose a reason for hiding this comment

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

Add comments above to explain them.

pid_file="$2"
module_name="$3"
times="$4" # 100
verbose="$5" # true/false
Copy link
Member

Choose a reason for hiding this comment

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

Can they have a default value?

sleep 1
ip link set eth0 down >> $log_file 2>&1
ip link set eth0 up >> $log_file 2>&1
dhcpcd -k eth0 >> $log_file 2>&1
Copy link
Member

Choose a reason for hiding this comment

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

Should it use dhclient_renew_command?

'
fi

# sudo sh -c '(for i in \$(seq 1 $times); do modprobe -r $verbose $mod_name >> $nohup_output_log_file_name 2>&1; modprobe $verbose $mod_name >> $nohup_output_log_file_name 2>&1; done; sleep 1; ip link set eth0 down; ip link set eth0 up; $renew_command)' & echo \$! > $loop_process_pid_file_name"
Copy link
Member

Choose a reason for hiding this comment

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

Remove it.

echo \$! > $pid_file'"


sudo nohup sh -c '
Copy link
Member

Choose a reason for hiding this comment

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

How about moving sudo, nohup, sh, and log file redirection outside the script? It could simplify the script by handling logging externally.

)
failed_modules[module] = failure_message

result_message = ""
Copy link
Member

Choose a reason for hiding this comment

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

This line shouldn't be followed by the next line. Use either += without this line, or = to set the value of result_message.

f"Expected {module} to be inserted {loop_count} times"
).is_equal_to(loop_count)
if failed_modules:
raise AssertionError(result_message)
Copy link
Member

Choose a reason for hiding this comment

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

raise LISAException(...

"\nTrying to reconnect to the remote node in 10 sec..."
)

time.sleep(10)
Copy link
Member

Choose a reason for hiding this comment

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

Make it shorter like 2 seconds.

@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch 3 times, most recently from 5412db8 to 0a0b061 Compare July 12, 2025 10:55
@kanchansenlaskar kanchansenlaskar force-pushed the kasenlaskar/hyperv_module_add_and_fix branch from 0a0b061 to d1ae631 Compare July 13, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants