[drloco]

Hi, I noticed a bug in how XBMC handles multiple shares when more than 1 NFS server is involved. This is probably only an issue on systems where the netbios lookup of the host name isn't available (OS X. iOS, Windows). Example:

I have 2 video shares added in XBMC: "Movies NAS" and "Movies MacMini". They are both added using the servers' host name (as opposed to IP-address). Only 1 of them works. The share which isn't working is trying to access the working server's export.

Poorly explained example, I know. Maybe explaining the fix will make it clearer.

The CNfsConnection class is persisting the resolved IP address of the last used host. When it goes to resolve a new host name, it sends the resolved IP address as an argument to CDNSNameCache::Lookup, which in some cases will return if it's non-empty (the logic assumes it has been successfully resolved if it's non-empty).

The following patch fixes the issue:

Code:

diff --git a/xbmc/filesystem/NFSFile.cpp b/xbmc/filesystem/NFSFile.cpp
index 9fc26b0..fb486fb 100644
--- a/xbmc/filesystem/NFSFile.cpp
+++ b/xbmc/filesystem/NFSFile.cpp
@@ -78,6 +78,10 @@ CNfsConnection::~CNfsConnection()
void CNfsConnection::resolveHost(const CURL &url)
{
   //resolve if hostname has changed
+  if(url.GetHostName() != m_resolvedHostName)
+  {
+      m_resolvedHostName.clear();
+  }
   CDNSNameCache::Lookup(url.GetHostName(), m_resolvedHostName);
}

Moving CDNSNameCache::Lookup(url.GetHostName(), m_resolvedHostName) into the if-statement is probably ok.

I'm not sure how CDNSNameCache::Lookup is supposed to behave, but maybe always clearing strIpAddress in there is a better fix than the above (although riskier...).

Hope someone more git savvy than me can fix this.

[Memphiz]

thx for the patch - looks good to me. Will add it to mainline once i have time again.

[Memphiz]

You found a problem with the caching code which assumed that the strIpAddress parameter was empty. When the nmlookup failed it would just cache the callers strIpAddress when it wasn't empty.

This should be the proper fix - could you give it a shot please?

Code:

diff --git a/xbmc/network/DNSNameCache.cpp b/xbmc/network/DNSNameCache.cpp
index 7787fe4..cfff714 100644
--- a/xbmc/network/DNSNameCache.cpp
+++ b/xbmc/network/DNSNameCache.cpp
@@ -56,6 +56,7 @@ bool CDNSNameCache::Lookup(const CStdString& strHostName, CStdString& strIpAddre
   char line[200];

   CStdString cmd = "nmblookup " + strHostName;
+  CStdString tmpIpAdr;
   FILE* fp = popen(cmd, "r");
   if (fp)
   {
@@ -64,14 +65,15 @@ bool CDNSNameCache::Lookup(const CStdString& strHostName, CStdString& strIpAddre
       if (sscanf(line, "%99s *<00>\n", nmb_ip))
       {
         if (inet_addr(nmb_ip) != INADDR_NONE)
-          strIpAddress = nmb_ip;
+          tmpIpAdr = nmb_ip;
       }
     }
     pclose(fp);
   }

-  if (!strIpAddress.IsEmpty())
+  if (!tmpIpAdr.IsEmpty())
   {
+    strIpAddress = tmpIpAdr;
     g_DNSCache.Add(strHostName, strIpAddress);
     return true;
   }

[drloco]

Yeah that fix works great Memphiz! I agree it's a much better solution than mine

[Memphiz]

Committed to mainline ... still modified it to just clear the strIpAddress - since it won't have any influences on the callers and is the right thing to do - thx.