ThemeUpdateChecker accesses Videos storage group

[thigger@…]

Kind of expecting Wontfix on this as it's fairly minor, but it's causing a lot of trouble for me!

To reproduce

• use fixes/0.27 with Mythbuntu theme, theme update checker turned on, and a Videos storage group defined

Expected

• No access to videos storage group

Result

• Videos storage group is accessed (I believe that when the file isn't found, MythTV falls back to checking every storage group)

Why this is important

I store my videos on a remote machine which is woken up on file access using autofs - when I use a Videos storage group then it gets woken up every hour by ThemeUpdateChecker?. Adding the videos directory directly to the frontend avoids the problem (though doesn't behave as well when using multiple directories)

I wonder what other situations would also expose this behaviour of scanning inappropriate directories - is this a general problem with storage groups?

Log

2016-03-06 02:06:39.093266 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/themes.zip
2016-03-06 02:06:39.093418 I  MythSocket(958ec58:61): write -> 61 54
   QUERY_FILE_EXISTS[]:[]remotethemes/themes.zip[]:[]Temp
2016-03-06 02:06:39.096212 I  MythSocket(958ec58:61): read  <- 61 184
   1[]:[]/home/mythtv/.mythtv/tmp/remotethemes/themes.zip[]:[]2069[]...
2016-03-06 02:06:39.096413 I  ThemeUpdateChecker Loading
'myth://Temp@tol-htpc/remotethemes/0.27.4/Mythbuntu/themeinfo.xml'
2016-03-06 02:06:39.096478 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/0.27.4/Mythbuntu/themeinfo.xml
2016-03-06 02:06:39.096583 I  MythSocket(958ec58:61): write -> 61 74
   QUERY_FILE_EXISTS[]:[]remotethemes/0.27.4/Mythbuntu/themeinfo.xml...
2016-03-06 02:06:39.141926 I  MythSocket(958ec58:61): read  <- 61 1       0
2016-03-06 02:06:39.142067 I  ThemeUpdateChecker Loading
'myth://Temp@tol-htpc/remotethemes/0.27.3/Mythbuntu/themeinfo.xml'
2016-03-06 02:06:39.142138 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/0.27.3/Mythbuntu/themeinfo.xml
2016-03-06 02:06:39.142274 I  MythSocket(958ec58:61): write -> 61 74
   QUERY_FILE_EXISTS[]:[]remotethemes/0.27.3/Mythbuntu/themeinfo.xml...
2016-03-06 02:06:39.144938 I  MythSocket(958ec58:61): read  <- 61 1       0
2016-03-06 02:06:39.150876 I  ThemeUpdateChecker Loading
'myth://Temp@tol-htpc/remotethemes/0.27.2/Mythbuntu/themeinfo.xml'
2016-03-06 02:06:39.150941 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/0.27.2/Mythbuntu/themeinfo.xml
2016-03-06 02:06:39.151058 I  MythSocket(958ec58:61): write -> 61 74
   QUERY_FILE_EXISTS[]:[]remotethemes/0.27.2/Mythbuntu/themeinfo.xml...
2016-03-06 02:06:39.153438 I  MythSocket(958ec58:61): read  <- 61 1       0
2016-03-06 02:06:39.153545 I  ThemeUpdateChecker Loading
'myth://Temp@tol-htpc/remotethemes/0.27.1/Mythbuntu/themeinfo.xml'
2016-03-06 02:06:39.153605 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/0.27.1/Mythbuntu/themeinfo.xml
2016-03-06 02:06:39.153705 I  MythSocket(958ec58:61): write -> 61 74
   QUERY_FILE_EXISTS[]:[]remotethemes/0.27.1/Mythbuntu/themeinfo.xml...
2016-03-06 02:06:39.155967 I  MythSocket(958ec58:61): read  <- 61 1       0
2016-03-06 02:06:39.156072 I  ThemeUpdateChecker Loading
'myth://Temp@tol-htpc/remotethemes/0.27/Mythbuntu/themeinfo.xml'
2016-03-06 02:06:39.156133 I  RemoteFile::Exists(): looking for remote
file: myth://Temp@tol-htpc/remotethemes/0.27/Mythbuntu/themeinfo.xml
2016-03-06 02:06:39.156312 I  MythSocket(958ec58:61): write -> 61 72
   QUERY_FILE_EXISTS[]:[]remotethemes/0.27/Mythbuntu/themeinfo.xml[]...
2016-03-06 02:06:39.157263 I  MythSocket(958ec58:61): read  <- 61 200
   1[]:[]/home/mythtv/.mythtv/tmp/remotethemes/0.27/Mythbuntu/themei...
2016-03-06 02:06:39.158085 I  MythSocket(9769de0:-1): MythSocket(-1, 0x0) ctor
2016-03-06 02:06:39.158397 I  MythSocket(9769de0:-1): IP is local,
using loopback address instead
2016-03-06 02:06:39.158423 I  MythSocket(9769de0:-1): attempting
connect() to (::1:6543)
2016-03-06 02:06:39.158823 I  MythSocket(9769de0:57): Connected to (::1:6543)
2016-03-06 02:06:39.159035 I  MythSocket(9769de0:57): write -> 57 30
   MYTH_PROTO_VERSION 77 WindMark
2016-03-06 02:06:39.160563 I  MythSocket(9769de0:57): read  <- 57 13
   ACCEPT[]:[]77
2016-03-06 02:06:39.160674 I  MythSocket(9769de0:57): write -> 57 23
   ANN Playback tol-htpc 0
2016-03-06 02:06:39.160946 I  MythSocket(9769de0:57): read  <- 57 2       OK
2016-03-06 02:06:39.161035 I  MythSocket(9818110:-1): MythSocket(-1, 0x0) ctor
2016-03-06 02:06:39.161264 I  MythSocket(9818110:-1): IP is local,
using loopback address instead
2016-03-06 02:06:39.161283 I  MythSocket(9818110:-1): attempting
connect() to (::1:6543)
2016-03-06 02:06:39.161587 I  MythSocket(9818110:60): Connected to (::1:6543)
2016-03-06 02:06:39.161788 I  MythSocket(9818110:60): write -> 60 30
   MYTH_PROTO_VERSION 77 WindMark
2016-03-06 02:06:39.162060 I  MythSocket(9818110:60): read  <- 60 13
   ACCEPT[]:[]77
2016-03-06 02:06:39.162172 I  MythSocket(9818110:60): write -> 60 87
   ANN FileTransfer tol-htpc 0 0
0[]:[]/remotethemes/0.27/Mythbuntu/...
2016-03-06 02:06:39.164168 I  MythSocket(9818110:60): read  <- 60 19
   OK[]:[]122[]:[]1257
2016-03-06 02:06:39.164309 I  MythSocket(9769de0:57): write -> 57 39
   QUERY_FILETRANSFER 122[]:[]REQUEST_SIZE
2016-03-06 02:06:39.164651 I  MythSocket(9769de0:57): read  <- 57 10
   1257[]:[]1
2016-03-06 02:06:39.164739 I  MythSocket(9769de0:57): write -> 57 49
   QUERY_FILETRANSFER 122[]:[]REQUEST_BLOCK[]:[]1257
2016-03-06 02:06:39.165437 I  MythSocket(9769de0:57): read  <- 57 4       1257
2016-03-06 02:06:39.165508 I  MythSocket(9769de0:57): write -> 57 31
   QUERY_FILETRANSFER 122[]:[]DONE
2016-03-06 02:06:39.165946 I  MythSocket(9769de0:57): read  <- 57 2       OK
2016-03-06 02:06:39.165980 I  (0x9818118)::DecrRef() -> 0
2016-03-06 02:06:39.165997 I  MythSocket(9818110:60): MythSocket dtor : cb 0x0
2016-03-06 02:06:39.166309 I  (0x9769de8)::DecrRef() -> 0
2016-03-06 02:06:39.166325 I  MythSocket(9769de0:57): MythSocket dtor : cb 0x0

[stuartm]

I agree that this behaviour is unwanted, even obnoxious. We shouldn't be searching storage groups of a different type.

[thigger@…]

mythfrontend and backend versions

[thigger@…]

Is this what's responsible? (from mythtv/libs/libmythbase/storagegroup.cpp)

I'm not sure I understand the logic (genuinely, I don't really get storage groups yet) - elsewhere in the code it talks about the 'Default' storage group but here it seems to go for Videos if it can't find what it needs?

    if (s_groupToUseCache.contains(groupKey))
    {
        tmpGroup = s_groupToUseCache[groupKey];
    }
    else
    {
        if (StorageGroup::FindDirs(sgroup, host))
        {
            s_groupToUseCache[groupKey] = sgroup;
        }
        else
        {
            LOG(VB_FILE, LOG_DEBUG,
                    QString("GetGroupToUse(): "
                            "falling back to Videos Storage Group for host %1 "
                            "since it does not have a %2 Storage Group.")
                    .arg(host).arg(sgroup));

            tmpGroup = "Videos";
            s_groupToUseCache[groupKey] = tmpGroup;
        }
    }

    return tmpGroup;

[thigger@…]

I've edited my storagegroup.cpp in two places:

StorageGroup::FindDirs?

• set group to "Default" if function called with a blank group, so not all groups are searched

StorageGroup::GetGroupToUse?

• fall back to "Default" rather than "Videos"

It's been long enough since I played with the MythTV source that I have no idea what the side-effects of these changes will be; so I can't recommend these changes unless someone who knows what they're talking about has had a look. All I can say is that my system seems to be working fine and the Videos group is only being accessed when it should be.

[thigger@…]

Patch I'm running with at present (may not be recommended!)

[sphery]

This needs to be changed at a very specific level, rather than through a global change. Much of the code in MythTV is dependent upon the very fallback structure that was coded into the Storage Groups.

Therefore, if you want to "fix" this--i.e. to make an exception to the designed-in SG fallback--then it needs to be done through the addition of a new method(s) to search only the specified group without fallback, or at the least, a new parameter that disables the fallback. Specifically (since this allowFallback functionality already exists in StorageGroup?), you'd need to add code to RemoteFile::Exists() to allow searching for the file without fallback--i.e. creating the StorageGroup? with allowFallback set to false at:

​https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythbase/remotefile.cpp#L493

(which means adding parameters to all the methods preceding that call as well as coming up with some mechanism by which to specify no fallback via the MythTV RemoteFile? URL).

There's no concept, at this point, of Storage Group types, so we shouldn't even assume that we should only search SGs of some type or purpose in any situation. The closest thing we have to SG types is the built-in SGs, which are generally used to define locations for specific data, but there is no guarantee (or requirement) that those group directories contain only that specific data--the user could put any MythTV files in any of those group directories--and, in truth, we've never previously required that the user put that data only in the specified group directories. Therefore, we need task-specific (and specifically requested) overrides of the fallback functionality rather than global changes to the coded SG behavior or assumptions that the coded SG behavior should be ignored.

[thigger@…]

Happy to submit something but I'm confused - there seems to be two types of 'fallback'

• explicit fallback by setting allowFallback 'true'
• implicit fallback by setting the group name to

As far as I can see I've messed with the second only (by making equivalent to 'Default') - is that something that's used a lot?

Fundamentally I guess there isn't much point fixing this so long as it's considered that Storage Groups don't have 'types' (personally I'd have thought that they should, and looks like stuartm was under that impression too)

