Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 4 Feb 2022 12:43:03 -0800
From:   David Vernet <void@manifault.com>
To:     Joe Lawrence <joe.lawrence@redhat.com>
Cc:     live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
        jpoimboe@redhat.com, pmladek@suse.com, jikos@kernel.org,
        mbenes@suse.cz, corbet@lwn.net, kernel-team@fb.com
Subject: Re: [PATCH] livepatch: Skip livepatch tests if ftrace cannot be
 configured
Message-ID: <20220204204303.scyxvt4yh2gjftce@dev0025.ash9.facebook.com>
References: <20220203233205.1554034-1-void@manifault.com>
 <917650f7-4e8c-4643-77d3-45d1ebbb217f@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <917650f7-4e8c-4643-77d3-45d1ebbb217f@redhat.com>
User-Agent: NeoMutt/20211029
Precedence: bulk

On Fri, Feb 04, 2022 at 03:30:02PM -0500, Joe Lawrence wrote:

Hi Joe,

Thanks for the review.

> sp: s/permament/permanent
> 

Ack, thanks for catching that.

> > +	if [[ "$result" == *"Device or resource busy"* ]]; then
> > +		skip "failed to set kernel.ftrace_enabled=$1"
> > +	fi
> > +
> 
> style nit: move the blank line from here to just before the new # Skip
> comment

Ack.

> Can we be more paranoid and just look for the exact result that we expect:
> 
> if [[ "$result" != "kernel.ftrace_enabled = 1" ]]; then
> 	skip "failed to set kernel.ftrace_enabled=$1"
> fi

Agreed, this is cleaner and less brittle.

I'll follow-up with a v2 patch shortly. Thanks!

- David