Skip to content
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

kpatch: Add subcommand '--enabling' to adjust new sysfs attribute 'st… #1419

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
105 changes: 79 additions & 26 deletions kpatch/kpatch
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ usage () {
echo >&2
usage_cmd "info <module>" "show information about a patch module"
echo >&2
usage_cmd "list" "list installed patch modules"
usage_cmd "list" "list installed patch modules, and list the enabling functions and its relationship from patch module to the function enabling in the system if 'stack_order' attribute is supported."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd simplify this to, "If the system supports the livepatch 'stack_order' sysfs attribute, provide the list of currently livepatched functions".

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this usage text change should be reflected in the manpage file: man/kpatch.1

echo >&2
usage_cmd "signal" "signal/poke any process stalling the current patch transition. This is only useful on systems that have the sysfs livepatch signal interface. On other systems, the signaling should be done automatically by the OS and this subcommand is a no-op."
echo >&2
Expand Down Expand Up @@ -446,6 +446,82 @@ get_module_version() {
MODVER="${MODVER/ */}"
}

show_enabled_function() {
declare -A function_map
for module_dir in "$SYSFS"/*; do
if [[ ! -d "$module_dir" ]] || \
[[ ! -e "$module_dir/stack_order" ]]; then
continue
fi
stack_order=$(cat "$module_dir/stack_order")
module_name=$(basename "$module_dir")
for obj_dir in "$module_dir"/*/; do
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: all the other directory iterators do so on the base directory and then check that the entry is a directory itself (for example, $func_dir). Might as well make it consistent here.

obj_name=$(basename "$obj_dir")
for func_dir in "$obj_dir"/*; do
if [[ ! -d "$func_dir" ]]; then
continue
fi
# we should take pos into account.
func_name_and_pos=$(basename "$func_dir")
key="$obj_name:$func_name_and_pos"
if [[ -z "${function_map[$key]}" ]]; then
function_map[$key]="$stack_order:$module_name"
else
# Update the map only iff this livepatch has a
# higher stack_order value
IFS=':' read -r recorded_order module_name <<< "${function_map[$key]}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a subtle bug here, where module_name is read in from the function_map, but then a few lines later, the map is updated with $stack_order:$module_name:$obj_name so the net effect is that the map gets the previous (now lower stack_order) livepatch module name.

I think the simplest fix would be to rename this sole variable instance like:
IFS=':' read -r recorded_order recorded_module_name ... that way it's read and stashed in an unused temporary variable. The higher stack_order livepatch in question module_name is left intact and the map update should now be accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bravo! I didn't realize that there may have such problem. Thank you for pointing it out!

if [[ $recorded_order -lt $stack_order ]]; then
function_map[$key]="$stack_order:$module_name:$obj_name"
fi
fi
done
done

done

# Pretty print the function map if it has any contents
if [[ ${#function_map[@]} -ne 0 ]]; then
echo ""
echo "Currently livepatched functions:"
declare -a output_data=("Module Object Function/Occurrence")
for key in "${!function_map[@]}"; do
IFS=':' read -r stack_order module_name obj_name <<< "${function_map[$key]}"
obj_name=${key%%:*}
func_name_and_pos=${key##*:}
output_data+=("$module_name $obj_name $func_name_and_pos")
done
printf "%s\n" "${output_data[@]}" | column -t
fi
}

print_patch_info() {
echo "Loaded patch modules:"
for module in "$SYSFS"/*; do
if [[ -e "$module" ]]; then
modname=$(basename "$module")
if [[ "$(cat "$module/enabled" 2>/dev/null)" -eq 1 ]]; then
in_transition "$modname" && state="enabling..." \
|| state="enabled"
else
in_transition "$modname" && state="disabling..." \
|| state="disabled"
fi
echo "$modname [$state]"
fi
done
show_stalled_processes
echo ""
echo "Installed patch modules:"
for kdir in "$INSTALLDIR"/*; do
[[ -e "$kdir" ]] || continue
for module in "$kdir"/*.ko; do
[[ -e "$module" ]] || continue
mod_name "$module"
echo "$MODNAME ($(basename "$kdir"))"
done
done
}

unset MODULE

# Initialize the $SYSFS var. This only works if the core module has been
Expand Down Expand Up @@ -593,31 +669,8 @@ case "$1" in

"list")
[[ "$#" -ne 1 ]] && usage
echo "Loaded patch modules:"
for module in "$SYSFS"/*; do
if [[ -e "$module" ]]; then
modname=$(basename "$module")
if [[ "$(cat "$module/enabled" 2>/dev/null)" -eq 1 ]]; then
in_transition "$modname" && state="enabling..." \
|| state="enabled"
else
in_transition "$modname" && state="disabling..." \
|| state="disabled"
fi
echo "$modname [$state]"
fi
done
show_stalled_processes
echo ""
echo "Installed patch modules:"
for kdir in "$INSTALLDIR"/*; do
[[ -e "$kdir" ]] || continue
for module in "$kdir"/*.ko; do
[[ -e "$module" ]] || continue
mod_name "$module"
echo "$MODNAME ($(basename "$kdir"))"
done
done
print_patch_info
show_enabled_function
;;

"info")
Expand Down
Loading