On Tue, Jul 09, 2024 at 12:10:23PM -0700, Wengang Wang wrote:
> Add this handler to break the defrag better, so it has
> 1. the stats reporting
> 2. remove the temporary file
>
> Signed-off-by: Wengang Wang <wen.gang.wang@xxxxxxxxxx>
> ---
> spaceman/defrag.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/spaceman/defrag.c b/spaceman/defrag.c
> index 9f11e36b..61e47a43 100644
> --- a/spaceman/defrag.c
> +++ b/spaceman/defrag.c
> @@ -297,6 +297,13 @@ get_time_delta_us(struct timeval *pre_time, struct timeval *cur_time)
> return us;
> }
>
> +static volatile bool usedKilled = false;
> +void defrag_sigint_handler(int dummy)
> +{
> + usedKilled = true;
Not sure why some of these variables are camelCase and others not.
Or why this global variable doesn't have a g_ prefix like the others?
> + printf("Please wait until current segment is defragmented\n");
Is it actually safe to call printf from a signal handler? Handlers must
be very careful about what they call -- regreSSHion was a result of
openssh not getting this right.
(Granted spaceman isn't as critical...)
Also would you rather SIGINT merely terminate the spaceman process? I
think the file locks drop on termination, right?
--D
> +};
> +
> /*
> * defragment a file
> * return 0 if successfully done, 1 otherwise
> @@ -345,6 +352,8 @@ defrag_xfs_defrag(char *file_path) {
> goto out;
> }
>
> + signal(SIGINT, defrag_sigint_handler);
> +
> do {
> struct timeval t_clone, t_unshare, t_punch_hole;
> struct defrag_segment segment;
> @@ -434,7 +443,7 @@ defrag_xfs_defrag(char *file_path) {
> if (time_delta > max_punch_us)
> max_punch_us = time_delta;
>
> - if (stop)
> + if (stop || usedKilled)
> break;
> } while (true);
> out:
> --
> 2.39.3 (Apple Git-146)
>
>
