lib/nfs/Manager: add method MakeConnection()

This uses the libnfs function nfs_parse_url_dir() which means MPD
gains support for the libnfs arguments like "version".

Closes https://github.com/MusicPlayerDaemon/MPD/issues/2039

NEWS

@@ -26,6 +26,7 @@ ver 0.24 (not yet released)
 * storage
   - curl: optimize database update
   - nfs: require libnfs 4.0 or later
+  - nfs: support libnfs URL arguments
 * input
   - alsa: limit ALSA buffer time to 2 seconds
   - curl: add "connect_timeout" configuration

doc/plugins.rst

@@ -102,7 +102,12 @@ nfs
 
 Load music files from a NFS server.  It is used when
 :code:`music_directory` contains a ``nfs://`` URI according to
-RFC2224, for example :samp:`nfs://servername/path`.
+RFC2224, for example :samp:`nfs://servername/path`.  MPD supports the
+libnfs URL arguments as documented in the `libnfs README
+<https://github.com/sahlberg/libnfs/blob/master/README>`__.  For
+example, you can use NFSv4 with the ``version`` argument::
+
+ music_directory "nfs://server/music?version=4"
 
 See :ref:`input_nfs` for more information.
 
@@ -307,7 +312,7 @@ used according to RFC2224. Example:
 
      mpc add nfs://servername/path/filename.ogg
 
-This plugin uses :program:`libnfs`, which supports only NFS version 3.
+This plugin uses :program:`libnfs`.
 Since :program:`MPD` is not allowed to bind to so-called "privileged
 ports", the NFS server needs to enable the ``insecure`` setting;
 example :file:`/etc/exports`:

src/lib/nfs/Connection.cxx

@@ -174,16 +174,15 @@ events_to_libnfs(unsigned i) noexcept
 }
 
 NfsConnection::NfsConnection(EventLoop &_loop,
+			     nfs_context *_context,
 			     std::string_view _server,
 			     std::string_view _export_name)
 	:socket_event(_loop, BIND_THIS_METHOD(OnSocketReady)),
 	 defer_new_lease(_loop, BIND_THIS_METHOD(RunDeferred)),
 	 mount_timeout_event(_loop, BIND_THIS_METHOD(OnMountTimeout)),
 	 server(_server), export_name(_export_name),
-	 context(nfs_init_context())
+	 context(_context)
 {
-	if (context == nullptr)
-		throw std::runtime_error{"nfs_init_context() failed"};
 }
 
 NfsConnection::~NfsConnection() noexcept
@@ -202,7 +201,7 @@ NfsConnection::~NfsConnection() noexcept
 void
 NfsConnection::AddLease(NfsLease &lease) noexcept
 {
-	assert(GetEventLoop().IsInside());
+	assert(!GetEventLoop().IsAlive() || GetEventLoop().IsInside());
 
 	new_leases.push_back(lease);
 
@@ -212,7 +211,7 @@ NfsConnection::AddLease(NfsLease &lease) noexcept
 void
 NfsConnection::RemoveLease(NfsLease &lease) noexcept
 {
-	assert(GetEventLoop().IsInside());
+	assert(!GetEventLoop().IsAlive() || GetEventLoop().IsInside());
 
 	lease.unlink();
 }

src/lib/nfs/Connection.hxx

@@ -131,6 +131,7 @@ public:
 	 */
 	[[gnu::nonnull]]
 	NfsConnection(EventLoop &_loop,
+		      nfs_context *_context,
 		      std::string_view _server,
 		      std::string_view _export_name);
 

src/lib/nfs/Glue.cxx

@@ -39,6 +39,14 @@ nfs_get_event_loop() noexcept
 	return nfs_glue->GetEventLoop();
 }
 
+NfsConnection &
+nfs_make_connection(const char *url)
+{
+	assert(in_use > 0);
+
+	return nfs_glue->MakeConnection(url);
+}
+
 NfsConnection &
 nfs_get_connection(std::string_view server,
 		   std::string_view export_name)

src/lib/nfs/Glue.hxx

@@ -21,6 +21,13 @@ nfs_finish() noexcept;
 EventLoop &
 nfs_get_event_loop() noexcept;
 
+/**
+ * Throws on error.
+ */
+[[nodiscard]]
+NfsConnection &
+nfs_make_connection(const char *url);
+
 /**
  * Throws on error.
  */

src/lib/nfs/Manager.cxx

@@ -3,13 +3,18 @@
 
 #include "Manager.hxx"
 #include "Connection.hxx"
+#include "Error.hxx"
 #include "lib/fmt/ExceptionFormatter.hxx"
 #include "event/Loop.hxx"
 #include "util/DeleteDisposer.hxx"
 #include "util/Domain.hxx"
-#include "util/StringAPI.hxx"
+#include "util/ScopeExit.hxx"
 #include "Log.hxx"
 
+extern "C" {
+#include <nfsc/libnfs.h>
+}
+
 static constexpr Domain nfs_domain("nfs");
 
 class NfsManager::ManagedConnection final
@@ -20,9 +25,10 @@ class NfsManager::ManagedConnection final
 
 public:
 	ManagedConnection(NfsManager &_manager, EventLoop &_loop,
+			  struct nfs_context *_context,
 			  std::string_view _server,
 			  std::string_view _export_name)
-		:NfsConnection(_loop, _server, _export_name),
+		:NfsConnection(_loop, _context, _server, _export_name),
 		 manager(_manager) {}
 
 protected:
@@ -54,6 +60,28 @@ NfsManager::~NfsManager() noexcept
 	connections.clear_and_dispose(DeleteDisposer());
 }
 
