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

Add status and action strings, and func to retrieve them, including formatting option #42

Merged
merged 1 commit into from Apr 19, 2022

Conversation

deitch
Copy link
Contributor

@deitch deitch commented Apr 18, 2022

As discussed in #41

@deitch
Copy link
Contributor Author

deitch commented Apr 18, 2022

The only thing I didn't include is where it actually reports which features are unsupported, see here and here.

To do that, I would need to know what part of go-libzfs I would use to find the unsupported features for the equivalent of zpool_print_unsup_feat, which is defined here, imported for libzfs.h.

If I can get some pointers, happy to include it, or we can make it a future iteration.

Copy link
Contributor

@fkasumovic fkasumovic left a comment

Choose a reason for hiding this comment

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

zpool_print_unsup_feat is not used, and if you going to do it,
do it in the next iteration, and please don't print :-).

Because libzfs C library (wrapped in go here) is not designed as an actual general-purpose API for ZFS but rather as a part of user-space utils of the ZFS, it has these awkward printing functions for the library :-). There is libzfs_core that suppose to be a general-purpose library but does not support all the features and so it's not used here until that changes.

Implement the Pool type method to return a list of unsupported features, and then the developer using go-libzfs can choose to print it or whatever.

func (p *Pool) UnsupportedFeatures() (features map[string]string) {
...
}

common.go Outdated Show resolved Hide resolved
common.go Outdated Show resolved Hide resolved
@deitch deitch force-pushed the status-strings branch 2 times, most recently from 80ec123 to 200fbb2 Compare April 19, 2022 08:11
…ormatting

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@deitch
Copy link
Contributor Author

deitch commented Apr 19, 2022

zpool_print_unsup_feat is not used, and if you going to do it,
do it in the next iteration, and please don't print :-).

OK, I replace the "it uses the following feature(s) not supported on this system:" part for that message with "it uses feature(s) not supported on this system."

That makes it correct grammatically, while leaving it open in the future to add those details, like you suggested.

Ready for another review.

@deitch
Copy link
Contributor Author

deitch commented Apr 19, 2022

Implement the Pool type method to return a list of unsupported features, and then the developer using go-libzfs can choose to print it or whatever.

func (p *Pool) UnsupportedFeatures() (features map[string]string) {
...
}

I rather like that idea. But I do not know what goes in that function. The original source for it appears to be in this thankfully short function, but unsure what it is doing.

@fkasumovic
Copy link
Contributor

fkasumovic commented Apr 19, 2022

@deitch Well when I take a better look, I think unsupported features is available only for exported pool,

So maybe this should be added in to PoolImportSearch, and returned as a part of ExportedPool type/struct.

ep.Status = PoolStatus(reason)
ep.UnsupportedFeatures = unsupportedFeatures(config)

You can wrap with cgo, implementation for fetching the list would probably look like the following:

func unsupportedFeatures(config C.nvlist_ptr) (features map[string]string) {
	features = make(map[string]string)
	var nvinfo, unsup_feat C.nvlist_ptr

	if C.nvlist_exists(config, C.sZPOOL_CONFIG_LOAD_INFO) != 0 {
		nvinfo = C.fnvlist_lookup_nvlist(config, C.sZPOOL_CONFIG_LOAD_INFO)
		if C.nvlist_exists(nvinfo, C.sZPOOL_CONFIG_UNSUP_FEAT) != 0 {
			unsup_feat = C.fnvlist_lookup_nvlist(nvinfo, C.sZPOOL_CONFIG_UNSUP_FEAT)

			for nvp := C.nvlist_next_nvpair(unsup_feat, nil); nvp != nil; {
				desc := C.GoString(C.fnvpair_value_string(nvp))
				name := C.GoString(C.nvpair_name(nvp))
				features[name] = desc

				nvp = C.nvlist_next_nvpair(unsup_feat, nil)
			}
		}
	}
	return
}

And then do lot of testing :-)

@fkasumovic fkasumovic merged commit f9d12fe into bicomsystems:master Apr 19, 2022
@fkasumovic fkasumovic mentioned this pull request Apr 19, 2022
@deitch deitch deleted the status-strings branch April 19, 2022 14:15
@deitch
Copy link
Contributor Author

deitch commented Apr 19, 2022

I hadn't even thought of that, just of recreating the logic. What you did is much smoother. You already pretty much did it.

giggsoff pushed a commit to giggsoff/go-libzfs that referenced this pull request May 4, 2022
…ormatting (bicomsystems#42)

Signed-off-by: Avi Deitcher <avi@deitcher.net>

(cherry picked from commit f9d12fe)
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
giggsoff pushed a commit to giggsoff/go-libzfs that referenced this pull request May 4, 2022
Add status and action strings, and func to retrieve them, including formatting

Signed-off-by: Avi Deitcher <avi@deitcher.net>

(cherry picked from commit f9d12fe)
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
fkasumovic pushed a commit that referenced this pull request Jul 12, 2023
Add status and action strings, and func to retrieve them, including formatting



(cherry picked from commit f9d12fe)

Signed-off-by: Avi Deitcher <avi@deitcher.net>
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
Co-authored-by: Avi Deitcher <avi@deitcher.net>
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.

None yet

2 participants