[sphery]

allowFallback defaults to true

​https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythbase/storagegroup.h#L14

Therefore, unless disabling fallback is explicitly requested, MythTV will always fall back to searching through all SGs.

So, since ThemeUpdateChecker::checkForUpdate() uses RemoteFile::Exists():

​https://github.com/MythTV/mythtv/blob/master/mythtv/programs/mythfrontend/themechooser.cpp#L1090

which creates a StorageGroup? without specifying allowFallback:

​https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythbase/remotefile.cpp#L493

and RemoteFile::Exists() sends a QUERY_FILE_EXISTS, which is handled by MainServer::HandleQueryFileExists?() which creates a StorageGroup? without specifying allowFallback:

​https://code.mythtv.org/cgit/mythtv/tree/mythtv/programs/mythbackend/mainserver.cpp#n3513

the ThemeUpdateChecker? (and, actually, any use of RemoteFile?) will always use allowFallback = true.

Modifying RemoteFile? to allow MythTV URLs that specify allowFallback (or even the quick and dirty approach of creating a new method for RemoteFile? that doesn't do fallback--though this would still require modifying the QUERY_FILE_EXISTS backend protocol command to permit specifying allowFallback since adding a new backend protocol command would be extremely dirty) would allow code that doesn't desire fallback to do a quick check of only the specified SG's directories.