> Given the example of pool creation, what would you envision the arguments to
> lza_pool_create() being?
I imagine pool creation to look something like the following:
1
2
3
4
5
6
7
8
9
10
11
12
13
typedef struct pool_spec {
const char *name;
nvlist_t *nvroot;
nvlist_t *props;
nvlist_t *fsprops;
} pool_spec_t;
typedef struct lza_error {
zfs_error_t err; // It'd be nice if we can reuse this enum but might need
to have a libzfs_api specific one
char *description;
} lza_error_t;
lza_error_t lza_pool_create(pool_spec_t *spec);
The kind of flow I'm imagining is as follows:
┌─┐
║"│
└┬┘
┌┼┐
│ ┌──────────┐
┌──────┐
┌┴┐ │libzfs_api│
│libzfs│
Client Application └──────────┘
└──────┘
│────┐ │
│ │ Translate client model │
│<───┘ to pool spec │
│ │
│ │ │
│ create pool │ │
│ ──────────────────────────> │
│ │ │
│ │────┐
│ │ │ validate pool specificiation
│ │<───┘
│ │ │
│ │ create pool │
│ │ ────────────────────────────────>
│ │ │
│ │ │
│ │ <────────────────────────────────
│ │ │
│ │ │
│ <────────────────────────── │
Client Application ┌──────────┐
┌──────┐
┌─┐ │libzfs_api│
│libzfs│
║"│ └──────────┘
└──────┘
└┬┘
┌┼┐
│
┌┴┐
For the above diagram, any client to ZFS that creates zpools will likely have
it's own way to represent a vdev hierarchy. Zpool currently utilizes text input
to organize what a given pool looks like (i.e. 'pool1 mirror dev1 dev2'). If it
was RESTful API, you could imagine there would be some sort of JSON document
along these lines:
1
2
3
4
5
6
7
8
9
10
{
"name": "k",
"vdevs": {
"type": "mirror",
"devices": ["dev1", "dev2"]
},
"props": {
"ashift": 12
}
}
From a given client representation, the client would own translating this to a
common format, the pool_specification, that the libzfs_api would define and
understand. This definition could mirror what libzfs currently exposes in the
arguments of zpool_create (i.e. zpool_create(libzfs_handle_t *hdl, const char
*pool, nvlist_t *nvroot, nvlist_t *props, nvlist_t *fsprops). If it made sense,
there might be convenience functions in libzfs_api that provide a simpler, more
obvious way to construct the pool specification.
> 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).
As far as errors go, I think the following struct would be flexible enough to
allow for the current functionality while allowing the client to have an easier
time programmatically handling the error using an enum value.
1
2
3
4
typedef struct lza_error {
zfs_error_t err; // It'd be nice if we can reuse this enum but might need
to have a libzfs_api specific one to encompass additional error cases
char *description;
} lza_error_t;
With regards to check_replication(), it would utilize vdev_error() to produce
equivalent error messages that exist today and put them into the description of
the lza_error struct while also supplying an enum value to indicate the error
type.
--Mike
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, March 12, 2020 9:12 PM, Matthew Ahrens <mahr...@delphix.com> wrote:
> 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-M1992df9620d8aaf81b3037fd)
zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
nvlist_t *props, nvlist_t *fsprops)
------------------------------------------
openzfs: openzfs-developer
Permalink:
https://openzfs.topicbox.com/groups/developer/Tdde1f0006baa1227-Md2e7eb02031cb4f9b49613bd
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription
