• [6.9.2] Fix for Unassigned Devices temperature notifications

    bland328

    By bland328

      • Minor

    tl;dr: It appears to me that Unraid 6.9.2 doesn't honor device-specific temperature notification settings for Unassigned Devices for a straightforward reason that is easily fixed.

     

    Now that I have two unassigned NVME drives in my Unraid server, the annoyance of over-temp notifications that ignore the per-device settings has doubled, so I've come up with what is hopefully a true fix, rather than a workaround, in the form of a small change to the check_temp function in /usr/local/emhttp/plugins/dynamix/scripts/monitor.

     

    Here's the diff for /usr/local/emhttp/plugins/dynamix/scripts/monitor in Unraid 6.9.2: 

    61,62c61,66
<   global $notify,$disks,$saved,$display,$server,$top;
<   $disk = &$disks[$name];
---
>   global $notify,$disks,$devs,$saved,$display,$server,$top;
>   if (isset($disks[$name])) {
>     $disk = &$disks[$name];
>   } else {
>     $disk = &$devs[$name];
>   }

     

    The logic behind the change is that it appears to me that while the $devs array does properly include the hotTemp ("Warning disk temperature threshold") and maxTemp ("Critical disk temperature threshold") values as a result of CustomMerge.php merging them from the /boot/config/smart-one.cfg file, the check_temp function in 6.9.2 fails to consider the $devs array at all.

     

    This patch changes check_temp so that $devs is included as a global, so that if the passed $name can't be found in the $disks array, a lookup in $devs can be attempted, instead.

     

    I suspect there's a more elegant way to implement the fallback from $disks to $devs, but I'll leave that as an exercise for people who know PHP well :)

     

    I don't claim this to be well-researched or production-quality code, but it does fundamentally make sense, and It Works On My System™, so I hope this is helpful.

    • Like 1
    • Thanks 1
    Go to reports Stable Releases

    User Feedback

    Recommended Comments

    bonienl

    Regular disks and unassigned disks are called separately in the monitor script 

    // check array devices
foreach ($disks as $disk) {
..
// process disk temperature notifications
  check_temp($disk,$text,$info);
..
}

// check unassigned devices
foreach ($devs as $dev) {
..
// process disk temperature notifications
  check_temp($dev,$text,$info);
..
}

     

    Link to comment
    bland328

    Hmm...very strange--the code you posted doesn't match the version of the monitor script I'm running. On my 6.9.2 install, /usr/local/emhttp/plugins/dynamix/scripts/monitor calls check_temp thusly:

      

    // check array devices
foreach ($disks as $disk) {
..
// process disk temperature notifications
  check_temp($name,$disk['temp'],$text,$info);
..
}

// check unassigned devices
foreach ($devs as $dev) {
..
// process disk temperature notifications
  check_temp($name,$temp,$text,$info);
..
}

     

    This explains why the tweak I posted makes check_temp work properly on my system, but doesn't explain why my 6.9.2 monitor code differs from what you expect.

     

    Might the code you posted be from a post-6.9.2 revision?

    Link to comment
    bland328

    Ah--that's excellent, thanks. The new approach of passing the appropriate $disk or $dev to check_temp (rather than just $name) makes a bunch of sense.

    Link to comment
    ezhik

    Can confirm run into the same issue. Drives that are not assigned to an array and for example are used as passthrough drives for the VM, send out notifications for drives surpassing the default thresholds of 45C, regardless of the values you set. 

    Link to comment
    Joshndroid

    If i implement this 'fix' will this clash with any upgrade to 6.10 in the future? or will 6.10 just flat out overwrite this area such that i dont have keep track of this change and revert it before upgrading to 6.10? (just a bit of a sanity check before i implement it). I am seeing this exact error constantly on my NVME drive i use for my VM's and its frustrating as heck.

    Link to comment


    Join the conversation

    You can post now and register later. If you have an account, sign in now to post with your account.
    Note: Your post will require moderator approval before it will be visible.

    Guest
    Add a comment...

    ×   Pasted as rich text.   Restore formatting

      Only 75 emoji are allowed.

    ×   Your link has been automatically embedded.   Display as a link instead

    ×   Your previous content has been restored.   Clear editor

    ×   You cannot paste images directly. Upload or insert images from URL.

    ×

  • Status Definitions

     

    Open = Under consideration.

     

    Solved = The issue has been resolved.

     

    Solved version = The issue has been resolved in the indicated release version.

     

    Closed = Feedback or opinion better posted on our forum for discussion. Also for reports we cannot reproduce or need more information. In this case just add a comment and we will review it again.

     

    Retest = Please retest in latest release.

    Priority Definitions

     

    Minor = Something not working correctly.

     

    Urgent = Server crash, data loss, or other showstopper.

     

    Annoyance = Doesn't affect functionality but should be fixed.

     

    Other = Announcement or other non-issue.