Consider splitting Dataset into Snapshot, Clone, Volume types with embedded dataset type about go-zfs

[bakins]

I thought about this, but TBH, when we started this, I was fairly new to Go. Let me play with it, but seems reasonable. ZFS is somewhat interesting in how volumes/filesystems/etc are treated WRT the same commands.

[mdlayher]

Yeah, certainly! I'm still fairly new to ZFS, and from a systems administrator standpoint, I appreciate how much work it does behind the scenes to blur the lines between different dataset types.

Conversely, from a programmer standpoint, I like the fact that one would never have to wonder if it's possible to send/receive/rollback a clone, since it's clear that those methods are only available and usable with a snapshot.

That is my two-cents on the matter!

[bakins]

Should dataset be an interface? I just wonder for times when you get a list back and just need to iterate, etc.

[mdlayher]

Good question. I suppose it could be, but that would also mean manually implementing the Name(), Used(), Avail() methods on all types, instead of relying on method promotion from the struct (since interfaces can't have struct members, Go doesn't have inheritance, etc.).

If we are trying to distinguish between dataset types, I'm not sure how helpful an interface would be in this case. I would probably still opt toward an unexported type dataset struct for embedding purposes, but perhaps some experimentation will reveal which method is easier.

[mdlayher]

As far as iterating goes, the following method could perhaps be: https://github.com/mistifyio/go-zfs/blob/master/zfs.go#L38-L41

// Snapshots returns a slice of all snapshots
func Snapshots(filter string) ([]*Snapshot, error) {
    datasets, err := listByType("snapshot", filter)
    if err != nil {
        return nil, err
    }

    snapshots := make([]*Snapshot, len(datasets))
    for i := range datasets {
        snapshots[i] = &Snapshot{
            dataset: datasets[i],
        }
    }

    return snapshots, nil
}

It's not perfect, but it would do the job.

[mdlayher]

Also, I didn't realize that this library exposed the ability to retrieve any kind of dataset. Perhaps the dataset type should be Dataset instead, so that Snapshot, Volume, and Clone still embed Dataset, but the type is still useful on its own, with its own basic methods.

[mdlayher]

Sorry for all the comments, thoughts keep coming to me. :) I'd be happy to mock up a PR in the next couple of days if you'd like.

[mdlayher]

I did a bit of quick and dirty hacking on my end, but I'm honestly not sure the result is much better. The Clone, Filesystem, and Volume types have almost no merit on their own (and no unique methods). The only type that may be worthwhile would be Snapshot, but if one is judicious in using the existing API appropriately, perhaps it's not worth the trouble.

For the time being, I'm going to close this issue. Here are the files I modified, just for reference:

https://gist.github.com/mdlayher/9cb32b96a9b6259bdf30