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.

Sign up for GitHub

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

Jump to bottom

WIP: Add zfs vol driver #11

Closed
wants to merge 29 commits into from
Closed

WIP: Add zfs vol driver #11

wants to merge 29 commits into from

Conversation

MatiasVara
Copy link

@MatiasVara MatiasVara commented Nov 2, 2022 •
edited

This PR adds a ZFS volume driver. The current work is a work-in-progress. The goal of this PR is only for reviewing. Note that both the architecture and the implementation shall be reviewed. The architecture is described in the design document.

@MatiasVara
WIP: Add support for zfs volume driver
f3343ae 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add sr.destroy()
e0e9fd9 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add vdi.destroy()
133f294 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add vdi.stat()
79c9bcd 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add sr.ls()
0df9704 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add vol.snapshot()
dcbaa90 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
WIP: Add vol.clone()
b9090a7 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add clone and snapshot in plugin.py
72b0bc1 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
VDI_DESTROY fails if volume has children
9fc88fc 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add support for revert-from-snap.
c2cfc4e 
The clone method only accepts snapshot. Destroy does not fail
if the volume has children.

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add promote to remove parent when during revert-from-snapshot
d47c273 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Force creation when blk device is dirty
c0563ca 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Creates a clone during snapshot and removes it when the snapshot is d…
e498115 
…estroyed

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Fix typo in cmd
6abe871 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add parameters during vdi creation.
aefb743 
This commit adds the following parameters during vdi creation:
- mountpoint
- compression
- devices
- mode

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add promotion of clone during snapshot
42f9004 
This commit adds promoting when a snapshot is taken together with the clone
that is created. This allows to remove the main volume during
revert-from-snapshot. This also allows to revert from any snapshot.

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Do not use parent id during stat
727b208 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add support for destroy volumes and snapshots
2f07456 
This commit improves how the driver destroys volumes and snapshots. The
commit creates a tree structure in which snapshots are the parent of volumes
and other snapshots. The root of this tree is the oldest snapshot. The button
of the three is the latest snapshot. Snapshots can be destroy from top to
button (from oldest to lastest) or from button to top (from latest to oldest).
Those snapshots that have volumes cannot be destroyed until all volumes are
destroyed.

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Use COWUtil interface for volume operations
c4bf28c 
This commit creates zfsutil.py library that contains the class ZFSUtil that
instantiates the interface COWUtil. This interface contains common operations
over volumes. The commit rewrites volume.py and sr.py by using this library.
The commit also extends current COWUtil interface by adding three new
operations:
- promote
- destroy
- clone
These operations are supported by zfs volumes.

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Remove unnecessary constants
42fb603 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara MatiasVara changed the title Add zfs vol driver WIP: Add zfs vol driver Nov 2, 2022
Wescoeur
Wescoeur reviewed Nov 3, 2022
View reviewed changes
plugins/volume/org.xen.xapi.storage.zfs-ng/plugin.py Outdated Show resolved Hide resolved
xapi/storage/libs/libcow/zfsutil.py Show resolved Hide resolved
xapi/storage/libs/libcow/zfsutil.py Show resolved Hide resolved
xapi/storage/libs/libcow/zfsutil.py

ZFS_UTIL_BIN = 'zfs'

class ZFSUtil(COWUtil):
Copy link
Member

Choose a reason for hiding this comment

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

This method is not implemented:

    @staticmethod
    def getImgFormat(dbg):
        return 'raw'

I suppose we must return 'raw' like the RawUtil helper.

Copy link
Member

Choose a reason for hiding this comment

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

Same for is_empty, I know it's used in epc_open.

Copy link
Author

Choose a reason for hiding this comment

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

I implemented getImgFormat() and is_empty(). However, I am not sure what is the meaning of is_empty(). I implemented like:

    @staticmethod
    def is_empty(dbg, vol_path):
        raise NotImplementedError()

xapi/storage/libs/libcow/zfsutil.py Outdated Show resolved Hide resolved
plugins/volume/org.xen.xapi.storage.zfs-ng/volume.py Outdated Show resolved Hide resolved
plugins/volume/org.xen.xapi.storage.zfs-ng/volume.py Outdated Show resolved Hide resolved
plugins/volume/org.xen.xapi.storage.zfs-ng/volume.py Outdated Show resolved Hide resolved
plugins/volume/org.xen.xapi.storage.zfs-ng/volume.py Outdated Show resolved Hide resolved
plugins/volume/org.xen.xapi.storage.zfs-ng/volume.py Outdated Show resolved Hide resolved
@MatiasVara
Address comments in review
a940857 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Add support for destroying snapshot with a single volume child
3033b8a 
This commit deals with the case in which a snapshot with a single volume child
is destroyed. To do so, the volume is first promoted, then the clone is
destroyed and finally the snapshot can be destroyed.

Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Store mounpoint when creating a sr
270e00f 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@MatiasVara
Use name instead of mountpoint in Destroy
c8cd67a 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
MatiasVara and others added 2 commits February 1, 2023 14:49
@MatiasVara
Use first snapshot child during destroy
c246517 
Signed-off-by: Matias Ezequiel Vara Larsen <matias.vara@vates.fr>
@ydirson
Fix error cases
eb08a8a
@ydirson
util.call*: fix error reporting
e456863 
Trying to interpret the exit status of arbitrary called processes as an
errno value, and disregard the stderr content we extract for a reason, had
little chance to be useful.

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
(cherry picked from commit 2b6fb6c)
@ydirson
Use "sr-{uuid}" instead of label to name the zpool 1/2
12ce090 
And mount it under /var/run/sr-mount/ not / for consistency with other
SRs.

* characters valid in XAPI SR name is not the same as those valid for
  a ZFS pool name, so use UUID instead (would not be able to use under
  xcp-ng-tests otherwise)
* ZFS requires the pool name to start with a letter, so use "sr-" as
  prefix

Just enough for sr-create/destroy, but breaks all volume manipulation.
@ydirson
Use "sr-{uuid}" instead of label to name the zpool 2/2
b752c6c 
This part deals with the volume manipulation.  Since the volume plugin
does not use a "SR metadata" struct we have to introduce a separate
helper function here - and since we mostly care about it when
accessing a volume, simply handle creation of volume name in one
place.

This is unfortunately needed in other loosely-related code units, so
hardcoding goes on...
@ydirson ydirson mentioned this pull request Mar 14, 2024
Merged
@ydirson
Copy link
Contributor

ydirson commented Apr 4, 2024

Included-in/superceded-by #17, now merged!

@ydirson ydirson closed this Apr 4, 2024
@ydirson ydirson deleted the add-zfs-vol-driver branch April 4, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wescoeur Wescoeur Wescoeur left review comments

@benjamreis benjamreis benjamreis left review comments

Assignees

@MatiasVara MatiasVara

@Wescoeur Wescoeur

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MatiasVara @ydirson @Wescoeur @benjamreis