From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E79CAC001B0 for ; Fri, 7 Jul 2023 15:01:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232974AbjGGPBe (ORCPT ); Fri, 7 Jul 2023 11:01:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43522 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233227AbjGGPBc (ORCPT ); Fri, 7 Jul 2023 11:01:32 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4E2E1FE2 for ; Fri, 7 Jul 2023 08:01:31 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6248C61900 for ; Fri, 7 Jul 2023 15:01:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B52BCC433C7; Fri, 7 Jul 2023 15:01:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1688742090; bh=WBXI6iimnhNE0Z+C9D2kJXNvyBBBZL6KGvLBxgOgDeQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=TF3bNs98Zj1wvctm58Sz9erJoveH/tlC/HEzAfj+r4ApWkfmAbKiacwSNLrYuvJgL s+IOruzulOAcrnXCo0khzoE5coQOsdchGvmxgKUQGHPK6yLB/n48UgseKg15FZqfg+ 1cOwophg5da7RDzRaWzRmpnNzOOu7Jp1kYOl5wKadO+Q6jrV3ozmIN/Xw8PePk/FjO iEFcB9IabG4AYI6pax4/DsmOdTJZwOjaW5yykwTPyr+LGvHciHUAHH/Zq1JRZ1o+TI sEDkACCSNE8iDTw49biQ1RQCNlfxoYF2ki7VDHPxvUjq6txnDYyhSANOn+LVMfJtH5 /0H4/icIMSfKg== Date: Fri, 7 Jul 2023 08:01:30 -0700 From: "Darrick J. Wong" To: Theodore Ts'o Cc: fstests@vger.kernel.org Subject: Re: [PATCH 1/2] report: safely update the result.xml file Message-ID: <20230707150130.GA11442@frogsfrogsfrogs> References: <20230706204232.1577765-1-tytso@mit.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230706204232.1577765-1-tytso@mit.edu> Precedence: bulk List-ID: X-Mailing-List: fstests@vger.kernel.org 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 > --- > 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 "" > $REPORT_DIR/result.xml > + echo "" > "$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 > xmlns="$fstests_ns" > xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" > @@ -142,19 +144,20 @@ ENDL > __generate_report_vars > > # Properties > - echo -e "\t" >> $REPORT_DIR/result.xml > + echo -e "\t" >> "$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" >> $REPORT_DIR/result.xml > + done) | sort >> "$tmp_fn" > + echo -e "\t" >> "$tmp_fn" > if [ -f $report ]; then > - cat $report >> $REPORT_DIR/result.xml > + cat $report >> "$tmp_fn" > fi > - echo "" >> $REPORT_DIR/result.xml > - echo "Xunit report: $REPORT_DIR/result.xml" > + echo "" >> "$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 --D > + echo "Xunit report: $out_fn" > } > > _xunit_make_testcase_report() > -- > 2.31.0 >