Given the example of pool creation, what would you envision the arguments
to lza_pool_create() being? Would it be able to return a detailed
description of the error, and if so how? Take for example the errors
handled in check_replication(). Would you simply return the string that
vdev_error() prints to stderr today? That would be straightforward to
implement, but it doesn't improve on the string-parsing requirement for
consumers (at least for errors).
--matt
On Tue, Mar 10, 2020 at 2:31 PM mikefcarlin via openzfs-developer <
developer@lists.open-zfs.org> wrote:
> What are the expected benefits over fork/exec'ing the zpool/zfs commands?
>
>
> - Reduced overhead from calling library functions rather than spinning
> off subprocesses (although configuration in this case isn't a performance
> path by any means). This would be helpful for situations where you're
> constantly polling information/statistics from ZFS.
> - Allows the user of the library to keep their implementation native
> (data structures, control flow, etc.) and avoid the need to parse CLI output
> - Simplifies implementing native bindings in other languages.
>
> There are a lot of things happening in zfs_main.c and zpool_main.c. How
> much of that are you suggesting move to the new library? E.g. we could be
> talking about an API like `int run_zfs_command(char *argv[])` that
> essentially invokes main(). But having that library print to stdout, etc
> might not be what you're looking for.
>
>
> I'm suggesting that this new API layer contains all the same actions
> (probably some exceptions to this) that are present in zpool & zfs (read
> do_zpool_*() and do_zfs_*()). This could looks like the following:
>
> - lza_pool_create()
> - lza_pool_destroy()
> - etc.
>
> Any business logic that was present in those actions is pushed down into
> the libzfs_api library equivalent, leaving the CLI related functionality
> separated from the business logic that ZFS needs to execute a given action.
> If there isn't any real business logic that needs to be pushed down, the
> libzfs_api call would still acts as a proxy to the libzfs call that was
> previously made. This would allow libzfs_api to be an all encompassing API,
> preventing the client from needing to mix and match calls between the
> different ZFS libraries.
>
> How would you want errors to be expressed in the new API?
>
>
> I'd like to keep the API away from any sort of stdout/stderr output and
> leave that to the CLI layer. The API would return error codes (and possibly
> error text if that made sense) but wouldn't directly output anything and
> instead leave that up to the function caller (i.e. CLI would output to
> stdout/stderr, to Josh's example a REST API layer could respond to a
> request using the error code + text from the function call).
>
> Maybe you could give some examples of subcommands that do not have good
> equivalents in libzfs. Here are a few that I looked at:
>
> zfs_do_set(): it seems to me that zfs_prop_set_list() is a good libzfs
> interface to use in its place. But maybe you have something different in
> mind?
>
> zfs_do_send(): its CLI parsing is much more complicated but it looks like
> it boils down to one call to zfs_send_*(). Are you thinking that we should
> make a single API entry point that would cover all the zfs_send_*()
> variants?
>
> zfs_do_list(): the logic in print_dataset() is a little involved, but
> unless you want the library to be generating the same string output that we
> have from the CLI, it seems like the existing libzfs functions for
> examining the zfs_handle_t seem decent. Again, maybe an example of what
> you would like to see for a "true" API that's equivalent to zfs_do_list()
> would be helpful.
>
>
> The few functions you mentioned (set, send and list) are pretty barebones
> with regards to the business logic I'm interested in moving to the API
> layer. A couple examples of the functions that led me down this path were
> do_zpool_create() and do_zpool_add() with their usage of functions from
> zpool_vdev.c.
>
> do_zpool_create():
>
> ...
>
> /* pass off to make_root_vdev for bulk processing */
> nvroot = make_root_vdev(NULL, props, force, !force, B_FALSE, dryrun,
> argc - 1, argv + 1);
> if (nvroot == NULL)
> goto errout;
>
> /* make_root_vdev() allows 0 toplevel children if there are spares */
> if (!zfs_allocatable_devs(nvroot)) {
> (void) fprintf(stderr, gettext("invalid vdev "
> "specification: at least one toplevel vdev must be "
> "specified\n"));
> goto errout;
> }
>
> ...
>
>
> This creation of the vdev spec and related functionality in zpool_vdev.c
> is decently involved and would likely be need to be duplicated by any
> client looking mimic zpool create or add.
>
> *openzfs <https://openzfs.topicbox.com/latest>* / openzfs-developer / see
> discussions <https://openzfs.topicbox.com/groups/developer> + participants
> <https://openzfs.topicbox.com/groups/developer/members> + delivery options
> <https://openzfs.topicbox.com/groups/developer/subscription> Permalink
> <https://openzfs.topicbox.com/groups/developer/Tdde1f0006baa1227-M6a3e8c4c47f5fd468da4b846>
>
------------------------------------------
openzfs: openzfs-developer
Permalink:
https://openzfs.topicbox.com/groups/developer/Tdde1f0006baa1227-M1992df9620d8aaf81b3037fd
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription
