public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Kanchan Joshi <joshi.k@samsung.com>
Cc: "hch@lst.de" <hch@lst.de>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH blktests] nvme/046: add test for unprivileged passthrough
Date: Fri, 10 Feb 2023 02:01:14 +0000	[thread overview]
Message-ID: <20230210020114.zzmazatkxeomowxq@shindev> (raw)
In-Reply-To: <20230209094541.248729-1-joshi.k@samsung.com>

On Feb 09, 2023 / 15:15, Kanchan Joshi wrote:
> Ths creates a non-root user "blktest46", alters permissions for
> char-device node (/dev/ngX) and runs few passthrough commands.
> At the end of the test, user is deleted and permissions are reverted.
> 
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>

Thanks for the patch. I guess this test case exercises nvme_cmd_allowed() in
drivers/nvme/host/ioctl.c. The test contents look valid and good.

This test case adds and deletes a user. For every test case run, it creates and
removes the user home directory and touches /etc files. It does not sound right
for me. It changes system set up, and sudden test case stop will leave the user.

I suggest to ask blktests users to prepare the normal user and specify it to a
config file variable (it can be named NORMAL_USER or something). I also suggest
to add two new helper functions: _require_user() will check that the specified
user is valid, and _run_user() will wrap the "su $NORMAL_USER -c" command line.

If you don't mind, I can create another patch for further discussion based on
the suggestion above, and modify your patch to use the new helper functions.

-- 
Shin'ichiro Kawasaki

  parent reply	other threads:[~2023-02-10  2:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230209094631epcas5p436d4f54caa91ff6d258928bba76206de@epcas5p4.samsung.com>
2023-02-09  9:45 ` [PATCH blktests] nvme/046: add test for unprivileged passthrough Kanchan Joshi
2023-02-09 16:22   ` kernel test robot
2023-02-10  2:01   ` Shinichiro Kawasaki [this message]
2023-02-10 11:12     ` Kanchan Joshi
2023-02-14  4:57       ` Shinichiro Kawasaki
2023-02-14  5:54         ` Kanchan Joshi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230210020114.zzmazatkxeomowxq@shindev \
    --to=shinichiro.kawasaki@wdc.com \
    --cc=hch@lst.de \
    --cc=joshi.k@samsung.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox