[libnbd PATCH 0/3] Expose server block size constraints
by Eric Blake
Necessary when writing a client that wants to avoid unnecessary EINVAL
errors from sending unaligned requests.
At some point, we may want to add synchronous convenience API wrappers
that do request splitting or read-modify-write to obey server
constraints while still appearing to the library client as accepting
any possible request. But such a wrapper should only be synchronous
and not copied to nbd_aio_*, because getting locking right is a bear
if you have two non-overlapping client requests that want to visit the
same larger-granularity window of the server in parallel.
Eric Blake (3):
states: Request NBD_INFO_BLOCK_SIZE during NBD_OPT_GO
api: Add new API to query negotiated block size
nbdinfo: Expose block size constraints
info/nbdinfo.pod | 6 +++
lib/internal.h | 7 +++
lib/nbd-protocol.h | 10 ++++-
generator/API.ml | 70 +++++++++++++++++++++++++++---
generator/state_machine.ml | 7 +++
generator/states-newstyle-opt-go.c | 31 +++++++++++--
lib/flags.c | 18 +++++++-
info/nbdinfo.c | 18 ++++++++
8 files changed, 156 insertions(+), 11 deletions(-)
--
2.27.0
3 years, 9 months
virt-v2v - Window firstboot service questions
by Sam Eiderman
Hi,
It seems that on Windows we create the following dir:
/Program Files/Guestfs/Firstboot/
Where '/' is the os volume (where /Windows reside)
Is it possible that Program Files is not actually located on the os volume?
(i.e. C:\Windows, D:\Program Files on original vm)
Does virt-v2v or even libguestfs's inspect_os() even support that?
(Looking around in the code tells me that if the Windows vm has
multiple volumes, or even if the os drive letter is not 'C:', virt-v2v
will not work correctly)
Wouldn't it be safer to create the Guestfs dir directly on root, and
not use the Program Files if it might be on a different volume?
Sam
3 years, 9 months
RFC: nbdkit block size advertisement
by Eric Blake
I'm thinking of adding one or more callbacks to nbdkit to let
plugins/filters enforce various block size alignments (for example, the
swab filter requires 2/4/8 alignment, or VDDK requires 512 alignment,
etc). The NBD protocol currently has NBD_INFO_BLOCK_SIZE which can be
sent in reply to NBD_OPT_GO to tell the client about sizing constraints;
qemu already implements it as both client and server, so we have some
reasonable testing setups (although libnbd will also need some additions
to make it easier to expose constraints to the user, and/or add new
convenience APIs to do blocksize-style read-modify-write at the libnbd
client side rather than needing the blocksize filter in the NBD server
side).
But NBD_INFO_BLOCK_SIZE is not the full picture: it only covers minimum
block size, preferred block size, and maximum data size; there has been
discussion on the NBD list about also advertising maximum action size
(discussion has mentioned trim and/or zero, but even cache could benefit
from larger buffer size than pread), which means we should be thinking
about supporting future protocol extensions in whatever we expose to
plugins.
So, I'm thinking something like the following:
New enum:
NBDKIT_BLOCK_SIZE_GET_MINIMUM
NBDKIT_BLOCK_SIZE_GET_PREFERRED
NBDKIT_BLOCK_SIZE_GET_MAX_DATA
NBDKIT_BLOCK_SIZE_GET_MAX_TRIM
NBDKIT_BLOCK_SIZE_GET_MAX_ZERO
NBDKIT_BLOCK_SIZE_GET_MAX_CACHE
along with a new callback for plugins:
int64_t block_size (void *handle, int which);
where 'which' is one of the enum values. A future nbdkit might request
an enum value not recognized at the time the plugin was compiled, so the
recommended behavior is that a plugin returns -1 on error, 0 to let
nbdkit pick a sane default (including when 'which' was unknown), or a
positive value for an actual limit. For now, nbdkit would advertise
MIN, PREFERRED, and MAX_DATA limits (to clients that use NBD_OPT_GO),
while the others are enforced only internally. The idea is that if the
plugin installs a limit, a client request that violates that limit will
fail with EINVAL for being unaligned, without even asking the plugin to
try the response. nbdkit calls the plugin callback once per connection
per reasonable 'which' (caching the results like it does for .get_size,
and skipping limits where .can_FOO fails). Returning int64_t allows us
to reuse this callback without change when we add v3 callbacks, although
values larger than 4G make no difference at present. I thought the use
of an enum was nicer than filling in a struct whose size might change,
or adding one callback per limit.
Filters can relax limits (such as blocksize turning a plugin MIN 512
into an advertised MIN 1, by doing read-modify-write as needed) or
tighten limits (the file plugin has MIN 1, but the swab filter imposes a
tighter MIN 2). Constraints between limits are as follows:
MIN: must be power of 2, between 1 and 64k; .get_size and .extents must
return results aligned to MIN (as any unaligned tail or extent
transition is inaccessible using only aligned operations). Defaults to 1.
PREFERRED: must be power of 2, between max(512, MIN) and 2M (this upper
limit is not specified by NBD spec, but matches qemu's implementation of
what it uses as the max qcow2 cluster size). Defaults to max(4k, MIN).
MAX_DATA: must be multiple of MIN and at least as large as PREFERRED;
values larger than 64M are okay but clamped by nbdkit's own internal
limits. Defaults to 64M.
MAX_TRIM, MAX_ZERO, MAX_CACHE: must be multiple of MIN, should be at
least as big as MAX_DATA. Values 4G and larger are clamped by nbdkit's
own internal limits. Defaults to 4G-MIN for TRIM/ZERO, and MAX_DATA for
CACHE.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3 years, 10 months
[nbdkit PATCH] server: Reinstate limited use of -e/-exportname.
by Eric Blake
While we are unlikely to change our decision that -e should not
control our response to NBD_OPT_LIST (because we intend to add a new
callback .extents_list for that), it turns out that it is a lot easier
to write:
nbdkit -U - -e foo info --run 'nbdsh -u "$uri" -c "print(h.pread(3, 0))"'
than it is to write the equivalent:
nbdkit -U - info --run 'nbdsh -u nbd+unix:///foo\?socket=$unixsocket \
-c "print(h.pread(3, 0))"'
and our test-long-name.sh was evidence of that fact.
Let's revert that part of the patch, but improve the documentation to
mention that -e is now useful _only_ with --run, and only if the
client uses it.
This partially reverts commit e71178e9b71403, although for -Wshadow
reasons, we now have to name the global variable export_name due to
other code in the meantime using parameters named exportname.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/nbdkit-captive.pod | 12 +++++++++++-
docs/nbdkit-protocol.pod | 2 +-
docs/nbdkit.pod | 18 ++++++++++++++++++
docs/synopsis.txt | 3 ++-
server/internal.h | 1 +
server/captive.c | 20 +++++++++++++++++---
server/main.c | 3 ++-
tests/test-long-name.sh | 10 +++++-----
8 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index f1ac9fc4..94e79c59 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -53,7 +53,9 @@ The following shell variables are available in the I<--run> argument:
A URI that refers to the nbdkit port or socket in the preferred form
documented by the NBD project. As this variable may contain a bare
C<?> for Unix sockets, it is safest to use $uri within double quotes
-to avoid unintentional globbing.
+to avoid unintentional globbing. For plugins that support distinct
+data based on export names, the B<-e> option to nbdkit controls which
+export name will be set in the URI.
=item C<$nbd>
@@ -72,6 +74,14 @@ If E<ne> "", the port number that nbdkit is listening on.
If E<ne> "", the Unix domain socket that nbdkit is listening on.
+=item C<$exportname>
+
+The export name (which may be "") that the process should use when
+connecting to nbdkit, as set by the B<-e> (B<--exportname>) command
+line option of nbdkit. This only matters to plugins that
+differentiate what they serve based on the export name requested by
+the client.
+
=back
I<--run> implies I<--foreground>. It is not possible, and probably
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index e0e4042b..8fe8c67e 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -102,7 +102,7 @@ Partially supported in nbdkit E<ge> 1.1.12. Support for plugins to
read the client export name added in nbdkit E<ge> 1.15.2.
Versions of nbdkit before 1.16 could advertise a single export name to
-clients, specified through the now deprecated I<-e> option. In nbdkit
+clients, via a now deprecated side effect of the I<-e> option. In nbdkit
1.15.2, plugins could read the client requested export name using
C<nbdkit_export_name()> and serve different content.
diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod
index b204709b..e23cc90f 100644
--- a/docs/nbdkit.pod
+++ b/docs/nbdkit.pod
@@ -214,6 +214,24 @@ An alternative to this is L<nbdkit-captive(1)/CAPTIVE NBDKIT>.
This option implies I<--foreground>.
+=item B<-e> EXPORTNAME
+
+=item B<--export> EXPORTNAME
+
+=item B<--export-name> EXPORTNAME
+
+=item B<--exportname> EXPORTNAME
+
+Set a preferred exportname to expose in the shell environment created
+during B<--run>. The use of this option without B<--run> has no
+effect. This option does I<not> change what nbdkit advertises as a
+server, but can aid in writing a captive client that wants to access
+particular content from a plugin that differentiates content based on
+the client's choice of export name.
+
+If not set, the B<--run> environment is set to access the default
+exportname C<""> (empty string).
+
=item B<-f>
=item B<--foreground>
diff --git a/docs/synopsis.txt b/docs/synopsis.txt
index caf49922..07b9dcff 100644
--- a/docs/synopsis.txt
+++ b/docs/synopsis.txt
@@ -1,4 +1,5 @@
-nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N] [--exit-with-parent]
+nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N]
+ [-e|--exportname EXPORTNAME] [--exit-with-parent]
[--filter FILTER ...] [-f|--foreground]
[-g|--group GROUP] [-i|--ipaddr IPADDR]
[--log stderr|syslog|null]
diff --git a/server/internal.h b/server/internal.h
index 4911a49b..1acbbb06 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -108,6 +108,7 @@ enum log_to {
};
extern struct debug_flag *debug_flags;
+extern const char *export_name;
extern bool foreground;
extern const char *ipaddr;
extern enum log_to log_to;
diff --git a/server/captive.c b/server/captive.c
index 53b82782..a8947d7c 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -61,6 +61,9 @@ run_command (void)
if (!run)
return;
+ if (!export_name)
+ export_name = "";
+
fp = open_memstream (&cmd, &len);
if (fp == NULL) {
perror ("open_memstream");
@@ -72,15 +75,26 @@ run_command (void)
if (port) {
fprintf (fp, "nbd://localhost:");
shell_quote (port, fp);
+ if (strcmp (export_name, "") != 0) {
+ putc ('/', fp);
+ uri_quote (export_name, fp);
+ }
}
else if (unixsocket) {
- fprintf (fp, "nbd+unix://\\?socket=");
+ fprintf (fp, "nbd+unix://");
+ if (strcmp (export_name, "") != 0) {
+ putc ('/', fp);
+ uri_quote (export_name, fp);
+ }
+ fprintf (fp, "\\?socket=");
uri_quote (unixsocket, fp);
}
putc ('\n', fp);
- /* $exportname is for backwards compatibility. */
- fprintf (fp, "exportname=\n");
+ /* Expose $exportname. */
+ fprintf (fp, "exportname=");
+ shell_quote (export_name, fp);
+ putc ('\n', fp);
/* Construct older $nbd "URL". Unfortunately guestfish and qemu take
* different syntax, so try to guess which one we need.
diff --git a/server/main.c b/server/main.c
index d815fe12..2f0aaf07 100644
--- a/server/main.c
+++ b/server/main.c
@@ -79,6 +79,7 @@ static bool is_config_key (const char *key, size_t len);
struct debug_flag *debug_flags; /* -D */
bool exit_with_parent; /* --exit-with-parent */
+const char *export_name; /* -e */
bool foreground; /* -f */
const char *ipaddr; /* -i */
enum log_to log_to = LOG_TO_DEFAULT; /* --log */
@@ -344,7 +345,7 @@ main (int argc, char *argv[])
break;
case 'e':
- /* Does nothing, ignored for compatibility with older nbdkit. */
+ export_name = optarg;
break;
case 'f':
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 1da2515a..897f8e94 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
# nbdkit
-# Copyright (C) 2019 Red Hat Inc.
+# Copyright (C) 2019-2020 Red Hat Inc.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -53,10 +53,6 @@ name1k=$name256$name256$name256$name256
name4k=$name1k$name1k$name1k$name1k
almost4k=${name4k%8$name16}
-# Test behavior of -e: accept 4k max, reject longer
-nbdkit -U - -e $name4k null --run true || fail=1
-nbdkit -U - -e a$name4k null --run true && fail=1
-
# Test that $exportname and $uri reflect the name
out=$(nbdkit -U - -e $name4k null --run 'echo $exportname')
if test "$name4k" != "$out"; then
@@ -110,6 +106,9 @@ assert h.get_size() == 0
EOF
'
+# TODO when we add .list_extents, we should test a plugin advertising
+# a name near 4k in length. nbdkit -e no longer does this
+if false; then
# The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0.
if ! qemu-nbd --help | grep -sq -- --list; then
echo "$0: skipping because qemu-nbd does not support the --list option"
@@ -118,5 +117,6 @@ fi
# Test response to NBD_OPT_LIST
nbdkit -U - -e $name4k null --run 'qemu-nbd --list -k $unixsocket' || fail=1
+fi
exit $fail
--
2.27.0
3 years, 10 months
[PATCH nbdkit] server: Pass the export name through filter .open calls.
by Richard W.M. Jones
To allow filters to modify the export name as it passes through the
layers this commit makes several changes:
The filter .open callback now takes an extra parameter, the export
name. This is always non-NULL (for oldstyle it is ""). This string
has a short lifetime and filters that need to hang on to it must take
a copy. The filter must pass the exportname parameter down to the
next layer (or potentially modify it).
The plugin .open callback is unchanged, but the binding to it in
server/plugins.c takes a copy of the exportname in the handle and this
is what nbdkit_export_name returns to plugins. Eventually we will
deprecate that function and the V3 .open callback in plugins will have
the extra parameter, but this is not implemented yet.
nbdkit_export_name can only be called from plugins, not filters.
Filters should use the .open(...exportname) parameter if they need the
export name.
Filter .reopen also takes the exportname. The only filter that uses
it (retry) must save the exportname from .open in order to pass it to
.reopen. This filter already does the same for the readonly flag so
this seems reasonable.
The handling of NBD_OPT_SET_META_CONTEXT interacted with the export
name (see commit 20b8509a9ccdab118ce7b7043be63bbad74f1e79). I have
attempted to keep previous behaviour in this change, but note that
there is no regression test for this. I added a debug message so we
can tell when this unusual case actually happens which should help
with diagnosis of problems.
All of the above is internal refactoring and should have no visible
external effect on server behaviour or how plugins are written.
Filters need to be updated as discussed above. All the in-tree
filters have been, and we assume there are no out of tree filters
because of the unstable filter API.
See also this thread:
https://www.redhat.com/archives/libguestfs/2020-July/thread.html#00087
---
include/nbdkit-common.h | 1 -
include/nbdkit-filter.h | 7 +--
include/nbdkit-plugin.h | 1 +
server/internal.h | 26 +++++++----
server/backend.c | 14 +++---
server/connections.c | 2 +
server/filters.c | 6 +--
server/plugins.c | 20 ++++++++-
server/protocol-handshake-newstyle.c | 65 ++++++++++++++++++----------
server/protocol-handshake-oldstyle.c | 2 +-
server/protocol-handshake.c | 5 ++-
server/public.c | 3 ++
filters/cow/cow.c | 5 ++-
filters/exitlast/exitlast.c | 4 +-
filters/ext2/ext2.c | 31 ++++++++++---
filters/gzip/gzip.c | 5 ++-
filters/limit/limit.c | 4 +-
filters/log/log.c | 5 ++-
filters/partition/partition.c | 5 ++-
filters/rate/rate.c | 5 ++-
filters/retry/retry.c | 15 +++++--
filters/tar/tar.c | 5 ++-
filters/truncate/truncate.c | 5 ++-
filters/xz/xz.c | 5 ++-
tests/test-layers-filter.c | 5 ++-
25 files changed, 171 insertions(+), 80 deletions(-)
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 47288050..671cd4a4 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -110,7 +110,6 @@ extern int nbdkit_stdio_safe (void);
extern int nbdkit_read_password (const char *value, char **password);
extern char *nbdkit_realpath (const char *path);
extern int nbdkit_nanosleep (unsigned sec, unsigned nsec);
-extern const char *nbdkit_export_name (void);
extern int nbdkit_peer_name (struct sockaddr *addr, socklen_t *addrlen);
extern void nbdkit_shutdown (void);
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 229a54b4..cec12db7 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -65,13 +65,14 @@ typedef int nbdkit_next_config_complete (nbdkit_backend *nxdata);
typedef int nbdkit_next_get_ready (nbdkit_backend *nxdata);
typedef int nbdkit_next_after_fork (nbdkit_backend *nxdata);
typedef int nbdkit_next_preconnect (nbdkit_backend *nxdata, int readonly);
-typedef int nbdkit_next_open (nbdkit_backend *nxdata, int readonly);
+typedef int nbdkit_next_open (nbdkit_backend *nxdata,
+ int readonly, const char *exportname);
struct nbdkit_next_ops {
/* Performs close + open on the underlying chain.
* Used by the retry filter.
*/
- int (*reopen) (nbdkit_backend *nxdata, int readonly);
+ int (*reopen) (nbdkit_backend *nxdata, int readonly, const char *exportname);
/* The rest of the next ops are the same as normal plugin operations. */
int64_t (*get_size) (nbdkit_backend *nxdata);
@@ -156,7 +157,7 @@ struct nbdkit_filter {
int readonly);
void * (*open) (nbdkit_next_open *next, nbdkit_backend *nxdata,
- int readonly);
+ int readonly, const char *exportname);
void (*close) (void *handle);
int (*prepare) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata,
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index cc261635..c6c1256c 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -142,6 +142,7 @@ struct nbdkit_plugin {
};
extern void nbdkit_set_error (int err);
+extern const char *nbdkit_export_name (void);
#define NBDKIT_REGISTER_PLUGIN(plugin) \
NBDKIT_CXX_LANG_C \
diff --git a/server/internal.h b/server/internal.h
index ebc63a52..4911a49b 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -246,8 +246,6 @@ struct connection {
struct handle *handles; /* One per plugin and filter. */
size_t nr_handles;
- char exportname[NBD_MAX_STRING + 1];
- uint32_t exportnamelen;
uint32_t cflags;
uint16_t eflags;
bool handshake_complete;
@@ -255,6 +253,9 @@ struct connection {
bool structured_replies;
bool meta_context_base_allocation;
+ char *exportname_from_set_meta_context;
+ char *exportname;
+
int sockin, sockout;
connection_recv_function recv;
connection_send_function send;
@@ -273,8 +274,9 @@ extern int connection_set_status (int value);
/* protocol-handshake.c */
extern int protocol_handshake (void);
-extern int protocol_common_open (uint64_t *exportsize, uint16_t *flags)
- __attribute__((__nonnull__ (1, 2)));
+extern int protocol_common_open (uint64_t *exportsize, uint16_t *flags,
+ const char *exportname)
+ __attribute__((__nonnull__ (1, 2, 3)));
/* protocol-handshake-oldstyle.c */
extern int protocol_handshake_oldstyle (void);
@@ -358,7 +360,7 @@ struct backend {
void (*get_ready) (struct backend *);
void (*after_fork) (struct backend *);
int (*preconnect) (struct backend *, int readonly);
- void *(*open) (struct backend *, int readonly);
+ void *(*open) (struct backend *, int readonly, const char *exportname);
int (*prepare) (struct backend *, void *handle, int readonly);
int (*finalize) (struct backend *, void *handle);
void (*close) (struct backend *, void *handle);
@@ -402,8 +404,13 @@ extern void backend_load (struct backend *b, const char *name,
extern void backend_unload (struct backend *b, void (*unload) (void))
__attribute__((__nonnull__ (1)));
-extern int backend_open (struct backend *b, int readonly)
- __attribute__((__nonnull__ (1)));
+/* exportname is only valid for this call and almost certainly will be
+ * freed on return of this function, so backends must save the
+ * exportname if they need to refer to it later.
+ */
+extern int backend_open (struct backend *b,
+ int readonly, const char *exportname)
+ __attribute__((__nonnull__ (1, 3)));
extern int backend_prepare (struct backend *b)
__attribute__((__nonnull__ (1)));
extern int backend_finalize (struct backend *b)
@@ -414,8 +421,9 @@ extern bool backend_valid_range (struct backend *b,
uint64_t offset, uint32_t count)
__attribute__((__nonnull__ (1)));
-extern int backend_reopen (struct backend *b, int readonly)
- __attribute__((__nonnull__ (1)));
+extern int backend_reopen (struct backend *b,
+ int readonly, const char *exportname)
+ __attribute__((__nonnull__ (1, 3)));
extern int64_t backend_get_size (struct backend *b)
__attribute__((__nonnull__ (1)));
extern int backend_can_write (struct backend *b)
diff --git a/server/backend.c b/server/backend.c
index cf87e35a..d39fdeaf 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -152,12 +152,13 @@ backend_unload (struct backend *b, void (*unload) (void))
}
int
-backend_open (struct backend *b, int readonly)
+backend_open (struct backend *b, int readonly, const char *exportname)
{
GET_CONN;
struct handle *h = get_handle (conn, b->i);
- controlpath_debug ("%s: open readonly=%d", b->name, readonly);
+ controlpath_debug ("%s: open readonly=%d exportname=\"%s\"",
+ b->name, readonly, exportname);
assert (h->handle == NULL);
assert ((h->state & HANDLE_OPEN) == 0);
@@ -168,7 +169,7 @@ backend_open (struct backend *b, int readonly)
/* Most filters will call next_open first, resulting in
* inner-to-outer ordering.
*/
- h->handle = b->open (b, readonly);
+ h->handle = b->open (b, readonly, exportname);
controlpath_debug ("%s: open returned handle %p", b->name, h->handle);
if (h->handle == NULL) {
@@ -268,14 +269,15 @@ backend_valid_range (struct backend *b, uint64_t offset, uint32_t count)
/* Wrappers for all callbacks in a filter's struct nbdkit_next_ops. */
int
-backend_reopen (struct backend *b, int readonly)
+backend_reopen (struct backend *b, int readonly, const char *exportname)
{
- controlpath_debug ("%s: reopen readonly=%d", b->name, readonly);
+ controlpath_debug ("%s: reopen readonly=%d exportname=\"%s\"",
+ b->name, readonly, exportname);
if (backend_finalize (b) == -1)
return -1;
backend_close (b);
- if (backend_open (b, readonly) == -1) {
+ if (backend_open (b, readonly, exportname) == -1) {
backend_close (b);
return -1;
}
diff --git a/server/connections.c b/server/connections.c
index cc552b90..69e4c0c0 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -356,6 +356,8 @@ free_connection (struct connection *conn)
pthread_mutex_destroy (&conn->write_lock);
pthread_mutex_destroy (&conn->status_lock);
+ free (conn->exportname_from_set_meta_context);
+ free (conn->exportname);
free (conn->handles);
free (conn);
threadlocal_set_conn (NULL);
diff --git a/server/filters.c b/server/filters.c
index 2d705e1e..7d268096 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -238,7 +238,7 @@ plugin_magic_config_key (struct backend *b)
}
static void *
-filter_open (struct backend *b, int readonly)
+filter_open (struct backend *b, int readonly, const char *exportname)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
void *handle;
@@ -247,8 +247,8 @@ filter_open (struct backend *b, int readonly)
* inner-to-outer ordering.
*/
if (f->filter.open)
- handle = f->filter.open (backend_open, b->next, readonly);
- else if (backend_open (b->next, readonly) == -1)
+ handle = f->filter.open (backend_open, b->next, readonly, exportname);
+ else if (backend_open (b->next, readonly, exportname) == -1)
handle = NULL;
else
handle = NBDKIT_HANDLE_NOT_NEEDED;
diff --git a/server/plugins.c b/server/plugins.c
index 285569bb..8449b1d9 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -278,12 +278,30 @@ plugin_preconnect (struct backend *b, int readonly)
}
static void *
-plugin_open (struct backend *b, int readonly)
+plugin_open (struct backend *b, int readonly, const char *exportname)
{
+ GET_CONN;
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
assert (p->plugin.open != NULL);
+ /* Save the exportname since the lifetime of the string passed in
+ * here is likely to be brief. In addition this provides a place
+ * for nbdkit_export_name to retrieve it if called from the plugin.
+ *
+ * In API V3 we propose to pass the exportname as an extra parameter
+ * to the (new) plugin.open and deprecate nbdkit_export_name for V3
+ * users. Even then we will still need to save it in the handle
+ * because of the lifetime issue.
+ */
+ if (conn->exportname == NULL) {
+ conn->exportname = strdup (exportname);
+ if (conn->exportname == NULL) {
+ nbdkit_error ("strdup: %m");
+ return NULL;
+ }
+ }
+
return p->plugin.open (readonly);
}
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index fa44f253..0ed3c7c7 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -200,11 +200,29 @@ conn_recv_full (void *buf, size_t len, const char *fmt, ...)
* in that function, and must not cause any wire traffic.
*/
static int
-finish_newstyle_options (uint64_t *exportsize)
+finish_newstyle_options (uint64_t *exportsize,
+ const char *exportname_in, uint32_t exportnamelen)
{
GET_CONN;
- if (protocol_common_open (exportsize, &conn->eflags) == -1)
+ /* Since the exportname string passed here comes directly out of the
+ * NBD protocol make a temporary copy of the exportname into a
+ * \0-terminated buffer.
+ */
+ CLEANUP_FREE char *exportname = strndup (exportname_in, exportnamelen);
+ if (exportname == NULL) {
+ nbdkit_error ("strndup: %m");
+ return -1;
+ }
+
+ if (conn->exportname_from_set_meta_context &&
+ strcmp (conn->exportname_from_set_meta_context, exportname) != 0) {
+ debug ("newstyle negotiation: NBD_OPT_SET_META_CONTEXT export name \"%s\" ≠ final client exportname \"%s\", so discarding the previous context",
+ conn->exportname_from_set_meta_context, exportname);
+ conn->meta_context_base_allocation = false;
+ }
+
+ if (protocol_common_open (exportsize, &conn->eflags, exportname) == -1)
return -1;
debug ("newstyle negotiation: flags: export 0x%x", conn->eflags);
@@ -238,22 +256,13 @@ check_string (uint32_t option, char *buf, uint32_t len, uint32_t maxlen,
*/
static int
check_export_name (uint32_t option, char *buf,
- uint32_t exportnamelen, uint32_t maxlen, bool save)
+ uint32_t exportnamelen, uint32_t maxlen)
{
GET_CONN;
if (check_string (option, buf, exportnamelen, maxlen, "export name") == -1)
return -1;
- assert (exportnamelen < sizeof conn->exportname);
- if (save) {
- if (exportnamelen != conn->exportnamelen ||
- memcmp (conn->exportname, buf, exportnamelen) != 0)
- conn->meta_context_base_allocation = false;
- memcpy (conn->exportname, buf, exportnamelen);
- conn->exportname[exportnamelen] = '\0';
- conn->exportnamelen = exportnamelen;
- }
debug ("newstyle negotiation: %s: client requested export '%.*s'",
name_of_nbd_opt (option), (int) exportnamelen, buf);
return 0;
@@ -329,13 +338,13 @@ negotiate_handshake_newstyle_options (void)
if (conn_recv_full (data, optlen,
"read: %s: %m", name_of_nbd_opt (option)) == -1)
return -1;
- if (check_export_name (option, data, optlen, optlen, true) == -1)
+ if (check_export_name (option, data, optlen, optlen) == -1)
return -1;
/* We have to finish the handshake by sending handshake_finish.
* On failure, we have to disconnect.
*/
- if (finish_newstyle_options (&exportsize) == -1)
+ if (finish_newstyle_options (&exportsize, data, optlen) == -1)
return -1;
memset (&handshake_finish, 0, sizeof handshake_finish);
@@ -468,7 +477,7 @@ negotiate_handshake_newstyle_options (void)
* or else we drop the support for that context.
*/
if (check_export_name (option, &data[4], exportnamelen,
- optlen - 6, true) == -1) {
+ optlen - 6) == -1) {
if (send_newstyle_option_reply (option, NBD_REP_ERR_INVALID)
== -1)
return -1;
@@ -483,7 +492,8 @@ negotiate_handshake_newstyle_options (void)
* client and let them try another NBD_OPT, rather than
* disconnecting.
*/
- if (finish_newstyle_options (&exportsize) == -1) {
+ if (finish_newstyle_options (&exportsize,
+ &data[4], exportnamelen) == -1) {
if (backend_finalize (top) == -1)
return -1;
backend_close (top);
@@ -601,17 +611,26 @@ negotiate_handshake_newstyle_options (void)
continue;
}
+ memcpy (&exportnamelen, &data[0], 4);
+ exportnamelen = be32toh (exportnamelen);
+ what = "validating export name";
+ if (check_export_name (option, &data[4], exportnamelen,
+ optlen - 8) == -1)
+ goto opt_meta_invalid_option_len;
+
/* Remember the export name: the NBD spec says that if the client
* later uses NBD_OPT_GO on a different export, then the context
* returned here is not usable.
*/
- memcpy (&exportnamelen, &data[0], 4);
- exportnamelen = be32toh (exportnamelen);
- what = "validating export name";
- if (check_export_name (option, &data[4], exportnamelen,
- optlen - 8,
- option == NBD_OPT_SET_META_CONTEXT) == -1)
- goto opt_meta_invalid_option_len;
+ if (option == NBD_OPT_SET_META_CONTEXT) {
+ conn->exportname_from_set_meta_context =
+ strndup (&data[4], exportnamelen);
+ if (conn->exportname_from_set_meta_context == NULL) {
+ nbdkit_error ("malloc: %m");
+ return -1;
+ }
+ }
+
opt_index = 4 + exportnamelen;
/* Read the number of queries. */
diff --git a/server/protocol-handshake-oldstyle.c b/server/protocol-handshake-oldstyle.c
index 4ad4720f..d4692362 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -56,7 +56,7 @@ protocol_handshake_oldstyle (void)
/* With oldstyle, our only option if .open or friends fail is to
* disconnect, as we cannot report the problem to the client.
*/
- if (protocol_common_open (&exportsize, &eflags) == -1)
+ if (protocol_common_open (&exportsize, &eflags, "") == -1)
return -1;
gflags = 0;
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 5e4b5e6e..80233713 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -72,14 +72,15 @@ protocol_handshake ()
* simply opening a TCP connection.
*/
int
-protocol_common_open (uint64_t *exportsize, uint16_t *flags)
+protocol_common_open (uint64_t *exportsize, uint16_t *flags,
+ const char *exportname)
{
GET_CONN;
int64_t size;
uint16_t eflags = NBD_FLAG_HAS_FLAGS;
int fl;
- if (backend_open (top, read_only) == -1)
+ if (backend_open (top, read_only, exportname) == -1)
return -1;
/* Prepare (for filters), called just after open. */
diff --git a/server/public.c b/server/public.c
index cef25426..c00a5cb6 100644
--- a/server/public.c
+++ b/server/public.c
@@ -654,6 +654,9 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec)
#endif
}
+/* This function will be deprecated for API V3 users. The preferred
+ * approach will be to get the exportname from .open().
+ */
const char *
nbdkit_export_name (void)
{
diff --git a/filters/cow/cow.c b/filters/cow/cow.c
index fae36174..0faf2726 100644
--- a/filters/cow/cow.c
+++ b/filters/cow/cow.c
@@ -93,10 +93,11 @@ cow_config (nbdkit_next_config *next, void *nxdata,
"cow-on-cache=<BOOL> Set to true to treat client cache requests as writes.\n"
static void *
-cow_open (nbdkit_next_open *next, void *nxdata, int readonly)
+cow_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
/* Always pass readonly=1 to the underlying plugin. */
- if (next (nxdata, 1) == -1)
+ if (next (nxdata, 1, exportname) == -1)
return NULL;
return NBDKIT_HANDLE_NOT_NEEDED;
diff --git a/filters/exitlast/exitlast.c b/filters/exitlast/exitlast.c
index 51efda03..378cda75 100644
--- a/filters/exitlast/exitlast.c
+++ b/filters/exitlast/exitlast.c
@@ -51,9 +51,9 @@ static _Atomic unsigned connections;
static void *
exitlast_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
- int readonly)
+ int readonly, const char *exportname)
{
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
connections++;
diff --git a/filters/ext2/ext2.c b/filters/ext2/ext2.c
index 92c8601a..72b7ac9f 100644
--- a/filters/ext2/ext2.c
+++ b/filters/ext2/ext2.c
@@ -112,6 +112,7 @@ ext2_config_complete (nbdkit_next_config_complete *next, void *nxdata)
/* The per-connection handle. */
struct handle {
+ char *exportname; /* Client export name. */
ext2_filsys fs; /* Filesystem handle. */
ext2_ino_t ino; /* Inode of open file. */
ext2_file_t file; /* File handle. */
@@ -120,20 +121,37 @@ struct handle {
/* Create the per-connection handle. */
static void *
-ext2_open (nbdkit_next_open *next, void *nxdata, int readonly)
+ext2_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h;
- /* Request write access to the underlying plugin, for journal replay. */
- if (next (nxdata, 0) == -1)
- return NULL;
-
h = calloc (1, sizeof *h);
if (h == NULL) {
nbdkit_error ("calloc: %m");
return NULL;
}
+ /* Save the client exportname in the handle. */
+ h->exportname = strdup (exportname);
+ if (h->exportname == NULL) {
+ nbdkit_error ("strdup: %m");
+ free (h);
+ return NULL;
+ }
+
+ /* If file == NULL (ie. using exportname) then don't
+ * pass the client exportname to the lower layers.
+ */
+ exportname = file ? exportname : "";
+
+ /* Request write access to the underlying plugin, for journal replay. */
+ if (next (nxdata, 0, exportname) == -1) {
+ free (h->exportname);
+ free (h);
+ return NULL;
+ }
+
return h;
}
@@ -148,7 +166,7 @@ ext2_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle,
struct ext2_inode inode;
int64_t r;
CLEANUP_FREE char *name = NULL;
- const char *fname = file ?: nbdkit_export_name ();
+ const char *fname = file ?: h->exportname;
CLEANUP_FREE char *absname = NULL;
fs_flags = 0;
@@ -242,6 +260,7 @@ ext2_close (void *handle)
ext2fs_file_close (h->file);
ext2fs_close (h->fs);
}
+ free (h->exportname);
free (h);
}
diff --git a/filters/gzip/gzip.c b/filters/gzip/gzip.c
index 31fde3bb..8323b882 100644
--- a/filters/gzip/gzip.c
+++ b/filters/gzip/gzip.c
@@ -74,10 +74,11 @@ gzip_thread_model (void)
}
static void *
-gzip_open (nbdkit_next_open *next, nbdkit_backend *nxdata, int readonly)
+gzip_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+ int readonly, const char *exportname)
{
/* Always pass readonly=1 to the underlying plugin. */
- if (next (nxdata, 1) == -1)
+ if (next (nxdata, 1, exportname) == -1)
return NULL;
return NBDKIT_HANDLE_NOT_NEEDED;
diff --git a/filters/limit/limit.c b/filters/limit/limit.c
index 563481fa..7c4477eb 100644
--- a/filters/limit/limit.c
+++ b/filters/limit/limit.c
@@ -91,9 +91,9 @@ limit_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata,
static void *
limit_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
- int readonly)
+ int readonly, const char *exportname)
{
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
diff --git a/filters/log/log.c b/filters/log/log.c
index f47a6a93..8b146f1c 100644
--- a/filters/log/log.c
+++ b/filters/log/log.c
@@ -227,11 +227,12 @@ output_return (struct handle *h, const char *act, uint64_t id, int r, int *err)
/* Open a connection. */
static void *
-log_open (nbdkit_next_open *next, void *nxdata, int readonly)
+log_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = malloc (sizeof *h);
diff --git a/filters/partition/partition.c b/filters/partition/partition.c
index 41b85312..c348664b 100644
--- a/filters/partition/partition.c
+++ b/filters/partition/partition.c
@@ -87,11 +87,12 @@ struct handle {
/* Open a connection. */
static void *
-partition_open (nbdkit_next_open *next, void *nxdata, int readonly)
+partition_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = malloc (sizeof *h);
diff --git a/filters/rate/rate.c b/filters/rate/rate.c
index 95b83024..6e99fcc7 100644
--- a/filters/rate/rate.c
+++ b/filters/rate/rate.c
@@ -162,11 +162,12 @@ rate_get_ready (nbdkit_next_get_ready *next, void *nxdata)
/* Create the per-connection handle. */
static void *
-rate_open (nbdkit_next_open *next, void *nxdata, int readonly)
+rate_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct rate_handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = malloc (sizeof *h);
diff --git a/filters/retry/retry.c b/filters/retry/retry.c
index 8037f383..be334c39 100644
--- a/filters/retry/retry.c
+++ b/filters/retry/retry.c
@@ -106,16 +106,18 @@ retry_config (nbdkit_next_config *next, void *nxdata,
struct retry_handle {
int readonly; /* Save original readonly setting. */
+ const char *exportname; /* Client exportname. */
unsigned reopens;
bool open;
};
static void *
-retry_open (nbdkit_next_open *next, void *nxdata, int readonly)
+retry_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct retry_handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = malloc (sizeof *h);
@@ -125,6 +127,12 @@ retry_open (nbdkit_next_open *next, void *nxdata, int readonly)
}
h->readonly = readonly;
+ h->exportname = strdup (exportname);
+ if (h->exportname == NULL) {
+ nbdkit_error ("strdup: %m");
+ free (h);
+ return NULL;
+ }
h->reopens = 0;
h->open = true;
@@ -198,7 +206,8 @@ do_retry (struct retry_handle *h,
/* Reopen the connection. */
h->reopens++;
- if (next_ops->reopen (nxdata, h->readonly || force_readonly) == -1) {
+ if (next_ops->reopen (nxdata,
+ h->readonly || force_readonly, h->exportname) == -1) {
/* If the reopen fails we treat it the same way as a command
* failing.
*/
diff --git a/filters/tar/tar.c b/filters/tar/tar.c
index a2b26935..c04f09dd 100644
--- a/filters/tar/tar.c
+++ b/filters/tar/tar.c
@@ -112,11 +112,12 @@ struct handle {
};
static void *
-tar_open (nbdkit_next_open *next, nbdkit_backend *nxdata, int readonly)
+tar_open (nbdkit_next_open *next, nbdkit_backend *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = calloc (1, sizeof *h);
diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c
index 485f7f45..ee384871 100644
--- a/filters/truncate/truncate.c
+++ b/filters/truncate/truncate.c
@@ -124,11 +124,12 @@ struct handle {
/* Open a connection. */
static void *
-truncate_open (nbdkit_next_open *next, void *nxdata, int readonly)
+truncate_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
h = malloc (sizeof *h); /* h is populated during .prepare */
diff --git a/filters/xz/xz.c b/filters/xz/xz.c
index 365958da..9154dbeb 100644
--- a/filters/xz/xz.c
+++ b/filters/xz/xz.c
@@ -89,12 +89,13 @@ struct xz_handle {
/* Create the per-connection handle. */
static void *
-xz_open (nbdkit_next_open *next, void *nxdata, int readonly)
+xz_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct xz_handle *h;
/* Always pass readonly=1 to the underlying plugin. */
- if (next (nxdata, 1) == -1)
+ if (next (nxdata, 1, exportname) == -1)
return NULL;
h = malloc (sizeof *h);
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index b6ca05bd..29d8b669 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -107,7 +107,8 @@ test_layers_filter_preconnect (nbdkit_next_preconnect *next,
}
static void *
-test_layers_filter_open (nbdkit_next_open *next, void *nxdata, int readonly)
+test_layers_filter_open (nbdkit_next_open *next, void *nxdata,
+ int readonly, const char *exportname)
{
struct handle *h = malloc (sizeof *h);
@@ -118,7 +119,7 @@ test_layers_filter_open (nbdkit_next_open *next, void *nxdata, int readonly)
h->nxdata = nxdata;
h->next_ops = NULL;
- if (next (nxdata, readonly) == -1)
+ if (next (nxdata, readonly, exportname) == -1)
return NULL;
/* Debug after recursing, to show opposite order from .close */
--
2.27.0
3 years, 10 months
[PATCH nbdkit] server: Deprecate the -e/--exportname parameter.
by Richard W.M. Jones
This parameter provided a name for the "default export" -- ie. the one
and only export returned to the client by NBD_OPT_LIST. But nbdkit
traditionally didn't care what export name the client requested.
Since 1.16 plugins have been able to serve different content per
export name (and return errors for unknown exports), but the -e option
didn't reflect that and only created confusion.
What we actually have to do is ask the plugin what exports it provides
and return that list to the client. This is for future work.
This commit deprecates and hides the parameter. If used the option
now does nothing at all.
There are some visible but very minor changes resulting from this
commit:
(1) NBD_OPT_LIST advertises a single export called "" (instead of the
-e parameter). We intend to replace this implementation soon with a
correct one.
(2) The --run option no longer sets $exportname (to -e) nor puts the
export name into the $uri. However this was always the wrong
thing to do since export names are per connection, not per server.
Existing --run scripts will see $exportname expand to "" which is
most likely what they saw before and overwhelmingly more likely to
be correct than if the -e option had been used.
(3) The -e option had the side-effect of forcing the newstyle
protocol, so weirdly this worked and made the server use newstyle:
nbdkit -o -e '' ...
but this would fail with an error at start-up:
nbdkit -e '' -o ...
I thought it best to remove this odd behaviour completely.
(4) I have temporarily disabled tests/test-long-name.sh. This is
still a valid test, but we will have to rewrite it to use
(probably) sh or eval plugins once we have an implementation of
list_exports.
---
docs/nbdkit-captive.pod | 12 ++----------
docs/nbdkit-protocol.pod | 15 +++++++--------
docs/nbdkit.pod | 13 -------------
docs/synopsis.txt | 3 +--
server/internal.h | 1 -
server/captive.c | 18 +-----------------
server/main.c | 21 +--------------------
server/protocol-handshake-newstyle.c | 12 +++++++++---
tests/test-data-7E.sh | 2 +-
tests/test-data-raw.sh | 3 +--
tests/test-long-name.sh | 4 ++++
11 files changed, 27 insertions(+), 77 deletions(-)
diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index 09628367..66c5c81c 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -47,14 +47,12 @@ The following shell variables are available in the I<--run> argument:
=item C<$uri>
A URI that refers to the nbdkit port or socket in the preferred form
-documented by the NBD project. If nbdkit was started with the B<-e>
-option to set the preferred export name, this is included in the URI.
+documented by the NBD project.
=item C<$nbd>
An older URL that refers to the nbdkit port or socket in a manner
-specific to certain tools. This form does not include an export name,
-even if B<-e> was used.
+specific to certain tools.
Note there is some magic here, since qemu and guestfish URLs have a
different format, so nbdkit tries to guess which you are running. If
@@ -68,12 +66,6 @@ If E<ne> "", the port number that nbdkit is listening on.
If E<ne> "", the Unix domain socket that nbdkit is listening on.
-=item C<$exportname>
-
-The default export name (which may be "") that nbdkit will advertise
-in response to NBD_OPT_LIST. This comes from the B<-e>
-(B<--exportname>) command line option.
-
=back
I<--run> implies I<--foreground>. It is not possible, and probably
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index d90993fb..172d1326 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -5,7 +5,7 @@ nbdkit - which parts of the NBD protocol nbdkit supports
=head1 SYNOPSIS
nbdkit [-n|--newstyle] [--mask-handshake MASK] [--no-sr] [-o|--oldstyle]
- [-e|--exportname EXPORTNAME] [...]
+ [...]
=head1 DESCRIPTION
@@ -29,9 +29,6 @@ can support (in some cases, this can be limited by what callbacks the
plugin handles), even if the client does not negotiate to use all
advertised features.
-Use the I<-e> or I<--exportname> flag to set the optional exportname
-for the newstyle protocol.
-
Nbdkit also includes some options that are useful mainly when
performing integration tests, for proving whether clients have sane
fallback behavior when dealing various older servers permitted by the
@@ -101,11 +98,13 @@ Supported in nbdkit E<ge> 1.1.12, and the default in nbdkit E<ge> 1.3.
=item export names
-Supported in nbdkit E<ge> 1.1.12.
+Partially supported in nbdkit E<ge> 1.1.12. Support for plugins to
+read the client export name added in nbdkit E<ge> 1.15.2.
-nbdkit can advertise an export name, but ignores any export name sent
-by the client. nbdkit does I<not> support serving different data on
-different export names.
+Versions of nbdkit before 1.16 could advertize a single export name to
+clients, specified through the now deprecated I<-e> option. In nbdkit
+1.15.2, plugins could read the client requested export name using
+C<nbdkit_export_name()> and serve different content.
=item C<NBD_FLAG_NO_ZEROES>
diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod
index a6cf988c..b204709b 100644
--- a/docs/nbdkit.pod
+++ b/docs/nbdkit.pod
@@ -214,19 +214,6 @@ An alternative to this is L<nbdkit-captive(1)/CAPTIVE NBDKIT>.
This option implies I<--foreground>.
-=item B<-e> EXPORTNAME
-
-=item B<--export> EXPORTNAME
-
-=item B<--export-name> EXPORTNAME
-
-=item B<--exportname> EXPORTNAME
-
-Set the exportname.
-
-If not set, exportname C<""> (empty string) is used. Exportnames are
-not allowed with the oldstyle protocol.
-
=item B<-f>
=item B<--foreground>
diff --git a/docs/synopsis.txt b/docs/synopsis.txt
index 07b9dcff..caf49922 100644
--- a/docs/synopsis.txt
+++ b/docs/synopsis.txt
@@ -1,5 +1,4 @@
-nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N]
- [-e|--exportname EXPORTNAME] [--exit-with-parent]
+nbdkit [-D|--debug PLUGIN|FILTER|nbdkit.FLAG=N] [--exit-with-parent]
[--filter FILTER ...] [-f|--foreground]
[-g|--group GROUP] [-i|--ipaddr IPADDR]
[--log stderr|syslog|null]
diff --git a/server/internal.h b/server/internal.h
index 68c53366..ebc63a52 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -108,7 +108,6 @@ enum log_to {
};
extern struct debug_flag *debug_flags;
-extern const char *exportname;
extern bool foreground;
extern const char *ipaddr;
extern enum log_to log_to;
diff --git a/server/captive.c b/server/captive.c
index f8107604..aa717870 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -61,8 +61,6 @@ run_command (void)
if (!run)
return;
- assert (exportname != NULL);
-
fp = open_memstream (&cmd, &len);
if (fp == NULL) {
perror ("open_memstream");
@@ -74,27 +72,13 @@ run_command (void)
if (port) {
fprintf (fp, "nbd://localhost:");
shell_quote (port, fp);
- if (strcmp (exportname, "") != 0) {
- putc ('/', fp);
- uri_quote (exportname, fp);
- }
}
else if (unixsocket) {
- fprintf (fp, "nbd+unix://");
- if (strcmp (exportname, "") != 0) {
- putc ('/', fp);
- uri_quote (exportname, fp);
- }
- fprintf (fp, "\\?socket=");
+ fprintf (fp, "nbd+unix://\\?socket=");
uri_quote (unixsocket, fp);
}
putc ('\n', fp);
- /* Expose $exportname. */
- fprintf (fp, "exportname=");
- shell_quote (exportname, fp);
- putc ('\n', fp);
-
/* Construct older $nbd "URL". Unfortunately guestfish and qemu take
* different syntax, so try to guess which one we need.
*/
diff --git a/server/main.c b/server/main.c
index c432f5bd..d815fe12 100644
--- a/server/main.c
+++ b/server/main.c
@@ -79,7 +79,6 @@ static bool is_config_key (const char *key, size_t len);
struct debug_flag *debug_flags; /* -D */
bool exit_with_parent; /* --exit-with-parent */
-const char *exportname; /* -e */
bool foreground; /* -f */
const char *ipaddr; /* -i */
enum log_to log_to = LOG_TO_DEFAULT; /* --log */
@@ -345,13 +344,7 @@ main (int argc, char *argv[])
break;
case 'e':
- exportname = optarg;
- if (strnlen (exportname, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
- nbdkit_error ("export name too long");
- exit (EXIT_FAILURE);
- }
- /* TODO: Check that name is valid UTF-8? */
- newstyle = true;
+ /* Does nothing, ignored for compatibility with older nbdkit. */
break;
case 'f':
@@ -487,18 +480,6 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- /* Oldstyle protocol + exportname not allowed. */
- if (!newstyle && exportname != NULL) {
- fprintf (stderr,
- "%s: cannot use oldstyle protocol (-o) and exportname (-e)\n",
- program_name);
- exit (EXIT_FAILURE);
- }
-
- /* If exportname was not set on the command line, use "". */
- if (exportname == NULL)
- exportname = "";
-
/* --tls=require and oldstyle won't work. */
if (tls == 2 && !newstyle) {
fprintf (stderr,
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index 192d00f3..fa44f253 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -75,12 +75,15 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply)
return 0;
}
+/* Reply to NBD_OPT_LIST with a single empty export name.
+ * TODO: Ask the plugin for the list of exports.
+ */
static int
send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply)
{
GET_CONN;
struct nbd_fixed_new_option_reply fixed_new_option_reply;
- size_t name_len = strlen (exportname);
+ const size_t name_len = 0; /* length of export name */
uint32_t len;
fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
@@ -100,11 +103,14 @@ send_newstyle_option_reply_exportname (uint32_t option, uint32_t reply)
name_of_nbd_opt (option), "sending length");
return -1;
}
+#if 0
+ /* If we were sending a non-"" export name, this is what we'd use. */
if (conn->send (exportname, name_len, 0) == -1) {
nbdkit_error ("write: %s: %s: %m",
name_of_nbd_opt (option), "sending export name");
return -1;
}
+#endif
return 0;
}
@@ -364,8 +370,8 @@ negotiate_handshake_newstyle_options (void)
}
/* Send back the exportname. */
- debug ("newstyle negotiation: %s: advertising export '%s'",
- name_of_nbd_opt (option), exportname);
+ debug ("newstyle negotiation: %s: advertising export \"\"",
+ name_of_nbd_opt (option));
if (send_newstyle_option_reply_exportname (option, NBD_REP_SERVER) == -1)
return -1;
diff --git a/tests/test-data-7E.sh b/tests/test-data-7E.sh
index e06d3c7e..c4fa86b1 100755
--- a/tests/test-data-7E.sh
+++ b/tests/test-data-7E.sh
@@ -45,7 +45,7 @@ rm -f $files
cleanup_fn rm -f $files
# Run nbdkit.
-start_nbdkit -P data-7E.pid -U $sock --export= \
+start_nbdkit -P data-7E.pid -U $sock \
--filter=partition \
data size=7E partition=1 \
data="
diff --git a/tests/test-data-raw.sh b/tests/test-data-raw.sh
index 36ee8028..446e2093 100755
--- a/tests/test-data-raw.sh
+++ b/tests/test-data-raw.sh
@@ -44,8 +44,7 @@ rm -f $files
cleanup_fn rm -f $files
# Run nbdkit.
-start_nbdkit -P data-raw.pid -U $sock --export "" \
- data raw=123 size=512
+start_nbdkit -P data-raw.pid -U $sock data raw=123 size=512
nbdsh --connect "nbd+unix://?socket=$sock" \
-c '
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 6a512603..f70f492d 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -38,6 +38,10 @@ fail=0
# Test handling of NBD maximum string length of 4k.
+echo "$0: This test needs to be rewritten to use alternate method to"
+echo "send back the export name to the client."
+exit 77
+
requires qemu-io --version
requires qemu-nbd --version
requires nbdsh -c 'exit(not h.supports_uri())'
--
2.27.0
3 years, 10 months
[PATCH nbdkit] PROPOSED: server: Implement list_exports.
by Richard W.M. Jones
See also:
https://www.redhat.com/archives/libguestfs/2020-July/msg00090.html
---
docs/nbdkit-plugin.pod | 49 ++++++++++++++++++++++++++++++++++++----
docs/nbdkit-protocol.pod | 7 ++++--
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index f8e9962a..6553d186 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -152,6 +152,9 @@ the plugin:
│ preconnect │ │
└──────┬─────┘ │
┌──────┴─────┐ │
+ │list_exports│ │
+ └──────┬─────┘ │
+ ┌──────┴─────┐ │
│ open │ │
└──────┬─────┘ │
┌──────┴─────┐ NBD option │
@@ -160,10 +163,10 @@ the plugin:
┌──────┴─────┐ ┌──────┴─────┐ client #2
│ get_size │ │ preconnect │
└──────┬─────┘ └──────┬─────┘
- ┌──────┴─────┐ data ┌──────┴─────┐
- │ pread │ serving │ open │
- └──────┬─────┘↺ └──────┬─────┘
- ┌──────┴─────┐ ...
+ ┌──────┴─────┐ data
+ │ pread │ serving
+ └──────┬─────┘↺ ...
+ ┌──────┴─────┐
│ pwrite │
└──────┬─────┘↺ ┌──────┴─────┐
┌──────┴─────┐ │ close │
@@ -236,6 +239,12 @@ L<getpid(2)>.
Called when a TCP connection has been made to the server. This
happens early, before NBD or TLS negotiation.
+=item C<.list_exports>
+
+Early in option negotiation the client may try to list the exports
+served by the plugin, and plugins can optionally implement this
+callback to answer the client. See L</EXPORT NAME> below.
+
=item C<.open>
A new client has connected and finished the NBD handshake. TLS
@@ -652,6 +661,38 @@ Returning C<0> will allow the connection to continue. If there is an
error or you want to deny the connection, call C<nbdkit_error> with an
error message and return C<-1>.
+=head2 C<.list_exports>
+
+ int list_exports (int readonly, struct nbdkit_exports *exports);
+
+This optional callback is called if the client tries to list the
+exports served by the plugin (using C<NBD_OPT_LIST>). If the plugin
+does not supply this callback then a single export called C<""> is
+returned. The NBD protocol defines C<""> as the default export, so
+this is suitable for plugins which ignore the export name and always
+serve the same content. See also L</EXPORT NAME> below.
+
+The C<exports> parameter is an opaque object for collecting the list
+of exports. Call C<nbdkit_add_export> to add a single export to the
+list:
+
+ int nbdkit_add_export (struct nbdkit_export *exports,
+ const char *name, const char *description);
+
+The C<name> must be a non-NULL, UTF-8 string no longer than 4096
+bytes. C<description> is an optional description of the export which
+some clients can display but which is otherwise unused (if you don't
+want a description, you can pass this parameter as C<NULL>). The
+string(s) are copied into the exports list so you may free them
+immediately after calling this function.
+
+The C<readonly> flag informs the plugin that the server was started
+with the I<-r> flag on the command line.
+
+Returning C<0> will send the list of exports back to the client. If
+there is an error, C<.list_exports> should call C<nbdkit_error> with
+an error message and return C<-1>.
+
=head2 C<.open>
void *open (int readonly);
diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index e0e4042b..39b9481a 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -99,12 +99,15 @@ Supported in nbdkit E<ge> 1.1.12, and the default in nbdkit E<ge> 1.3.
=item export names
Partially supported in nbdkit E<ge> 1.1.12. Support for plugins to
-read the client export name added in nbdkit E<ge> 1.15.2.
+read the client export name added in nbdkit E<ge> 1.15.2. Support for
+C<NBD_OPT_LIST> was added in nbdkit E<ge> 1.21.20.
Versions of nbdkit before 1.16 could advertise a single export name to
clients, specified through the now deprecated I<-e> option. In nbdkit
1.15.2, plugins could read the client requested export name using
-C<nbdkit_export_name()> and serve different content.
+C<nbdkit_export_name()> and serve different content. In nbdkit
+1.21.20, plugins could implement C<.list_exports> to answer
+C<NBD_OPT_LIST> queries.
=item C<NBD_FLAG_NO_ZEROES>
--
2.27.0
3 years, 10 months
回复： 回复： bug report
by 谢威
Hi,Richard:
I use /bin/virt-copy-in failed , virt-copy-in have no diff with other nomarl machine(work well ) ,
/bin/virt-copy-in: symbol lookup error: /lib64/libguestfs.so.0: undefined symbol: json_string_length
here are the message , Thanks for you help .
# uname -a
Linux sh-op-offline-vmhost01.sh.sftcwl.com 4.19.8-1.el7.elrepo.x86_64 #1 SMP Sat Dec 8 10:07:47 EST 2018 x86_64 x86_64 x86_64 GNU/Linux
# cat /etc/redhat-release
CentOS Linux release 7.2.1511 (Core)
# /bin/virt-copy-in --version
libguestfs: trace: set_verbose true
libguestfs: trace: set_verbose = 0
libguestfs: create: flags = 0, handle = 0x56457b6e0700, program = virt-copy-in
virt-copy-in 1.40.2rhel=7,release=5.el7_7.3,libvirt
libguestfs: trace: close
libguestfs: closing guestfs handle 0x56457b6e0700 (state 0)
# virt-copy-in /etc/resolv.conf -a 10.188.188.34.sys.img /etc/
libguestfs: trace: set_verbose true
libguestfs: trace: set_verbose = 0
libguestfs: create: flags = 0, handle = 0x55a0a79a9700, program = virt-copy-in
libguestfs: trace: set_pgroup true
libguestfs: trace: set_pgroup = 0
libguestfs: trace: add_drive "10.188.188.34.sys.img"
libguestfs: trace: add_drive = 0
libguestfs: trace: is_config
libguestfs: trace: is_config = 1
libguestfs: trace: launch
libguestfs: trace: max_disks
libguestfs: trace: max_disks = 255
libguestfs: trace: get_tmpdir
libguestfs: trace: get_tmpdir = "/tmp"
libguestfs: trace: version
libguestfs: trace: version = <struct guestfs_version = major: 1, minor: 40, release: 2, extra: rhel=7,release=5.el7_7.3,libvirt, >
libguestfs: trace: get_backend
libguestfs: trace: get_backend = "libvirt"
libguestfs: launch: program=virt-copy-in
libguestfs: launch: version=1.40.2rhel=7,release=5.el7_7.3,libvirt
libguestfs: launch: backend registered: unix
libguestfs: launch: backend registered: uml
libguestfs: launch: backend registered: libvirt
libguestfs: launch: backend registered: direct
libguestfs: launch: backend=libvirt
libguestfs: launch: tmpdir=/tmp/libguestfs8F29Zb
libguestfs: launch: umask=0022
libguestfs: launch: euid=0
libguestfs: libvirt version = 4005000 (4.5.0)
libguestfs: guest random name = guestfs-z60wka9s9sjgb52d
libguestfs: connect to libvirt
libguestfs: opening libvirt handle: URI = qemu:///system, auth = default+wrapper, flags = 0
libguestfs: successfully opened libvirt handle: conn = 0x55a0a79ab150
libguestfs: qemu version (reported by libvirt) = 1005003 (1.5.3)
libguestfs: get libvirt capabilities
libguestfs: parsing capabilities XML
libguestfs: trace: get_backend_setting "force_tcg"
libguestfs: trace: get_backend_setting = NULL (error)
libguestfs: trace: get_backend_setting "internal_libvirt_label"
libguestfs: trace: get_backend_setting = NULL (error)
libguestfs: trace: get_backend_setting "internal_libvirt_imagelabel"
libguestfs: trace: get_backend_setting = NULL (error)
libguestfs: trace: get_backend_setting "internal_libvirt_norelabel_disks"
libguestfs: trace: get_backend_setting = NULL (error)
libguestfs: build appliance
libguestfs: trace: get_cachedir
libguestfs: trace: get_cachedir = "/var/tmp"
libguestfs: begin building supermin appliance
libguestfs: run supermin
libguestfs: command: run: /usr/bin/supermin5
libguestfs: command: run: \ --build
libguestfs: command: run: \ --verbose
libguestfs: command: run: \ --if-newer
libguestfs: command: run: \ --lock /var/tmp/.guestfs-0/lock
libguestfs: command: run: \ --copy-kernel
libguestfs: command: run: \ -f ext2
libguestfs: command: run: \ --host-cpu x86_64
libguestfs: command: run: \ /usr/lib64/guestfs/supermin.d
libguestfs: command: run: \ -o /var/tmp/.guestfs-0/appliance.d
supermin: version: 5.1.19
supermin: rpm: detected RPM version 4.11
supermin: package handler: fedora/rpm
supermin: acquiring lock on /var/tmp/.guestfs-0/lock
supermin: if-newer: output does not need rebuilding
libguestfs: finished building supermin appliance
libguestfs: trace: disk_create "/tmp/libguestfs8F29Zb/overlay1.qcow2" "qcow2" -1 "backingfile:/var/tmp/.guestfs-0/appliance.d/root" "backingformat:raw"
libguestfs: command: run: qemu-img
libguestfs: command: run: \ create
libguestfs: command: run: \ -f qcow2
libguestfs: command: run: \ -o backing_file=/var/tmp/.guestfs-0/appliance.d/root,backing_fmt=raw
libguestfs: command: run: \ /tmp/libguestfs8F29Zb/overlay1.qcow2
Formatting '/tmp/libguestfs8F29Zb/overlay1.qcow2', fmt=qcow2 size=4294967296 backing_file='/var/tmp/.guestfs-0/appliance.d/root' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off
libguestfs: trace: disk_create = 0
libguestfs: trace: get_sockdir
libguestfs: trace: get_sockdir = "/tmp"
libguestfs: set_socket_create_context: getcon failed: (none): Invalid argument [you can ignore this message if you are not using SELinux + sVirt]
libguestfs: clear_socket_create_context: setsockcreatecon failed: NULL: Invalid argument [you can ignore this message if you are not using SELinux + sVirt]
libguestfs: create libvirt XML
libguestfs: trace: disk_format "10.188.188.34.sys.img"
libguestfs: command: run: qemu-img --help | grep -sqE -- '\binfo\b.*-U\b'
libguestfs: command: run: qemu-img
libguestfs: command: run: \ info
libguestfs: command: run: \ --output json
libguestfs: command: run: \ ./10.188.188.34.sys.img
libguestfs: parse_json: qemu-img info JSON output:\n{\n "virtual-size": 53687091200, \n "filename": "./10.188.188.34.sys.img", \n "cluster-size": 65536, \n "format": "qcow2", \n "actual-size": 7686270976, \n "format-specific": {\n "type": "qcow2", \n "data": {\n "compat": "1.1", \n "lazy-refcounts": true\n }\n }, \n "dirty-flag": false\n}\n\n
/bin/virt-copy-in: symbol lookup error: /lib64/libguestfs.so.0: undefined symbol: json_string_length
3 years, 10 months
Re: [Libguestfs] [ovirt-devel] [ARM64] Possiblity to support oVirt on ARM64
by Nir Soffer
On Sun, Jul 19, 2020 at 5:04 PM Zhenyu Zheng <zhengzhenyulixi(a)gmail.com> wrote:
>
> Hi oVirt,
>
> We are currently trying to make oVirt work on ARM64 platform, since I'm quite new to oVirt community, I'm wondering what is the current status about ARM64 support in the oVirt upstream, as I saw the oVirt Wikipedia page mentioned there is an ongoing efforts to support ARM platform. We have a small team here and we are willing to also help to make this work.
Hi Zhenyu,
I think this is a great idea, both supporting more hardware, and
enlarging the oVirt
community.
Regarding hardware support we depend mostly on libvirt and qemu, and I
don't know
that is the status. Adding relevant lists and people.
I don't know about any effort on oVirt side, but last week I added
arm builds for
ovirt-imageio and it works:
https://copr.fedorainfracloud.org/coprs/nsoffer/ovirt-imageio-preview/bui...
We have many dependendencies, but oVirt itself is mostly python and java, with
tiny bits in C or using ctypes, so it should not be too hard.
I think the first thing is getting some hardware for testing. Do you
have such hardware,
or have some contacts that can help to get hardware contribution for this?
Nir
3 years, 10 months
[nbdkit PATCH] filters: Improve docs on .prepare prerequisites
by Eric Blake
Since .prepare is called before client negotiation has completed,
filters have an additional burden to ensure prerequisite functions are
called in order to avoid triggering assertions in backend.c.
See also: https://bugzilla.redhat.com/show_bug.cgi?id=1855330,
https://www.redhat.com/archives/libguestfs/2020-July/msg00041.html
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/nbdkit-filter.pod | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index acac3e50..3d201309 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -383,14 +383,20 @@ connection (C<.finalize>).
For example if you need to scan the underlying disk to check for a
partition table, you could do it in your C<.prepare> method (calling
-the plugin's C<.pread> method via C<next_ops>). Or if you need to
-cleanly update superblock data in the image on close you can do it in
-your C<.finalize> method (calling the plugin's C<.pwrite> method).
-Doing these things in the filter's C<.open> or C<.close> method is not
-possible.
+the plugin's C<.get_size> and C<.pread> methods via C<next_ops>). Or
+if you need to cleanly update superblock data in the image on close
+you can do it in your C<.finalize> method (calling the plugin's
+C<.pwrite> method). Doing these things in the filter's C<.open> or
+C<.close> method is not possible.
For C<.prepare>, the value of C<readonly> is the same as was passed to
-C<.open>, declaring how this filter will be used.
+C<.open>, declaring how this filter will be used. When calling plugin
+functions during C<.prepare>, the filter must ensure that prerequisite
+functions have succeeded (for example, calling C<.pwrite>) requires
+checking both C<.get_size> and C<.can_write>); while these
+prerequisites are automatically handled in most other cases thanks to
+client negotiation, the timing of C<.prepare> comes before client
+negotiation has completed.
There is no C<next_ops-E<gt>prepare> or C<next_ops-E<gt>finalize>.
Unlike other filter methods, prepare and finalize are not chained
--
2.27.0
3 years, 10 months
