[PATCH -v2 1/2] report: safely update the result.xml file

[PATCH -v2 1/2] report: safely update the result.xml file

[Theodore Ts'o]

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, 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 check script to be killed while it is in the middle of rewriting
the result.xml file.  If the result.xml file is only partially
written, information regarding the tests run before VM termination
will be lost.

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@mit.edu>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---

Changes from v1:
  * make sure result.xml.new doesn't exist (it could be named pipe
    or a symlink or some other foolishness) before writing to it.
  * fsync result.xml.new before renaming it to result.xml
  * minor updates to the commit description

 common/report | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/common/report b/common/report
index 9bfa09ecc..3c58f0e3c 100644
--- a/common/report
+++ b/common/report
@@ -109,13 +109,16 @@ _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
+	rm -f "$tmp_fn"
 	# Header
-	echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" > $REPORT_DIR/result.xml
+	echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" > "$tmp_fn"
 	if [ -n "$test_start_time" ]; then
 		timestamp="$(date -Iseconds --date="$test_start_time")"
 	else
@@ -123,7 +126,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 +145,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"
+	sync "$tmp_fn" && mv "$tmp_fn" "$out_fn"
+	echo "Xunit report: $out_fn"
 }
 
 _xunit_make_testcase_report()
-- 
2.31.0

[PATCH -v2 2/2] report: remove xmlns specifier

[Theodore Ts'o]

By specifying "xmlns=https://git.kernel.org/.../xfstests-dev.git",
this causes XML complaint parsers, such as the one used by the python
junitparser library, to put all of the XML elements into a namespace,
which then causes junitparser to toss its cookies.

This can be worked-around in a test runner script via:

    sed -i.orig -e 's/xmlns=\".*\"//' "$RESULT_BASE/result.xml"

but it's better not to include the xmlns line at all in the first
place, since this may cause other users of fstests who are using
the Python junitparser library a lot of headaches.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---

changes from v1:
  * minor updates to the commit description

 common/report | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/report b/common/report
index 3c58f0e3c..0e91e481f 100644
--- a/common/report
+++ b/common/report
@@ -128,7 +128,6 @@ _xunit_make_section_report()
 	local fstests_ns="https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git"
 	cat >> "$tmp_fn" << ENDL
 <testsuite
- xmlns="$fstests_ns"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="$fstests_ns $fstests_ns/tree/doc/xunit.xsd"
 
-- 
2.31.0