feat(readonly): Adding test cases for uzfs zvol readonly support#288
feat(readonly): Adding test cases for uzfs zvol readonly support#288vishnuitta merged 5 commits intomayadata-io:developfrom
Conversation
Signed-off-by: mayank <mayank.patel@mayadata.io>
|
depends on openebs-archive/libcstor#41 |
module/zfs/zfs_ioctl.c
Outdated
| error = zfs_set_targetip_prehook(zc->zc_name, source, targetip, | ||
| &curtargetip[0]); | ||
| } else { | ||
| error = uzfs_zfs_set_prop(zc->zc_name, source, nvl); |
There was a problem hiding this comment.
any errors in zfs_set_prop_nvlist needs reverting the changes done here.. instead, this can be called if no errors after performing zfs_set_prop_nvlist..
There was a problem hiding this comment.
lets make the fn name as 'uzfs_zinfo_set_prop' as this performs setting few flags on zinfo as well in case of readonly flag..
There was a problem hiding this comment.
calling function that can set all properties will make those variables get set in zinfo / zv.. for ex., zvol_workers will be set in zv, but, it won't get impacted until zrepl restarts..
So, instead of calling function saying uzfs_zinfo_set_prop, how about setting only for uzfs_zinfo_set_readonly if prop is READONLY
Signed-off-by: mayank <mayank.patel@mayadata.io>
| PROP_DEFAULT, ZFS_TYPE_VOLUME, "<zvol worker>", "NWORKER"); | ||
| zprop_register_string(ZFS_PROP_REPLICA_ID, "io.openebs:zvol_replica_id", | ||
| "", PROP_DEFAULT, ZFS_TYPE_VOLUME, "<replica id>", "REPLICA_ID"); | ||
| zprop_register_string(ZFS_PROP_ZVOL_READONLY, "io.openebs:readonly", "", |
There was a problem hiding this comment.
@vishnuitta @mynktl , io.openebs:readonly does not convey it's meaning, also it conflicts with the existing readony property. Can we rename it to io.openebs:suspend (or some better name)?
There was a problem hiding this comment.
both property have the same functionality but at different layer. To differentiate this i thought of giving a different name. One more point, this is completely limited to cstor API layer only, so better to group it with organization.
module/zfs/zfs_ioctl.c
Outdated
| if (targetip != NULL) { | ||
| error = zfs_set_targetip_prehook(zc->zc_name, source, targetip, | ||
| &curtargetip[0]); | ||
| } else if (error == 0 && nvlist_lookup_string(nvl, |
There was a problem hiding this comment.
lets allow setting both (targetip and readonly) at once if we want to?
There was a problem hiding this comment.
we can do that, but that will complicate creation part i think from control plane, need to check. We can plan this in next release.
There was a problem hiding this comment.
what is the problem if we remove 'else' which allows both the settings?
pawanpraka1
left a comment
pawanpraka1
left a comment
There was a problem hiding this comment.
looks good.
few minor comments.
Signed-off-by: mayank <mayank.patel@mayadata.io>
4 participants
changes:
To set zvol readonly:
To unset zvol readonly:
Test cases added for following scenarios:
Fixes: openebs/openebs#2937
Signed-off-by: mayank mayank.patel@mayadata.io