| Message ID | 0d3655d6-8551-486b-85ca-e64378231278@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | libxl: avoid infinite loop in libxl__remove_directory() | expand |
On 06.03.25 12:25, Jan Beulich wrote: > Infinitely retrying the rmdir() invocation makes little sense. While the > original observation was the log filling the disk (due to repeated > "Directory not empty" errors, in turn occurring for unclear reasons), > the loop wants breaking even if there was no error message being logged > (much like is done in the similar loops in libxl__remove_file() and > libxl__remove_file_or_directory()). > > Fixes: c4dcbee67e6d ("libxl: provide libxl__remove_file et al") > Signed-off-by: Jan Beulich <jbeulich@suse.com> While probably a more sane solution by reworking this whole mess is possible, this patch should solve the issue Jan has described. So: Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
--- a/tools/libs/light/libxl_utils.c +++ b/tools/libs/light/libxl_utils.c @@ -577,6 +577,7 @@ int libxl__remove_directory(libxl__gc *g if (errno == EINTR) continue; LOGE(ERROR, "failed to remove emptied directory %s", dirpath); rc = ERROR_FAIL; + break; } out:
Infinitely retrying the rmdir() invocation makes little sense. While the original observation was the log filling the disk (due to repeated "Directory not empty" errors, in turn occurring for unclear reasons), the loop wants breaking even if there was no error message being logged (much like is done in the similar loops in libxl__remove_file() and libxl__remove_file_or_directory()). Fixes: c4dcbee67e6d ("libxl: provide libxl__remove_file et al") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- This is the simplest possible adjustment. Depending on why there were retries, retrying a few times may make sense. But then, especially for the specific error observed, presumably only after having tried to empty the directory another time. It's further questionable whether trying the rmdir() makes sense at all when emptying the directory failed. After all failure of opendir() also results in bailing from the function without trying to rmdir(). If this makes sense, then I further think that "rc" would want resetting ahead of this final loop in the function: If the rmdir() succeeds despite earlier errors, all is (kind of) fine.