Re: [Libguestfs] [nbdkit PATCH 2/4] file: Add .list_exports support
On Thu, Aug 06, 2020 at 09:23:46PM -0500, Eric Blake wrote:
+ if (!filename == !directory) {
A bit tricksy. In plugins/nbd/nbd.c I used:
int c = !!sockname + !!hostname + !!uri +
(command.size > 0) + (socket_fd >= 0) + !!raw_cid;
/* Check the user passed exactly one connection parameter. */
if (c > 1) {
nbdkit_error ("cannot mix Unix ‘socket’, TCP ‘hostname’/‘port’, ‘vsock’, "
"‘command’, ‘socket-fd’ and ‘uri’ parameters");
return -1;
}
if (c == 0) {
nbdkit_error ("exactly one of ‘socket’, ‘hostname’, ‘vsock’, ‘command’, "
"‘socket-fd’ and ‘uri’ parameters must be specified");
return -1;
}
which might be longer but a bit clearer?
+ if (directory && (dir = opendir (directory)) == NULL) {
+ nbdkit_error ("opendir: %m");
return -1;
}
What happens if the contents of the directory change while the file
plugin is running? I guess we're using rewinddir so that's fine.
But maybe we want to defer opening this until we actually do list_exports?
I can see arguments both ways TBH, one obvious one being that it'd be
nice to fail early if directory doesn't exist at all. However if we
defer opening the directory til it is needed then we would have to use
nbdkit_realpath on the directory name, and that should check that the
directory (or something) exists.
+ if (directory) {
+ const char *exportname = nbdkit_export_name ();
+
+ if (strchr (exportname, '/')) {
+ nbdkit_error ("exportname cannot contain /");
+ errno = EINVAL;
+ free (h);
+ return NULL;
+ }
+ if (asprintf (&h->file, "%s/%s", directory, exportname) == -1) {
+ nbdkit_error ("asprintf: %m");
+ free (h);
+ return NULL;
+ }
+ }
+ else
+ h->file = strdup (filename);
+ if (h->file == NULL) {
+ nbdkit_error ("malloc: %m");
+ free (h);
+ return NULL;
+ }
+
flags = O_CLOEXEC|O_NOCTTY;
if (readonly)
flags |= O_RDONLY;
else
flags |= O_RDWR;
- h->fd = open (filename, flags);
+ h->fd = open (h->file, flags);
Maybe openat is safer than trying to concatenate filenames?
I would say that Windows is part of the argument here, but we don't
support it, and Windows doesn't have openat. Also there's an argument
that you want the full filename for error messages, but do we in fact
want to echo the exportname (client controlled) to log files?
+# Hack: This rejects libnbd 1.3.9, but we really need libnbd >=
1.3.11,
+# which does not have its own decent witness...
+requires nbdsh -c 'print (h.get_list_export_description)'
Some variation on:
$ nbdsh -c 'print(h.get_version())'
1.3.11
$ nbdsh -c 'from packaging import version ; print(version.parse(h.get_version()) >=
version.parse("1.3.11"))'
True
However 1.3.10 was only present for a short time and I already got rid
of it in Rawhide because it affected other nbdkit tests, so probably
what you have already is fine in practice.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top