+NfsConnection &
+NfsManager::MakeConnection(const char *url)
+{
+	struct nfs_context *const context = nfs_init_context();
+	if (context == nullptr)
+		throw std::runtime_error{"nfs_init_context() failed"};
+
+	auto *pu = nfs_parse_url_dir(context, url);
+	if (pu == nullptr) {
+		AtScopeExit(context) { nfs_destroy_context(context); };
+		throw NfsClientError(context, "nfs_parse_url_dir() failed");
+	}
+
+	AtScopeExit(pu) { nfs_destroy_url(pu); };
+
+	auto c = new ManagedConnection(*this, GetEventLoop(),
+				       context,
+				       pu->server, pu->path);
+	connections.push_front(*c);
+	return *c;
+}
+
 NfsConnection &
 NfsManager::GetConnection(std::string_view server, std::string_view export_name)
 {
@@ -64,7 +92,12 @@ NfsManager::GetConnection(std::string_view server, std::string_view export_name)
 		    c.GetExportName() == export_name)
 			return c;
 
+	struct nfs_context *const context = nfs_init_context();
+	if (context == nullptr)
+		throw std::runtime_error{"nfs_init_context() failed"};
+
 	auto c = new ManagedConnection(*this, GetEventLoop(),
+				       context,
 				       server, export_name);
 	connections.push_front(*c);
 	return *c;

src/lib/nfs/Manager.hxx

@@ -42,6 +42,20 @@ public:
 	}
 
 	/**
+	 * Create a new #NfsConnection, parsing the specified "nfs://"
+	 * URL.
+	 *
+	 * Throws on error.
+	 */
+	[[nodiscard]]
+	NfsConnection &MakeConnection(const char *url);
+
+	/**
+	 * Look up an existing #NfsConnection (or create a new one if
+	 * none matching the given parameters exists).  Unlike
+	 * MakeConnection(), this does not support options in a query
+	 * string.
+	 *
 	 * Throws on error.
 	 */
 	[[nodiscard]]

src/storage/plugins/NfsStorage.cxx

@@ -26,12 +26,16 @@ extern "C" {
 #include <nfsc/libnfs-raw-nfs.h>
 }
 
+#include <fmt/core.h>
+
 #include <cassert>
 #include <string>
 
 #include <sys/stat.h>
 #include <fcntl.h>
 
+using std::string_view_literals::operator""sv;
+
 class NfsStorage final
 	: public Storage, NfsLease {
 
@@ -39,9 +43,16 @@ class NfsStorage final
 		INITIAL, CONNECTING, READY, DELAY,
 	};
 
-	const std::string base;
+	/**
+	 * The full configured URL (with all arguemnts).  This is used
+	 * to reconnect.
+	 */
+	const std::string url;
 
-	const std::string server, export_name;
+	/**
+	 * The base URL for building file URLs (without arguments).
+	 */
+	const std::string base;
 
 	NfsConnection *connection;
 
@@ -50,18 +61,20 @@ class NfsStorage final
 
 	Mutex mutex;
 	Cond cond;
-	State state = State::INITIAL;
+	State state = State::CONNECTING;
 	std::exception_ptr last_exception;
 
 public:
-	NfsStorage(EventLoop &_loop, const char *_base,
+	NfsStorage(const char *_url, NfsConnection &_connection)
-		   std::string_view _server, std::string_view _export_name)
+		:url(_url),
-		:base(_base),
+		 base(fmt::format("nfs://{}{}"sv, _connection.GetServer(), _connection.GetExportName())),
-		 server(_server),
+		 connection(&_connection),
-		 export_name(_export_name),
+		 defer_connect(_connection.GetEventLoop(), BIND_THIS_METHOD(OnDeferredConnect)),
-		 defer_connect(_loop, BIND_THIS_METHOD(OnDeferredConnect)),
+		 reconnect_timer(_connection.GetEventLoop(), BIND_THIS_METHOD(OnReconnectTimer))
-		 reconnect_timer(_loop, BIND_THIS_METHOD(OnReconnectTimer)) {
+	{
-		nfs_init(_loop);
+		BlockingCall(GetEventLoop(), [this](){
+			connection->AddLease(*this);
+		});
 	}
 
 	~NfsStorage() override {
@@ -142,7 +155,7 @@ private:
 		assert(GetEventLoop().IsInside());
 
 		try {
-			connection = &nfs_get_connection(server, export_name);
+			connection = &nfs_make_connection(url.c_str());
 		} catch (...) {
 			SetState(State::DELAY, std::current_exception());
 			reconnect_timer.Schedule(std::chrono::minutes(10));
@@ -394,20 +407,20 @@ NfsStorage::OpenDirectory(std::string_view uri_utf8)
 static std::unique_ptr<Storage>
 CreateNfsStorageURI(EventLoop &event_loop, const char *base)
 {
-	const char *p = StringAfterPrefixCaseASCII(base, "nfs://");
+	if (!StringStartsWithCaseASCII(base, "nfs://"sv))
-	if (p == nullptr)
 		return nullptr;
 
-	const char *slash = std::strchr(p, '/');
+	nfs_init(event_loop);
-	if (slash == nullptr)
-		throw std::runtime_error("Malformed nfs:// URI");
 
-	const std::string_view server{p, slash}, mount{slash};
+	try {
+		auto &connection = nfs_make_connection(base);
+		nfs_set_base(connection.GetServer(), connection.GetExportName());
 
-	nfs_set_base(server, mount);
+		return std::make_unique<NfsStorage>(base, connection);
-
+	} catch (...) {
-	return std::make_unique<NfsStorage>(event_loop, base,
+		nfs_finish();
-					    server, mount);
+		throw;
+	}
 }
 
 static constexpr const char *nfs_prefixes[] = { "nfs://", nullptr };