-
Notifications
You must be signed in to change notification settings - Fork 148
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
libtcmu: allow multiple daemons co-exist #519
base: main
Are you sure you want to change the base?
Conversation
libtcmu.c
Outdated
@@ -432,8 +434,8 @@ static int add_device(struct tcmulib_context *ctx, char *dev_name, | |||
|
|||
dev->handler = find_handler(ctx, dev->cfgstring); | |||
if (!dev->handler) { | |||
tcmu_err("could not find handler for %s\n", dev->dev_name); | |||
goto err_free; | |||
tcmu_dbg("could not find handler for %s\n", dev->dev_name); |
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.
should this be a warning tcmu_warn () ?
For tcmu-runner case, it is helpful to have this msg at logs with non-debug log levels too.
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.
Yeah, that makes sense to me, thanks.
libtcmu.c
Outdated
err_nohandler: | ||
free(dev); | ||
|
||
return 1; |
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.
In general, It makes more sense to return -1/1 for all those str***() function errors, and return ENOENT for nohandler case.
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.
OK, will fix it, thanks.
As we support libtcmu again users could want to run several daemons at the same time. So don't fail the device addition via netlink reply when can't find a handler in ctx to handle the device. This just means the device addition event is not for this ctx so give other daemons a chance to try. Signed-off-by: Yaowei Bai <[email protected]>
5853939
to
f38a12c
Compare
Updated, please review. Thanks. @pkalever @mikechristie |
Hey Mike, what do you think about this? @mikechristie |
I am on PTO from Jan 28 - Feb 8. I am going to try and get to this next week while on vacation. |
This looks fine to me. |
Thanks @bgly .Mike, what do you think about this? @mikechristie |
targetcli hangs if the added device subtype is not found by any daemon. Restarting tcmu-runner solves the problem because it resets the netlink interface, but this is not a good solution. Also consider that a custom daemon might crash or be killed during the handling of TCMU_CMD_ADDED_DEVICE, which would also cause targetcli to hang. |
As we support libtcmu again users could want to run several
daemons at the same time. So don't fail the device addition
via netlink reply when can't find a handler in ctx to handle
the device. This just means the device addition event is not
for THIS ctx so give other daemons a chance to try.
This fix is for issue #516 .
Signed-off-by: Yaowei Bai [email protected]