-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New feature - option to use libtcmu without netlink #694
base: main
Are you sure you want to change the base?
Conversation
This PR addresses some of the goals outlined in #519, |
Could you add the detail comments and signed-off-by in the commit ? |
tcmu-synthesizer.c
Outdated
@@ -197,7 +198,7 @@ int main(int argc, char **argv) | |||
exit(1); | |||
} | |||
|
|||
ctx = tcmulib_initialize(&syn_handler, 1); | |||
ctx = tcmulib_initialize(&syn_handler, 1, use_netlink); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
|
||
static bool create_backstore() | ||
{ | ||
int ret = system("./create_backstore.sh"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect approach.
Why not comply to the current consumer.c
, which will create the devices via targetcli or ceph-iscsi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For one, I don't believe targetcli
has the ability to specify nl_reply_supported=-1
when creating the backstore. This is vital if you want to use TCMU without netlink. Secondly, I think there's merit in having a completely stand-alone example which doesn't require the user to have external tools installed.
Some folks might be using libtcmu in their own applications which don't hook into TCMU runner, much like the consumer.c example. In those cases, it can be useful to avoid using netlink altogether. As I'm sure you are aware, it's almost impossible to have more than one user of TCMU when netlink is involved. This is because the kernel broadcasts device events to all TCMU listeners over netlink and simply accepts the first response it receives. This can lead to the following scenario: 1. Application A creates a TCMU backstore with handler=app_a 2. The kernel (target_core_user) broadcasts the netlink message to all instances of libtcmu 3. Application B receives the message first. Application B doesn't have a handler for app_a, so it replies with an error code to the kernel. 4. The kernel receives the error and aborts backstore creation. 5. Application A wonders why its backstore creation failed. Users can avoid using netlink altogether by setting nl_reply_supported=-1 at backing store creation time. This is great as multiple applications using TCMU can now coexist on the same machine. The problem is we don't have a nice way to utilise libtcmu in this model. We need a way to notify libtcmu that a device has been created, reconfigured, removed so the relevant handlers can be called. Currently, this is done via netlink. This PR allows users to utilise all the great features of libtcmu, but without using netlink. The PR makes the following changes: * modify tcmulib_initialize to take a boolean parameter which informs libtcmu whether or not it should set up netlink. If true we do. If false we don't. * expose new methods tcmulib_notify_device_added, tcmulib_notify_device_removed and tcmulib_notify_device_reconfiged, which application code should call to inform libtcmu that a device has been added, removed, or reconfigured. These API's take the dev_name and pass through to the existing device_add, device_remove and device_reconfig functions in libtcmu.c. tcmulib_notify_device_reconfiged will need additional parameters to specify the new config. * updates the documentation to reflect these changes and notes that tcmulib_master_fd_ready and tcmulib_get_master_fd should not be called if you chose to call tcmulib_initialize in "don't use netlink mode". * adds an example application consumer_no_netlink which shows how to create a standalone application using libtcmu without netlink. Issue open-iscsi#678. PR open-iscsi#694 Signed-off-by: Xiubo Li <[email protected]>
8ab145a
to
e53de87
Compare
Done. |
Some folks might be using libtcmu in their own applications which don't hook into TCMU runner, much like the consumer.c example. In those cases, it can be useful to avoid using netlink altogether. As I'm sure you are aware, it's almost impossible to have more than one user of TCMU when netlink is involved. This is because the kernel broadcasts device events to all TCMU listeners over netlink and simply accepts the first response it receives. This can lead to the following scenario: 1. Application A creates a TCMU backstore with handler=app_a 2. The kernel (target_core_user) broadcasts the netlink message to all instances of libtcmu 3. Application B receives the message first. Application B doesn't have a handler for app_a, so it replies with an error code to the kernel. 4. The kernel receives the error and aborts backstore creation. 5. Application A wonders why its backstore creation failed. Users can avoid using netlink altogether by setting nl_reply_supported=-1 at backing store creation time. This is great as multiple applications using TCMU can now coexist on the same machine. The problem is we don't have a nice way to utilise libtcmu in this model. We need a way to notify libtcmu that a device has been created, reconfigured, removed so the relevant handlers can be called. Currently, this is done via netlink. This PR allows users to utilise all the great features of libtcmu, but without using netlink. The PR makes the following changes: * modify tcmulib_initialize to take a boolean parameter which informs libtcmu whether or not it should set up netlink. If true we do. If false we don't. * expose new methods tcmulib_notify_device_added, tcmulib_notify_device_removed and tcmulib_notify_device_reconfiged, which application code should call to inform libtcmu that a device has been added, removed, or reconfigured. These API's take the dev_name and pass through to the existing device_add, device_remove and device_reconfig functions in libtcmu.c. tcmulib_notify_device_reconfiged will need additional parameters to specify the new config. * updates the documentation to reflect these changes and notes that tcmulib_master_fd_ready and tcmulib_get_master_fd should not be called if you chose to call tcmulib_initialize in "don't use netlink mode". * adds an example application consumer_no_netlink which shows how to create a standalone application using libtcmu without netlink. Issue open-iscsi#678. PR open-iscsi#694 Signed-off-by: Alex Reid <[email protected]>
e53de87
to
4099806
Compare
Fixed commit comments |
+1 For my use-case i need for multiple libtcmu instances in different containers to coexist. @geordieintheshellcode thank you for your contribution. I will port your PR to my fork for now https://github.com/hocus-dev/photon-libtcmu/tree/hocus |
Some folks might be using libtcmu in their own applications which don't hook into TCMU runner, much like the
consumer.c
example. In those cases, it can be useful to avoid using netlink altogether. As I'm sure you are aware, it's almost impossible to have more than one user of TCMU when netlink is involved. This is because the kernel broadcasts device events to all TCMU listeners over netlink and simply accepts the first response it receives. This can lead to the following scenario:Users can avoid using netlink altogether by setting
nl_reply_supported=-1
at backing store creation time. This is great as multiple applications using TCMU can now coexist on the same machine. The problem is we don't have a nice way to utilise libtcmu in this model. We need a way to notify libtcmu that a device has been created, reconfigured, removed so the relevant handlers can be called. Currently, this is done via netlink.This PR allows users to utilise all the great features of libtcmu, but without using netlink. The PR makes the following changes:
tcmulib_initialize
to take a boolean parameter which informs libtcmu whether or not it should set up netlink. Iftrue
we do. Iffalse
we don't.tcmulib_notify_device_added
,tcmulib_notify_device_removed
andtcmulib_notify_device_reconfiged
, which application code should call to inform libtcmu that a device has been added, removed, or reconfigured. These API's take thedev_name
and pass through to the existingdevice_add
,device_remove
anddevice_reconfig
functions in libtcmu.c.tcmulib_notify_device_reconfiged
will need additional parameters to specify the new config.tcmulib_master_fd_ready
andtcmulib_get_master_fd
should not be called if you chose to calltcmulib_initialize
in "don't use netlink mode".consumer_no_netlink
which shows how to create a standalone application using libtcmu without netlink.Initial discussion of this feature: #678