Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(list_snap): LIST_SNAP uzfs ioctl implementation #49

Merged
merged 9 commits into from
Mar 30, 2020

Conversation

vishnuitta
Copy link
Contributor

@vishnuitta vishnuitta commented Mar 20, 2020

This PR adds the LIST_SNAP uzfs ioctl implementation.
For command zfs listsnap <dataset>, output will be in json format as:

vitta@vitta-laptop:~/openebs/lzfs$ ./cmd/zfs/zfs listsnap pool1/ds0 | jq
{
  "name": "pool1/ds0",
  "snaplist": {
    "istgt1": null,
    "snap4": null,
    "istgt2": null,
    "snap1": null,
    "snap2": null,
    "istgt3": null,
    "snap3": null
  }
}

This will NOT list the snapshots created through 'zfs snapshot' until the zrepl is restarted. However, it lists the snapshots created through istgt iscsi target.

This command implementation reads the snapshot list from disk once and caches the list. This cache will be updated with snapshot name for every SNAP_CREATE command from istgt. This cache will be served for every trigger of this ioctl.

Working on testcases.

@vishnuitta
Copy link
Contributor Author

vishnuitta commented Mar 20, 2020

Related PR to this in cstor: mayadata-io/cstor#295

Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the PR description with updated sample JSON output?

@mittachaitu
Copy link

Can you update the PR description with updated sample JSON output?

Ok it was updated

Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked more at usability angle and changes are good

src/mgmt_conn.c Outdated Show resolved Hide resolved
Copy link
Contributor

@mynktl mynktl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

src/mgmt_conn.c Show resolved Hide resolved
src/mgmt_conn.c Outdated Show resolved Hide resolved
src/mgmt_conn.c Outdated Show resolved Hide resolved
src/mgmt_conn.c Outdated Show resolved Hide resolved
src/mgmt_conn.c Outdated Show resolved Hide resolved
Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are good

Copy link
Contributor

@pawanpraka1 pawanpraka1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

@pawanpraka1 pawanpraka1 merged commit 85dcd42 into openebs-archive:master Mar 30, 2020
@pawanpraka1
Copy link
Contributor

merged this, travis the failing as there is dependency on mayadata-io/cstor#295.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants