zcp: get_prop: fix encryptionroot and encryption #17280
Conversation
It was reported that channel programs' zfs.get_prop doesn't work for dataset properties encryption and encryptionroot. They are handled in get_special_prop due to the need to call dsl_dataset_crypt_stats to load those dsl props. Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
| @@ -404,6 +407,25 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname, | |||
| break; | |||
| } | |||
|
|
|||
| case ZFS_PROP_ENCRYPTION_ROOT: { | |||
| setpoint[0] = '\0'; | |||
| numval = 0; | |||
There was a problem hiding this comment.
I wonder why do you initialize numval here, not strval?
And also wonder why you are not setting setpoint below, similar to above?
There was a problem hiding this comment.
numval is a leftover, can delete that;
filling setpoint with the path didn't work (the result I was getting was empty AFAIK) - I wasn't clear on what setpoint is for, intuitively I would say that it's relevant when the value is numerical or if there's the need to pass another string along; looking at dsl_get_mountpoint, its impl refers to the third parameter as source
originally I tried to lump ZFS_PROP_ENCRYPTION_ROOT with the rest of the pack above but those are all numeric values
if you check the other properties in this function above, some set only strval and only a few set both setpoint and strval
There was a problem hiding this comment.
@amotin while we're at this, what do you think of this:
- if you do
zfs get all, thenencryptionrootisn't listed there - but
zfs get encryptionrootworks - I know some properties might be intentionally hidden, those are AFAIK registered with the
zprop_register_hiddenvariant - butZFS_PROP_ENCRYPTION_ROOTis registered withzprop_register_string, so that should be visible inget all, shouldn't it?
should I file an issue for that or am I missing something and it should in fact be hidden?
There was a problem hiding this comment.
I don't know why it is named so, but in the code above setpoint seems to be set to the property source, which is "default", "local", "received", etc. And unless it somehow makes no sense for ZFS_PROP_ENCRYPTION_ROOT, it would be nice to report it too.
I am not aware why ZFS_PROP_ENCRYPTION_ROOT might be hidden. I can think of two reasons to hide properties: either they are too technical to expose to user, but still somehow useful for some tools, or it is too expensive to fetch. I don't think either applies to ZFS_PROP_ENCRYPTION_ROOT, but I might be wrong on the first side, since I am not too deep in encryption administration.
There was a problem hiding this comment.
ok will take another look at setpoint;
btw @ zfs get all, apparently a property that doesn't have a value at all isn't listed in get all, although it is accessible via direct get; if I do get all with a dataset that does have encryptionroot, it works
Motivation and Context
#12337
Description
It was reported that channel programs' zfs.get_prop doesn't work for
dataset properties encryption and encryptionroot.
They are handled in get_special_prop due to the need to call
dsl_dataset_crypt_stats to load those dsl props.
@grahamc already worked on a testcase for this (#12335), we could revive and merge that PR now
How Has This Been Tested?
Reproducer in #12337
Types of changes
Checklist:
Signed-off-by.