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;
Message-ID: <cb468a17-c653-b257-8f5d-bf5981c22de5@grimberg.me>
Date:   Thu, 17 Aug 2023 11:33:36 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH blktests v3 13/13] nvme: Introduce
 nvmet_target_{setup/cleanup} common code
Content-Language: en-US
To:     Daniel Wagner <dwagner@suse.de>
Cc:     linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-block@vger.kernel.org, Chaitanya Kulkarni <kch@nvidia.com>,
        Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>,
        Hannes Reinecke <hare@suse.de>,
        James Smart <jsmart2021@gmail.com>,
        Bart Van Assche <bvanassche@acm.org>
References: <20230811093614.28005-1-dwagner@suse.de>
 <20230811093614.28005-14-dwagner@suse.de>
 <58d299c4-84e1-603d-6c99-15d0484f9609@grimberg.me>
 <lfr3tkzz3sqnsw4y7sqyxc42ptvzkyfqfpeko6gzs42jdgjl2d@u5fbzjksbztd>
From:   Sagi Grimberg <sagi@grimberg.me>
In-Reply-To: <lfr3tkzz3sqnsw4y7sqyxc42ptvzkyfqfpeko6gzs42jdgjl2d@u5fbzjksbztd>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


>> It is very very strange that _setup returns a port
>> which is passed to _cleanup...
> 
> This is the only information the _cleanup helper needs and that is why
> it survived the refactoring so far.

Doesn't change the fact that it is awkward.

>> I think that _cleanup should simply remove all
>> ports, and that setup should not return a port
>> to begin with.
> 
> This assumes that blktests is the single user and can blindly remove
> everything. I would like to play nice here and only cleanup resources
> blktests actually allocates.

I suggest that the test records resources that it is using
and destroy them in the cleanup (IIRC Shinichiro did something
similar with the modules in some other place).

>> If someone needs the actual port number, then it
>> should either not use this _setup helper or
>> query it somehow.
> 
> I try to figure out how to implement such a query helper then.

That would probably be needed when a test wants to cherry-pick
which stuff it is destroying. But on the other hand, in this
case it also doesn't make sense that this test will be using
the default basic setup function. So I don't think it is
very much needed.