dak180 / FreeNAS-Report

SMART & ZPool Status Report for FreeNAS/TrueNAS
GNU General Public License v3.0
38 stars 8 forks source link

Question about possible feature #13

Open mth309 opened 2 years ago

mth309 commented 2 years ago

I'm not sure if opening an issue is the right way to send you questions like this, forgive my GitHub ignorance and please let me know if there's a better way to reach out.

I was wondering if you'd be interested in adding a feature to the summary tables for drives that are in a mirror vdev, where it would print the name of the device they are mirrored to? If the device is not part of a mirror (e.g. in a RAIDZ vdev) it prints 'N/A' instead. I already wrote the proof of concept code for SAS drives, so I just need to copy and paste it into the other summary table sections and it would be no work to you.

I was thinking of adding an option in the config file where the user would select whether they want this information printed or not, but then I didn't know if you'd want me changing your config file format, or what affect that has on people already using the script with an "old" config file, etc.

I attached an example of what the output looks like, and for the record in the screenshot /dev/da8 is (was) paired to /dev/da10, however da10 was faulted and removed from the system, so it currently just prints '/dev/'. I plan on fixing that.

Mirror Example

mth309 commented 2 years ago

I finished up the coding and testing and sent you a pull request if you're interested in this feature. Reference commit 359011e376f66501b3cfef20e25a81d3c5179862 for a detailed description of the behavior. Also, I don't have any NVME drives to test if you'd like to give that a shot. Mirrored NVME is likely a common boot configuration, so hopefully it works the same as the other drive types.

dak180 commented 2 years ago

I was wondering if you'd be interested in adding a feature to the summary tables for drives that are in a mirror vdev, where it would print the name of the device they are mirrored to? If the device is not part of a mirror (e.g. in a RAIDZ vdev) it prints 'N/A' instead. I already wrote the proof of concept code for SAS drives, so I just need to copy and paste it into the other summary table sections and it would be no work to you.

Seems like it could be useful, but is it more useful than the glabel and zpool status sections farther down?

I was thinking of adding an option in the config file where the user would select whether they want this information printed or not, but then I didn't know if you'd want me changing your config file format, or what affect that has on people already using the script with an "old" config file, etc.

The right way to do it is to make sure that nothing changes if it is not set in the config file.

mth309 commented 2 years ago

Seems like it could be useful, but is it more useful than the glabel and zpool status sections farther down?

It's definitely not necessary, it's just a time saver that gives you the information at a quick glance instead of having to look at both the zpool and glabel outputs and manually correlate gptids to device names to figure out which devices go together. In particular where I think it adds value is the critical colorization if one of the mirrored pairs is offline. As an example, in my system with 26 drives, /dev/da10 is offline right now. The current script runs and shows me 25 drives that all look healthy, so if I'm not paying close attention I don't notice that missing 26th drive. But the new feature colorizes the partner column of /dev/da8 as critical and says it's partner is "offline", so it's immediately noticeable that something is wrong. Of course the zpool status is showing degraded as well, and TrueNAS sent me a couple emails about the bad drive, so again none of this is necessary, which is why I considered it an option to be controlled by the user in the config file.

The right way to do it is to make sure that nothing changes if it is not set in the config file.

That's exactly how I implemented it, so we're 100% on the same page there!

mth309 commented 2 years ago

One other thing, if you don't mind continuing to explain GitHub to me, but if you decide you don't want to incorporate this feature, is there some way for me to mark that code to not be submitted back to you, but allow me to make other changes and continue to send you new pull requests. Or would I have to create another fork, one that stays in sync with you that I can send pulls against, and one where I go off on my own with the features you don't want?

dak180 commented 2 years ago

One other thing, if you don't mind continuing to explain GitHub to me, but if you decide you don't want to incorporate this feature, is there some way for me to mark that code to not be submitted back to you, but allow me to make other changes and continue to send you new pull requests.

The key terms here are branch and rebase.