target-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Christie <mchristi@redhat.com>
To: target-devel@vger.kernel.org
Subject: Re: [PATCH] target: Add netlink command reply supported option for each device
Date: Wed, 13 Sep 2017 18:43:47 +0000	[thread overview]
Message-ID: <59B97C63.8090101@redhat.com> (raw)
In-Reply-To: <20170913050122.13731-1-nakayamakenjiro@gmail.com>

On 09/13/2017 01:28 PM, Nakayama Kenjiro wrote:
>> I am ok with the idea for the patch, but what type of setup do you have
>> where there are multiple applications using the interface and some
>> support it and some do not? Is it because tcmu-runner is starting by
>> default due to something like a distro systemd unit file and then you
>> also have your app running too?
> 
> Though I am not going to run multiple applications using the
> support/non-support interface atm, I have to note to use the non-support
> app as  "Please do not run tcmu-runner (or any app that enables
> netlink reply support) before/during running this app. If you ran it, you
> have to restart the host and never run it again". It sounds like a
> little bit silly note...

Yes. Agree.

> 
>> Also, if you do not implement sync netlink support, how does your daemon
>> tell the kernel if the device failed to setup in the daemon? For
>> deletion how did you work around the uio crashes and leaks?
> 
> Yes, I admit that the application should implement the netlink sync
> support,

Actually maybe I would not add support for it at all if I were you.

Some other questions/comments, because how the daemons like tcmu-runner
do setup might be a result of using targetcli/rtslib, and if you are not
going to use that then you can simplfy things.

The current normal way to create a device is:

1. Start app which listens on nl.
2. targetcli/rtslib run configfs commands to add and enable tcmu device.
3. tcmu sends nl event to app which sets up its objects for the device
and returns success/failure.

This approach works well for the targetcli/rtslib device management
flow, but has those extra steps.

If you are not going to use targetcli/rtslib then you do not need
netlink and to go through those extra steps.

It sounds like you want to do:

1. app runs configfs commands to add and enable tcmu device.
2. app knows tcmu device name and other info so it can just setup its
objects on successful writing to the enable file.
3. It sounds like you do not need the nl events at all at this point right.

  parent reply	other threads:[~2017-09-13 18:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-13  5:01 [PATCH] target: Add netlink command reply supported option for each device Kenjiro Nakayama
2017-09-13 15:10 ` Bryant G. Ly
2017-09-13 15:58 ` Mike Christie
2017-09-13 16:04 ` Mike Christie
2017-09-13 18:35 ` Nakayama Kenjiro
2017-09-13 18:43 ` Mike Christie [this message]
2017-09-13 19:27 ` Nakayama Kenjiro
2017-09-13 19:31 ` Mike Christie
2017-09-16  1:01 ` Mike Christie

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=59B97C63.8090101@redhat.com \
    --to=mchristi@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).