On Thu, Jul 06, 2023 at 04:42:31PM -0400, Theodore Ts'o wrote:
> After every single test, we rewrite result.xml from scratch. This
> ensures that the XML file is always in a valid, parseable state, even
> if the check script is killed or the machine crashes in the middle of
> a test.
>
> If the test is being run in a Cloud VM as a "spot" (Amazon, Azure, or
> GCE) or "preemptible" (Oracle) instance, it is possible that the VM
> can be halted whenever the Cloud provider needs the capacity for
> customers who are willing to pay full price. ("Spot" instances can be
> 60% to 90% cheaper --- allowing the frugal kernel developer to get up
> to 10 times more testing for the same amount of money. :-)
>
> Since a "spot" VM can get terminated at any time, it is possible for
> the VM to be terminated immediately after a test has completed and
> while the result.xml file is in the middle of being written out. In
> that case, the result.xml file could partially written, resulting in
> an invalid result.xml file and lost information about the tests run
> before the VM was terminated.
>
> To address this race, write the new result.xml file as result.xml.new,
> and only rename it to result.xml after the XML file is fully written
> out.
>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
> common/report | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/common/report b/common/report
> index 9bfa09ecc..3ad14f94e 100644
> --- a/common/report
> +++ b/common/report
> @@ -109,13 +109,15 @@ _xunit_make_section_report()
> local notrun_count="$4"
> local sect_time="$5"
> local timestamp
> + local tmp_fn="$REPORT_DIR/result.xml.new"
> + local out_fn="$REPORT_DIR/result.xml"
>
> if [ $sect_name == '-no-sections-' ]; then
> sect_name='global'
> fi
> local report=$tmp.report.xunit.$sect_name.xml
> # Header
> - echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" > $REPORT_DIR/result.xml
> + echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" > "$tmp_fn"
Nit: You might want to rm -f $tmp_fn first to reduce the nastiness if
someone plants a named pipe at that path.
> if [ -n "$test_start_time" ]; then
> timestamp="$(date -Iseconds --date="$test_start_time")"
> else
> @@ -123,7 +125,7 @@ _xunit_make_section_report()
> fi
>
> local fstests_ns="https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git";
> - cat >> $REPORT_DIR/result.xml << ENDL
> + cat >> "$tmp_fn" << ENDL
> <testsuite
> xmlns="$fstests_ns"
> xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
> @@ -142,19 +144,20 @@ ENDL
> __generate_report_vars
>
> # Properties
> - echo -e "\t<properties>" >> $REPORT_DIR/result.xml
> + echo -e "\t<properties>" >> "$tmp_fn"
> (for key in "${!REPORT_VARS[@]}"; do
> _xunit_add_property "$key" "${REPORT_VARS["$key"]}"
> done;
> for p in "${REPORT_ENV_LIST[@]}"; do
> _xunit_add_property "$p" "${!p}"
> - done) | sort >> $REPORT_DIR/result.xml
> - echo -e "\t</properties>" >> $REPORT_DIR/result.xml
> + done) | sort >> "$tmp_fn"
> + echo -e "\t</properties>" >> "$tmp_fn"
> if [ -f $report ]; then
> - cat $report >> $REPORT_DIR/result.xml
> + cat $report >> "$tmp_fn"
> fi
> - echo "</testsuite>" >> $REPORT_DIR/result.xml
> - echo "Xunit report: $REPORT_DIR/result.xml"
> + echo "</testsuite>" >> "$tmp_fn"
> + mv "$tmp_fn" "$out_fn"
Second nit: Make sure we actually wrote tmp_fn before blowing away the
old report.
sync "$tmp_fn" && mv "$tmp_fn" "$out_fn"
With that fixed,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> + echo "Xunit report: $out_fn"
> }
>
> _xunit_make_testcase_report()
> --
> 2.31.0
>
