On 12/03, Sergey Senozhatsky wrote:
Hello,
On Thu, Dec 2, 2021 at 1:57 AM Enzo Matsumiya <ematsumiya@xxxxxxx> wrote:
> ...
> The intention is to make it more like other modern tools (e.g. git,
> nvme, virsh, etc) which have more clear user interface, readable
> commands, and also makes it easier to script.
>
> Example commands:
> # ksmbdctl share add myshare -o "guest ok=yes, writable=yes, path=/mnt/data"
> # ksmbdctl user add myuser
> # ksmbdctl user add -i $HOME/mysmb.conf anotheruser
> # ksmbdctl daemon start
>
> Besides adding a new "share list" command, any previously working
> functionality shouldn't be affected.
- clearer user interface: having a single binary looks much clearer than
having several separate programs when, e.g. the user is looking which
program does what.
- more readable commands: continuing from topic above, the situation is
improved especially when you see that, e.g., the ksmbd.addshare program
also updates and deletes shares. With this unification, it is way more
intuitive to use:
I've always preferred the UNIX way: one app does one thing and one thing only.
This is why we have cp and mv, mkdir and touch, etc. we don't have a
single vfs-ctl
app that takes several hundred arguments and whose man page is basically a
small book (20+ pages long). This way we:
- keep manpages short and clear
- avoid params conflicts and ambiguity
- keep eggs in different baskets
Sure, I agree. Again, this could also be worked out in my proposal (I
haven't touched much besides README yet though).
For the missing stuff, e.g. manpages, we can also implement them the git
way, like "man 1 ksmbdctl" vs "man 1 ksmbdctl-share", where the former
would mention the latter, but only briefly and then indicate that a
sub-manpage exists for that command.
I ask you to consider the applications I used as inspiration for such change, such as git
... snip ...
Built-in commands are, basically, independent binaris that have a
common ancestor with the
only exception that git does not fork/exec them (not all of them).
They even have entry points
that resemble main() - they take "int argc, const char **argv" - and
git passes its argc and argv
down to built-ins.
Schematically
git: main(int argc, char **argv) {
builtin = lookup_builtin_command();
builtin->run(argc, argv);
}
Is this what you have in mind?
Yes, I've implemented it exactly that way:
share/share_admin.h:
void share_usage(ksmbd_share_cmd cmd);
int share_cmd(int argc, char *argv[]);
user/user_admin.h:
void user_usage(ksmbd_user_cmd cmd);
int user_cmd(int argc, char *argv[]);
daemon/daemon.h:
void daemon_usage(ksmbd_daemon_cmd cmd);
int daemon_cmd(int argc, char *argv[]);
The *_usage() functions were something I was preparing to accomodate the
new command abstraction I mentioned earlier, but I still haven't got to
finish. I wanted to get this unification approved first.
# ksmbdctl daemon start
Is this going to fork ksmb daemon? Otherwise this looks confusing. I'd
say that ksmbd daemon
needs to have a different name that will clearly show that it's a ksmb
daemon, not the "control"
tool that adds shares and deletes users.
At daemon_process_start(), I did:
...
if(prctl(PR_SET_NAME, "ksmbd-daemon\0", 0, 0, 0);
pr_info("Can't set program name: %s\n", strerr(errno));
...
which TBH I'm not sure is good enough. Alternatives/opinions are
welcome.
Cheers,
Enzo
