All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitriy Bogdanov <d.bogdanov@yadro.com>
To: Mike Christie <michael.christie@oracle.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	"target-devel@vger.kernel.org" <target-devel@vger.kernel.org>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux@yadro.com" <linux@yadro.com>,
	Nilesh Javali <njavali@marvell.com>, Chris Boot <bootc@bootc.net>
Subject: RE: [PATCH 0/4] target: make tpg/enable attribute
Date: Thu, 18 Mar 2021 08:58:43 +0000	[thread overview]
Message-ID: <2b099689cee84bd4b27509425c8139b4@yadro.com> (raw)
In-Reply-To: <03cdc625-98de-7c41-d8cf-74143b2b7446@oracle.com>

Hi Mike,

Thank you for quick response.

I got my mistakes.
Will try to come up with a new solution.

BR,
 Dmitry

-----Original Message-----
From: Mike Christie <michael.christie@oracle.com> 
Sent: Wednesday, March 17, 2021 7:32 PM
To: Dmitriy Bogdanov <d.bogdanov@yadro.com>; Martin Petersen <martin.petersen@oracle.com>; target-devel@vger.kernel.org
Cc: linux-scsi@vger.kernel.org; linux@yadro.com; Nilesh Javali <njavali@marvell.com>; Chris Boot <bootc@bootc.net>
Subject: Re: [PATCH 0/4] target: make tpg/enable attribute

On 3/17/21 5:46 AM, Dmitry Bogdanov wrote:
> All target transport drivers have its own 'tpg/enable' attribute 
> implementation. This produces unnecessary code duplication
I don't think that is correct. Some drivers don't have en enable attr:

- vhost-scsi, xen and loop have a nexus attr that sort of leaves it in the equivalent of enabled.

- usb has a nexus file like above, but after you write to it you still have to write to the enable attr.

- tcm_fc does not have an enable and has it's own initialization strategy.

For drivers that have an enable file your patches missed usb, srpt and ibm_vscsi.

> Also it makes difficult to control that attribute and to depend on 
> that attribute inside of target core module.

I agree with this :) It's a bit of mess, but I think it's sometimes due to how the driver is implemented and how userspace has to set it up, so it's not as simple as in this patchset due to having to support existing tools.

      reply	other threads:[~2021-03-18  8:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 10:46 [PATCH 0/4] target: make tpg/enable attribute Dmitry Bogdanov
2021-03-17 10:46 ` [PATCH 1/4] target: core: add common " Dmitry Bogdanov
2021-03-17 10:46 ` [PATCH 2/4] target: iscsi: replace enable attr to ops.enable Dmitry Bogdanov
2021-03-17 10:46 ` [PATCH 3/4] target: qla2xx: " Dmitry Bogdanov
2021-03-17 10:46 ` [PATCH 4/4] target: sbp: " Dmitry Bogdanov
2021-03-17 16:32 ` [PATCH 0/4] target: make tpg/enable attribute Mike Christie
2021-03-18  8:58   ` Dmitriy Bogdanov [this message]

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=2b099689cee84bd4b27509425c8139b4@yadro.com \
    --to=d.bogdanov@yadro.com \
    --cc=bootc@bootc.net \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=martin.petersen@oracle.com \
    --cc=michael.christie@oracle.com \
    --cc=njavali@marvell.com \
    --cc=target-devel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.