3:41 p.m.
On 8/14/20 12:20 PM, Richard W.M. Jones wrote:
This creates filesystems on demand. A client simply connects with a
desired export name and a new export is created. The export is
persistent (until deleted by the server admin), and clients may
disconnect and reconnect. In some respects this is similar to the
nbdkit-tmpdisk-plugin, or nbdkit-file-plugin with the dir= option.
---
+
+This is a plugin for L<nbdkit(1)> which creates persistent filesystems
+on demand. Clients may simply connect to the server, requesting a
+particular export name, and a new filesystem is created if it does not
+exist already. Clients can also disconnect and reconnect with the
+same export name and the same filesystem will still be available.
+Filesystems are stored in a directory on the server, so they also
+persist over nbdkit and server restarts.
+
+Each filesystem is locked while it is in use by a client, preventing
+two clients from accessing the same filesystem (which would cause
+corruption).
A rather crucial difference from the file plugin dir= mode ;)
+
+Similar plugins include L<nbdkit-file-plugin(1)> which can serve a
+predefined set of exports (clients cannot create more),
Hmm - I wonder if it is worth a filter that runs a script any time an
.open fails. That script could send email to an administrator, or even
create a filesystem on the user's behalf; then run that filter in front
of the file plugin to let the user create images after all. The
exclusion between clients is an interesting trick to figure out, though
(the existing --filter=noparallel is too strict - you want parallel
clients visiting different files, but not parallel clients visiting the
same file).
+=head2 Security considerations
+
+You should B<only> use this in an environment where you trust all your
+clients, since clients can use this plugin to consume arbitrary
+amounts of disk space by creating unlimited exports. It is therefore
+best to take steps to limit where clients can connect from using
+L<nbdkit-ip-filter(1)>, firewalls, or TLS client certificates.
Yep, a definite need for this warning.
+++ b/plugins/ondemand/ondemand.c
+/* Because we rewind the exportsdir handle, we need a lock to
protect
+ * list_exports from being called in parallel.
+ */
+static pthread_mutex_t exports_lock = PTHREAD_MUTEX_INITIALIZER;
An alternative is to diropen() each time .list_exports gets called.
Either way should work, though.
I still have some pending patches to improve .list_exports (split out a
.default_export function, add an is_tls parameter), so there may be some
churn in this area (for that matter, I have not yet pushed my patches
for .list_exports in the file plugin, because I was trying to minimize
churn while working on pending patches). We'll just have to see how it
goes; I don't mind rebasing.
+
+static int
+ondemand_list_exports (int readonly, int default_only,
+ struct nbdkit_exports *exports)
+{
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&exports_lock);
+ struct dirent *d;
+
+ /* First entry should be the default export. XXX Should we check if
+ * the "default" file was created? I don't think we need to.
+ */
+ if (nbdkit_add_export (exports, "", NULL) == -1)
+ return -1;
+ if (default_only) return 0;
+
+ /* Read the rest of the exports. */
+ rewinddir (exportsdir);
+
+ /* XXX Output is not sorted. Does it matter? */
+ while (errno = 0, (d = readdir (exportsdir)) != NULL) {
+ /* Skip all dot files. "." anywhere in the export name is
+ * rejected by the plugin, so commands can use dot files to "hide"
+ * files in the export dir (eg. if needing to keep state).
+ */
+ if (d->d_name[0] == '.')
+ continue;
This skips dot files (leading dot); but not those containing '.' or ':'
elsewhere. Did you want to skip more files, so that you aren't
advertising stuff that can't pass open?
+
+ /* Skip the "default" filename which refers to the "" export.
*/
+ if (strcmp (d->d_name, "default") == 0)
+ continue;
+
+ if (nbdkit_add_export (exports, d->d_name, NULL) == -1)
+ return -1;
+ }
+
+ /* Did readdir fail? */
+ if (errno != 0) {
+ nbdkit_error ("readdir: %s: %m", dir);
+ return -1;
+ }
+
+ return 0;
+}
+
+struct handle {
+ int fd;
+ int64_t size;
+ const char *exportname;
+ bool can_punch_hole;
+};
+
+/* Since clients that want multi-conn should all pass the same export
+ * name, this is safe.
+ */
+static int
+ondemand_can_multi_conn (void *handle)
+{
+ return 1;
+}
Hmm. Are you permitting multiple clients to the same export, or did you
decide that was too likely to cause fs corruption, and locking out users
on the same export? The docs above said otherwise, making this look wrong.
+
+static int
+ondemand_can_trim (void *handle)
+{
+#ifdef FALLOC_FL_PUNCH_HOLE
+ return 1;
+#else
+ return 0;
+#endif
+}
The more plugins we add that touch the file system, the more it seems
like it is worth our while to factor out helper libraries.
+
+ /* Lock the file to prevent filesystem corruption. It's safe for
+ * all clients to be reading. If a client wants to write it must
+ * have exclusive access.
Ah, you aren't locking out parallel readers, but rather doing .pwrite
locking. Interesting.
+ *
+ * This uses a currently Linux-specific extension. It requires
+ * Linux >= 3.15 (released in 2014, later backported to RHEL 7).
+ * There is no sensible way to do this in pure POSIX.
+ */
+#ifdef F_OFD_SETLK
+ memset (&lock, 0, sizeof lock);
+ if (readonly)
+ lock.l_type = F_RDLCK;
+ else
+ lock.l_type = F_WRLCK;
+ lock.l_whence = SEEK_SET;
+ lock.l_start = 0;
+ lock.l_len = 0;
+ if (fcntl (h->fd, F_OFD_SETLK, &lock) == -1) {
+ if (errno == EACCES || errno == EAGAIN) {
+ nbdkit_error ("%s: filesystem is locked by another client",
+ h->exportname);
+ /* XXX Would be nice if NBD protocol supported some kind of "is
+ * locked" indication. If it did we could use it here.
Yeah, I've thought about the idea of adding a way for clients to
advertise their intent for read vs. read-write during handshaking.
+++ b/plugins/ondemand/default-command.sh.in
@@ -0,0 +1,57 @@
+# nbdkit
+# -*- mode: shell-script -*-
+# Copyright (C) 2017-2020 Red Hat Inc.
+#
No bash shebang line, but it looks portable to POSIX /bin/sh.
+++ b/tests/test-ondemand.sh
+++ b/TODO
@@ -154,6 +154,13 @@ nbdkit-torrent-plugin:
* The C++ could be a lot more natural. At the moment it's a kind of
“C with C++ extensions”.
+nbdkit-ondemand-plugin:
+
+* Implement more callbacks, eg. .zero
+
+* Allow client to select size up to a limit, eg. by sending export
+ names like ‘export:4G’.
I like the idea for using exportnames to pass in additional information.
Overall looks pretty reasonable.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org