* [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
@ 2020-09-22 10:49 Max Reitz
2020-09-22 10:49 ` [PATCH v2 01/20] configure: Detect libfuse Max Reitz
` (21 more replies)
0 siblings, 22 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Based-on: <20200907182011.521007-1-kwolf@redhat.com>
(“block/export: Add infrastructure and QAPI for block exports”)
(Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
Hi,
Ever since I found out that you can mount FUSE filesystems on regular
files (not just directories), I had the idea of adding FUSE block
exports to qemu where you can export block nodes as raw images. The
best thing is that you’d be able to mount an image on itself, so
whatever format it may be in, qemu lets it appear as a raw image (and
you can then use regular tools like dd on it).
The performance is quite bad so far, but we can always try to improve it
if the need arises. For now I consider it mostly a cute feature to get
easy access to the raw contents of image files in any format (without
requiring root rights).
In this version (as opposed to v1 linked above), I integrated the FUSE
export code into Kevin’s proposed common infrastructure for block
exports.
This series does the following:
First, add the FUSE export module (block/export/fuse.c) that implements
the basic file access functions. (Note that you need libfuse 3.8.0 or
later for SEEK_HOLE/SEEK_DATA.)
Second, it allows using FUSE exports as a protocol in the iotests and
makes many iotests work with it. (The file node is exported by a
background qemu instance to $SOCK_DIR.)
This gives us a lot of coverage for, well, not free (it does take twelve
patches), but for cheap; but there are still some more specialized
things we want to test, so third and last, this series adds an iotest
dedicated to FUSE exports.
(Note that as opposed to v1, I did run the iotests this time.)
Some notable changes from v1:
- Integrated everything into Kevin’s block-export infrastructure
- Use the storage daemon instead of full QEMU to provide FUSE exports
when running the iotests with -fuse
- meson rebase
- Some other rebase conflicts
git-backport-diff against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/20:[0007] [FC] 'configure: Detect libfuse'
002/20:[0255] [FC] 'fuse: Allow exporting BDSs via FUSE'
003/20:[0062] [FC] 'fuse: Implement standard FUSE operations'
004/20:[0018] [FC] 'fuse: Allow growable exports'
005/20:[0016] [FC] 'fuse: (Partially) implement fallocate()'
006/20:[0008] [FC] 'fuse: Implement hole detection through lseek'
007/20:[0036] [FC] 'iotests: Do not needlessly filter _make_test_img'
008/20:[----] [--] 'iotests: Do not pipe _make_test_img'
009/20:[0012] [FC] 'iotests: Use convert -n in some cases'
010/20:[0006] [FC] 'iotests: Avoid renaming images'
011/20:[0008] [FC] 'iotests: Derive image names from $TEST_IMG'
012/20:[----] [--] 'iotests/091: Use _cleanup_qemu instad of "wait"'
013/20:[0008] [FC] 'iotests: Restrict some Python tests to file'
014/20:[0010] [FC] 'iotests: Let _make_test_img guess $TEST_IMG_FILE'
015/20:[down] 'iotests/287: Clean up subshell test image'
016/20:[down] 'storage-daemon: Call bdrv_close_all() on exit'
017/20:[down] 'iotests: Give access to the qemu-storage-daemon'
018/20:[0042] [FC] 'iotests: Allow testing FUSE exports'
019/20:[0026] [FC] 'iotests: Enable fuse for many tests'
020/20:[0104] [FC] 'iotests/281: Add test for FUSE exports'
Max Reitz (20):
configure: Detect libfuse
fuse: Allow exporting BDSs via FUSE
fuse: Implement standard FUSE operations
fuse: Allow growable exports
fuse: (Partially) implement fallocate()
fuse: Implement hole detection through lseek
iotests: Do not needlessly filter _make_test_img
iotests: Do not pipe _make_test_img
iotests: Use convert -n in some cases
iotests/046: Avoid renaming images
iotests: Derive image names from $TEST_IMG
iotests/091: Use _cleanup_qemu instad of "wait"
iotests: Restrict some Python tests to file
iotests: Let _make_test_img guess $TEST_IMG_FILE
iotests/287: Clean up subshell test image
storage-daemon: Call bdrv_close_all() on exit
iotests: Give access to the qemu-storage-daemon
iotests: Allow testing FUSE exports
iotests: Enable fuse for many tests
iotests/308: Add test for FUSE exports
configure | 66 +++
qapi/block-export.json | 28 +-
include/block/fuse.h | 30 ++
block.c | 1 +
block/export/export.c | 4 +
block/export/fuse.c | 645 +++++++++++++++++++++++++++
storage-daemon/qemu-storage-daemon.c | 3 +
block/export/meson.build | 1 +
meson.build | 7 +
tests/qemu-iotests/025 | 2 +-
tests/qemu-iotests/026 | 2 +-
tests/qemu-iotests/028 | 16 +-
tests/qemu-iotests/028.out | 3 +
tests/qemu-iotests/031 | 2 +-
tests/qemu-iotests/034 | 2 +-
tests/qemu-iotests/036 | 2 +-
tests/qemu-iotests/037 | 2 +-
tests/qemu-iotests/038 | 2 +-
tests/qemu-iotests/039 | 2 +-
tests/qemu-iotests/046 | 7 +-
tests/qemu-iotests/046.out | 2 +-
tests/qemu-iotests/050 | 2 +-
tests/qemu-iotests/054 | 2 +-
tests/qemu-iotests/060 | 2 +-
tests/qemu-iotests/071 | 21 +-
tests/qemu-iotests/079 | 2 +-
tests/qemu-iotests/080 | 2 +-
tests/qemu-iotests/089 | 5 +-
tests/qemu-iotests/089.out | 1 +
tests/qemu-iotests/090 | 2 +-
tests/qemu-iotests/091 | 5 +-
tests/qemu-iotests/095 | 2 +-
tests/qemu-iotests/097 | 2 +-
tests/qemu-iotests/098 | 2 +-
tests/qemu-iotests/102 | 2 +-
tests/qemu-iotests/103 | 2 +-
tests/qemu-iotests/106 | 2 +-
tests/qemu-iotests/107 | 2 +-
tests/qemu-iotests/108 | 2 +-
tests/qemu-iotests/111 | 2 +-
tests/qemu-iotests/112 | 2 +-
tests/qemu-iotests/115 | 2 +-
tests/qemu-iotests/117 | 2 +-
tests/qemu-iotests/120 | 2 +-
tests/qemu-iotests/121 | 2 +-
tests/qemu-iotests/127 | 2 +-
tests/qemu-iotests/133 | 2 +-
tests/qemu-iotests/137 | 2 +-
tests/qemu-iotests/138 | 2 +-
tests/qemu-iotests/140 | 2 +-
tests/qemu-iotests/154 | 2 +-
tests/qemu-iotests/161 | 14 +-
tests/qemu-iotests/171 | 2 +-
tests/qemu-iotests/174 | 10 +-
tests/qemu-iotests/175 | 8 +-
tests/qemu-iotests/176 | 2 +-
tests/qemu-iotests/177 | 2 +-
tests/qemu-iotests/179 | 2 +-
tests/qemu-iotests/183 | 2 +-
tests/qemu-iotests/186 | 2 +-
tests/qemu-iotests/187 | 2 +-
tests/qemu-iotests/191 | 2 +-
tests/qemu-iotests/195 | 2 +-
tests/qemu-iotests/200 | 5 +-
tests/qemu-iotests/200.out | 4 +-
tests/qemu-iotests/204 | 2 +-
tests/qemu-iotests/206 | 3 +-
tests/qemu-iotests/214 | 2 +-
tests/qemu-iotests/217 | 2 +-
tests/qemu-iotests/220 | 2 +-
tests/qemu-iotests/221 | 2 +-
tests/qemu-iotests/229 | 5 +-
tests/qemu-iotests/229.out | 6 +-
tests/qemu-iotests/242 | 3 +-
tests/qemu-iotests/247 | 2 +-
tests/qemu-iotests/249 | 8 +-
tests/qemu-iotests/250 | 2 +-
tests/qemu-iotests/252 | 2 +-
tests/qemu-iotests/265 | 2 +-
tests/qemu-iotests/268 | 2 +-
tests/qemu-iotests/272 | 2 +-
tests/qemu-iotests/273 | 2 +-
tests/qemu-iotests/279 | 2 +-
tests/qemu-iotests/286 | 2 +-
tests/qemu-iotests/287 | 6 +-
tests/qemu-iotests/289 | 2 +-
tests/qemu-iotests/290 | 2 +-
tests/qemu-iotests/291 | 2 +-
tests/qemu-iotests/292 | 2 +-
tests/qemu-iotests/293 | 2 +-
tests/qemu-iotests/294 | 2 +-
tests/qemu-iotests/305 | 2 +-
tests/qemu-iotests/308 | 339 ++++++++++++++
tests/qemu-iotests/308.out | 97 ++++
tests/qemu-iotests/check | 17 +
tests/qemu-iotests/common.filter | 5 +-
tests/qemu-iotests/common.rc | 181 +++++++-
tests/qemu-iotests/group | 1 +
98 files changed, 1559 insertions(+), 126 deletions(-)
create mode 100644 include/block/fuse.h
create mode 100644 block/export/fuse.c
create mode 100755 tests/qemu-iotests/308
create mode 100644 tests/qemu-iotests/308.out
--
2.26.2
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 11:14 ` Thomas Huth
2020-09-22 10:49 ` [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE Max Reitz
` (20 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
configure | 34 ++++++++++++++++++++++++++++++++++
meson.build | 6 ++++++
2 files changed, 40 insertions(+)
diff --git a/configure b/configure
index ce27eafb0a..21c31e4694 100755
--- a/configure
+++ b/configure
@@ -538,6 +538,7 @@ meson=""
ninja=""
skip_meson=no
gettext=""
+fuse=""
bogus_os="no"
malloc_trim=""
@@ -1621,6 +1622,10 @@ for opt do
;;
--disable-libdaxctl) libdaxctl=no
;;
+ --enable-fuse) fuse=yes
+ ;;
+ --disable-fuse) fuse=no
+ ;;
*)
echo "ERROR: unknown option $opt"
echo "Try '$0 --help' for more information"
@@ -1945,6 +1950,7 @@ disabled with --disable-FEATURE, default is enabled if available:
xkbcommon xkbcommon support
rng-none dummy RNG, avoid using /dev/(u)random and getrandom()
libdaxctl libdaxctl support
+ fuse fuse block device export
NOTE: The object files are built at the place where configure is launched
EOF
@@ -6206,6 +6212,28 @@ but not implemented on your system"
fi
fi
+##########################################
+# FUSE support
+
+if test "$fuse" != "no"; then
+ cat > $TMPC <<EOF
+#define FUSE_USE_VERSION 31
+#include <fuse.h>
+#include <fuse_lowlevel.h>
+int main(void) { return 0; }
+EOF
+ fuse_cflags=$(pkg-config --cflags fuse3)
+ fuse_libs=$(pkg-config --libs fuse3)
+ if compile_prog "$fuse_cflags" "$fuse_libs"; then
+ fuse=yes
+ else
+ if test "$fuse" = "yes"; then
+ feature_not_found "fuse"
+ fi
+ fuse=no
+ fi
+fi
+
##########################################
# End of CC checks
# After here, no more $cc or $ld runs
@@ -7393,6 +7421,12 @@ if test "$secret_keyring" = "yes" ; then
echo "CONFIG_SECRET_KEYRING=y" >> $config_host_mak
fi
+if test "$fuse" = "yes"; then
+ echo "CONFIG_FUSE=y" >> $config_host_mak
+ echo "FUSE_CFLAGS=$fuse_cflags" >> $config_host_mak
+ echo "FUSE_LIBS=$fuse_libs" >> $config_host_mak
+fi
+
if test "$tcg_interpreter" = "yes"; then
QEMU_INCLUDES="-iquote ${source_path}/tcg/tci $QEMU_INCLUDES"
elif test "$ARCH" = "sparc64" ; then
diff --git a/meson.build b/meson.build
index 86e1cca0ad..85addd8562 100644
--- a/meson.build
+++ b/meson.build
@@ -436,6 +436,11 @@ if 'CONFIG_TASN1' in config_host
endif
keyutils = dependency('libkeyutils', required: false,
method: 'pkg-config', static: enable_static)
+libfuse = not_found
+if 'CONFIG_FUSE' in config_host
+ libfuse = declare_dependency(compile_args: config_host['FUSE_CFLAGS'].split(),
+ link_args: config_host['FUSE_LIBS'].split())
+endif
has_gettid = cc.has_function('gettid')
@@ -1531,6 +1536,7 @@ endif
summary_info += {'thread sanitizer': config_host.has_key('CONFIG_TSAN')}
summary_info += {'rng-none': config_host.has_key('CONFIG_RNG_NONE')}
summary_info += {'Linux keyring': config_host.has_key('CONFIG_SECRET_KEYRING')}
+summary_info += {'fuse exports': config_host.has_key('CONFIG_FUSE')}
summary(summary_info, bool_yn: true)
if not supported_cpus.contains(cpu)
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
2020-09-22 10:49 ` [PATCH v2 01/20] configure: Detect libfuse Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-10-15 8:57 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 03/20] fuse: Implement standard FUSE operations Max Reitz
` (19 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
block-export-add type=fuse allows mounting block graph nodes via FUSE on
some existing regular file. That file should then appears like a raw
disk image, and accesses to it result in accesses to the exported BDS.
Right now, we only implement the necessary block export functions to set
it up and shut it down. We do not implement any access functions, so
accessing the mount point only results in errors. This will be
addressed by a followup patch.
We keep a hash table of exported mount points, because we want to be
able to detect when users try to use a mount point twice. This is
because we invoke stat() to check whether the given mount point is a
regular file, but if that file is served by ourselves (because it is
already used as a mount point), then this stat() would have to be served
by ourselves, too, which is impossible to do while we (as the caller)
are waiting for it to settle. Therefore, keep track of mount point
paths to at least catch the most obvious instances of that problem.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 24 +++-
include/block/fuse.h | 30 +++++
block.c | 1 +
block/export/export.c | 4 +
block/export/fuse.c | 253 +++++++++++++++++++++++++++++++++++++++
block/export/meson.build | 1 +
6 files changed, 311 insertions(+), 2 deletions(-)
create mode 100644 include/block/fuse.h
create mode 100644 block/export/fuse.c
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 0c045dfe7b..cb5bd54cbf 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -174,6 +174,21 @@
##
{ 'command': 'nbd-server-stop' }
+##
+# @BlockExportOptionsFuse:
+#
+# Options for exporting a block graph node on some (file) mountpoint
+# as a raw image.
+#
+# @mountpoint: Path on which to export the block device via FUSE.
+# This must point to an existing regular file.
+#
+# Since: 5.2
+##
+{ 'struct': 'BlockExportOptionsFuse',
+ 'data': { 'mountpoint': 'str' },
+ 'if': 'defined(CONFIG_FUSE)' }
+
##
# @BlockExportType:
#
@@ -181,10 +196,13 @@
#
# @nbd: NBD export
#
+# @fuse: Fuse export (since: 5.2)
+#
# Since: 4.2
##
{ 'enum': 'BlockExportType',
- 'data': [ 'nbd' ] }
+ 'data': [ 'nbd',
+ { 'name': 'fuse', 'if': 'defined(CONFIG_FUSE)' } ] }
##
# @BlockExportOptions:
@@ -213,7 +231,9 @@
'*writethrough': 'bool' },
'discriminator': 'type',
'data': {
- 'nbd': 'BlockExportOptionsNbd'
+ 'nbd': 'BlockExportOptionsNbd',
+ 'fuse': { 'type': 'BlockExportOptionsFuse',
+ 'if': 'defined(CONFIG_FUSE)' }
} }
##
diff --git a/include/block/fuse.h b/include/block/fuse.h
new file mode 100644
index 0000000000..ffa91fe364
--- /dev/null
+++ b/include/block/fuse.h
@@ -0,0 +1,30 @@
+/*
+ * Present a block device as a raw image through FUSE
+ *
+ * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 or later of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef BLOCK_FUSE_H
+#define BLOCK_FUSE_H
+
+#ifdef CONFIG_FUSE
+
+#include "block/export.h"
+
+extern const BlockExportDriver blk_exp_fuse;
+
+#endif /* CONFIG_FUSE */
+
+#endif
diff --git a/block.c b/block.c
index acde53f92a..7bf45f6ede 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,7 @@
#include "block/trace.h"
#include "block/block_int.h"
#include "block/blockjob.h"
+#include "block/fuse.h"
#include "block/nbd.h"
#include "block/qdict.h"
#include "qemu/error-report.h"
diff --git a/block/export/export.c b/block/export/export.c
index 64d39a6934..ba866d6ba7 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -16,6 +16,7 @@
#include "block/block.h"
#include "sysemu/block-backend.h"
#include "block/export.h"
+#include "block/fuse.h"
#include "block/nbd.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-block-export.h"
@@ -24,6 +25,9 @@
static const BlockExportDriver *blk_exp_drivers[] = {
&blk_exp_nbd,
+#ifdef CONFIG_FUSE
+ &blk_exp_fuse,
+#endif
};
/* Only accessed from the main thread */
diff --git a/block/export/fuse.c b/block/export/fuse.c
new file mode 100644
index 0000000000..75f11d2514
--- /dev/null
+++ b/block/export/fuse.c
@@ -0,0 +1,253 @@
+/*
+ * Present a block device as a raw image through FUSE
+ *
+ * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 or later of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define FUSE_USE_VERSION 31
+
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "block/block.h"
+#include "block/export.h"
+#include "block/fuse.h"
+#include "block/qapi.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-block.h"
+#include "sysemu/block-backend.h"
+
+#include <fuse.h>
+#include <fuse_lowlevel.h>
+
+
+typedef struct FuseExport {
+ BlockExport common;
+
+ struct fuse_session *fuse_session;
+ struct fuse_buf fuse_buf;
+ bool mounted, fd_handler_set_up;
+
+ char *mountpoint;
+ bool writable;
+} FuseExport;
+
+static GHashTable *exports;
+static const struct fuse_lowlevel_ops fuse_ops;
+
+static void fuse_export_shutdown(BlockExport *exp);
+static void fuse_export_delete(BlockExport *exp);
+
+static void init_fuse(void);
+static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
+ Error **errp);
+static void read_from_fuse_export(void *opaque);
+
+static bool is_regular_file(const char *path, Error **errp);
+
+
+static int fuse_export_create(BlockExport *blk_exp,
+ BlockExportOptions *blk_exp_args,
+ Error **errp)
+{
+ FuseExport *exp = container_of(blk_exp, FuseExport, common);
+ BlockExportOptionsFuse *args = &blk_exp_args->u.fuse;
+ int ret;
+
+ assert(blk_exp_args->type == BLOCK_EXPORT_TYPE_FUSE);
+
+ init_fuse();
+
+ /*
+ * It is important to do this check before calling is_regular_file() --
+ * that function will do a stat(), which we would have to handle if we
+ * already exported something on @mountpoint. But we cannot, because
+ * we are currently caught up here.
+ * (Note that ideally we would want to resolve relative paths here,
+ * but bdrv_make_absolute_filename() might do the wrong thing for
+ * paths that contain colons, and realpath() would resolve symlinks,
+ * which we do not want: The mount point is not going to be the
+ * symlink's destination, but the link itself.)
+ * So this will not catch all potential clashes, but hopefully at
+ * least the most common one of specifying exactly the same path
+ * string twice.
+ */
+ if (g_hash_table_contains(exports, args->mountpoint)) {
+ error_setg(errp, "There already is a FUSE export on '%s'",
+ args->mountpoint);
+ ret = -EEXIST;
+ goto fail;
+ }
+
+ if (!is_regular_file(args->mountpoint, errp)) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ exp->mountpoint = g_strdup(args->mountpoint);
+ exp->writable = blk_exp_args->writable;
+
+ ret = setup_fuse_export(exp, args->mountpoint, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ g_hash_table_insert(exports, g_strdup(args->mountpoint), NULL);
+ return 0;
+
+fail:
+ fuse_export_shutdown(blk_exp);
+ fuse_export_delete(blk_exp);
+ return ret;
+}
+
+/**
+ * Ensure that the global FUSE context is set up.
+ */
+static void init_fuse(void)
+{
+ if (exports) {
+ return;
+ }
+
+ exports = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
+}
+
+/**
+ * Create exp->fuse_session and mount it.
+ */
+static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
+ Error **errp)
+{
+ const char *fuse_argv[2];
+ struct fuse_args fuse_args;
+ int ret;
+
+ fuse_argv[0] = ""; /* Dummy program name */
+ fuse_argv[1] = NULL;
+ fuse_args = (struct fuse_args)FUSE_ARGS_INIT(1, (char **)fuse_argv);
+
+ exp->fuse_session = fuse_session_new(&fuse_args, &fuse_ops,
+ sizeof(fuse_ops), exp);
+ if (!exp->fuse_session) {
+ error_setg(errp, "Failed to set up FUSE session");
+ return -EIO;
+ }
+
+ ret = fuse_session_mount(exp->fuse_session, mountpoint);
+ if (ret < 0) {
+ error_setg(errp, "Failed to mount FUSE session to export");
+ return -EIO;
+ }
+ exp->mounted = true;
+
+ aio_set_fd_handler(exp->common.ctx,
+ fuse_session_fd(exp->fuse_session), true,
+ read_from_fuse_export, NULL, NULL, exp);
+ exp->fd_handler_set_up = true;
+
+ return 0;
+}
+
+/**
+ * Callback to be invoked when the FUSE session FD can be read from.
+ * (This is basically the FUSE event loop.)
+ */
+static void read_from_fuse_export(void *opaque)
+{
+ FuseExport *exp = opaque;
+ int ret;
+
+ blk_exp_ref(&exp->common);
+
+ ret = fuse_session_receive_buf(exp->fuse_session, &exp->fuse_buf);
+ if (ret < 0) {
+ goto out;
+ }
+
+ fuse_session_process_buf(exp->fuse_session, &exp->fuse_buf);
+
+out:
+ blk_exp_unref(&exp->common);
+}
+
+static void fuse_export_shutdown(BlockExport *blk_exp)
+{
+ FuseExport *exp = container_of(blk_exp, FuseExport, common);
+
+ if (exp->fuse_session) {
+ fuse_session_exit(exp->fuse_session);
+
+ if (exp->mounted) {
+ fuse_session_unmount(exp->fuse_session);
+ exp->mounted = false;
+ }
+
+ if (exp->fd_handler_set_up) {
+ aio_set_fd_handler(exp->common.ctx,
+ fuse_session_fd(exp->fuse_session), true,
+ NULL, NULL, NULL, NULL);
+ exp->fd_handler_set_up = false;
+ }
+
+ fuse_session_destroy(exp->fuse_session);
+ exp->fuse_session = NULL;
+ }
+
+ if (exp->mountpoint) {
+ g_hash_table_remove(exports, exp->mountpoint);
+ }
+}
+
+static void fuse_export_delete(BlockExport *blk_exp)
+{
+ FuseExport *exp = container_of(blk_exp, FuseExport, common);
+
+ free(exp->fuse_buf.mem);
+ g_free(exp->mountpoint);
+}
+
+/**
+ * Check whether @path points to a regular file. If not, put an
+ * appropriate message into *errp.
+ */
+static bool is_regular_file(const char *path, Error **errp)
+{
+ struct stat statbuf;
+ int ret;
+
+ ret = stat(path, &statbuf);
+ if (ret < 0) {
+ error_setg_errno(errp, errno, "Failed to stat '%s'", path);
+ return false;
+ }
+
+ if (!S_ISREG(statbuf.st_mode)) {
+ error_setg(errp, "'%s' is not a regular file", path);
+ return false;
+ }
+
+ return true;
+}
+
+static const struct fuse_lowlevel_ops fuse_ops = {
+};
+
+const BlockExportDriver blk_exp_fuse = {
+ .type = BLOCK_EXPORT_TYPE_FUSE,
+ .instance_size = sizeof(FuseExport),
+ .create = fuse_export_create,
+ .delete = fuse_export_delete,
+ .request_shutdown = fuse_export_shutdown,
+};
diff --git a/block/export/meson.build b/block/export/meson.build
index 558ef35d38..3f7f8c5263 100644
--- a/block/export/meson.build
+++ b/block/export/meson.build
@@ -1 +1,2 @@
block_ss.add(files('export.c'))
+block_ss.add(when: ['CONFIG_FUSE', libfuse], if_true: files('fuse.c'))
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 03/20] fuse: Implement standard FUSE operations
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
2020-09-22 10:49 ` [PATCH v2 01/20] configure: Detect libfuse Max Reitz
2020-09-22 10:49 ` [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-10-15 9:46 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 04/20] fuse: Allow growable exports Max Reitz
` (18 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
This makes the export actually useful instead of only producing errors
whenever it is accessed.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/export/fuse.c | 226 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 226 insertions(+)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 75f11d2514..8fc667231d 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -32,6 +32,10 @@
#include <fuse_lowlevel.h>
+/* Prevent overly long bounce buffer allocations */
+#define FUSE_MAX_BOUNCE_BYTES (MIN(BDRV_REQUEST_MAX_BYTES, 64 * 1024 * 1024))
+
+
typedef struct FuseExport {
BlockExport common;
@@ -241,7 +245,229 @@ static bool is_regular_file(const char *path, Error **errp)
return true;
}
+
+/**
+ * Let clients look up files. Always return ENOENT because we only
+ * care about the mountpoint itself.
+ */
+static void fuse_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
+{
+ fuse_reply_err(req, ENOENT);
+}
+
+/**
+ * Let clients get file attributes (i.e., stat() the file).
+ */
+static void fuse_getattr(fuse_req_t req, fuse_ino_t inode,
+ struct fuse_file_info *fi)
+{
+ struct stat statbuf;
+ int64_t length, allocated_blocks;
+ time_t now = time(NULL);
+ ImageInfo *info = NULL;
+ FuseExport *exp = fuse_req_userdata(req);
+ mode_t mode;
+ Error *local_error = NULL;
+
+ length = blk_getlength(exp->common.blk);
+ if (length < 0) {
+ fuse_reply_err(req, -length);
+ return;
+ }
+
+ bdrv_query_image_info(blk_bs(exp->common.blk), &info, &local_error);
+ if (local_error) {
+ allocated_blocks = DIV_ROUND_UP(length, 512);
+ } else {
+ allocated_blocks = DIV_ROUND_UP(info->actual_size, 512);
+ }
+
+ qapi_free_ImageInfo(info);
+ error_free(local_error);
+ local_error = NULL;
+
+ mode = S_IFREG | S_IRUSR;
+ if (exp->writable) {
+ mode |= S_IWUSR;
+ }
+
+ statbuf = (struct stat) {
+ .st_ino = inode,
+ .st_mode = mode,
+ .st_nlink = 1,
+ .st_uid = getuid(),
+ .st_gid = getgid(),
+ .st_size = length,
+ .st_blksize = blk_bs(exp->common.blk)->bl.request_alignment,
+ .st_blocks = allocated_blocks,
+ .st_atime = now,
+ .st_mtime = now,
+ .st_ctime = now,
+ };
+
+ fuse_reply_attr(req, &statbuf, 1.);
+}
+
+static int fuse_do_truncate(const FuseExport *exp, int64_t size,
+ PreallocMode prealloc)
+{
+ uint64_t blk_perm, blk_shared_perm;
+ int ret;
+
+ blk_get_perm(exp->common.blk, &blk_perm, &blk_shared_perm);
+
+ ret = blk_set_perm(exp->common.blk, blk_perm | BLK_PERM_RESIZE,
+ blk_shared_perm, NULL);
+ if (ret < 0) {
+ return ret;
+ }
+
+ ret = blk_truncate(exp->common.blk, size, true, prealloc, 0, NULL);
+
+ /* Must succeed, because we are only giving up the RESIZE permission */
+ blk_set_perm(exp->common.blk, blk_perm, blk_shared_perm, &error_abort);
+
+ return ret;
+}
+
+/**
+ * Let clients set file attributes. Only resizing is supported.
+ */
+static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf,
+ int to_set, struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+ int ret;
+
+ if (!exp->writable) {
+ fuse_reply_err(req, EACCES);
+ return;
+ }
+
+ if (to_set & ~FUSE_SET_ATTR_SIZE) {
+ fuse_reply_err(req, ENOTSUP);
+ return;
+ }
+
+ ret = fuse_do_truncate(exp, statbuf->st_size, PREALLOC_MODE_OFF);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+
+ fuse_getattr(req, inode, fi);
+}
+
+/**
+ * Let clients open a file (i.e., the exported image).
+ */
+static void fuse_open(fuse_req_t req, fuse_ino_t inode,
+ struct fuse_file_info *fi)
+{
+ fuse_reply_open(req, fi);
+}
+
+/**
+ * Handle client reads from the exported image.
+ */
+static void fuse_read(fuse_req_t req, fuse_ino_t inode,
+ size_t size, off_t offset, struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+ int64_t length;
+ void *buf;
+ int ret;
+
+ /**
+ * Clients will expect short reads at EOF, so we have to limit
+ * offset+size to the image length.
+ */
+ length = blk_getlength(exp->common.blk);
+ if (length < 0) {
+ fuse_reply_err(req, -length);
+ return;
+ }
+
+ size = MIN(size, FUSE_MAX_BOUNCE_BYTES);
+ if (offset + size > length) {
+ size = length - offset;
+ }
+
+ buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
+ if (!buf) {
+ fuse_reply_err(req, ENOMEM);
+ return;
+ }
+
+ ret = blk_pread(exp->common.blk, offset, buf, size);
+ if (ret >= 0) {
+ fuse_reply_buf(req, buf, size);
+ } else {
+ fuse_reply_err(req, -ret);
+ }
+
+ qemu_vfree(buf);
+}
+
+/**
+ * Handle client writes to the exported image.
+ */
+static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
+ size_t size, off_t offset, struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+ int64_t length;
+ int ret;
+
+ if (!exp->writable) {
+ fuse_reply_err(req, EACCES);
+ return;
+ }
+
+ /**
+ * Clients will expect short writes at EOF, so we have to limit
+ * offset+size to the image length.
+ */
+ length = blk_getlength(exp->common.blk);
+ if (length < 0) {
+ fuse_reply_err(req, -length);
+ return;
+ }
+
+ size = MIN(size, BDRV_REQUEST_MAX_BYTES);
+ if (offset + size > length) {
+ size = length - offset;
+ }
+
+ ret = blk_pwrite(exp->common.blk, offset, buf, size, 0);
+ if (ret >= 0) {
+ fuse_reply_write(req, size);
+ } else {
+ fuse_reply_err(req, -ret);
+ }
+}
+
+/**
+ * Let clients flush the exported image.
+ */
+static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
+ struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+ int ret;
+
+ ret = blk_flush(exp->common.blk);
+ fuse_reply_err(req, ret < 0 ? -ret : 0);
+}
+
static const struct fuse_lowlevel_ops fuse_ops = {
+ .lookup = fuse_lookup,
+ .getattr = fuse_getattr,
+ .setattr = fuse_setattr,
+ .open = fuse_open,
+ .read = fuse_read,
+ .write = fuse_write,
+ .flush = fuse_flush,
};
const BlockExportDriver blk_exp_fuse = {
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 04/20] fuse: Allow growable exports
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (2 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 03/20] fuse: Implement standard FUSE operations Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-10-15 10:41 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 05/20] fuse: (Partially) implement fallocate() Max Reitz
` (17 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
These will behave more like normal files in that writes beyond the EOF
will automatically grow the export size.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-export.json | 6 +++++-
block/export/fuse.c | 12 +++++++++++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index cb5bd54cbf..cb26daa98b 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -183,10 +183,14 @@
# @mountpoint: Path on which to export the block device via FUSE.
# This must point to an existing regular file.
#
+# @growable: Whether writes beyond the EOF should grow the block node
+# accordingly. (default: false)
+#
# Since: 5.2
##
{ 'struct': 'BlockExportOptionsFuse',
- 'data': { 'mountpoint': 'str' },
+ 'data': { 'mountpoint': 'str',
+ '*growable': 'bool' },
'if': 'defined(CONFIG_FUSE)' }
##
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 8fc667231d..f3a84579ba 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -45,6 +45,7 @@ typedef struct FuseExport {
char *mountpoint;
bool writable;
+ bool growable;
} FuseExport;
static GHashTable *exports;
@@ -101,6 +102,7 @@ static int fuse_export_create(BlockExport *blk_exp,
exp->mountpoint = g_strdup(args->mountpoint);
exp->writable = blk_exp_args->writable;
+ exp->growable = args->growable;
ret = setup_fuse_export(exp, args->mountpoint, errp);
if (ret < 0) {
@@ -436,7 +438,15 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
size = MIN(size, BDRV_REQUEST_MAX_BYTES);
if (offset + size > length) {
- size = length - offset;
+ if (exp->growable) {
+ ret = fuse_do_truncate(exp, offset + size, PREALLOC_MODE_OFF);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ } else {
+ size = length - offset;
+ }
}
ret = blk_pwrite(exp->common.blk, offset, buf, size, 0);
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 05/20] fuse: (Partially) implement fallocate()
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (3 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 04/20] fuse: Allow growable exports Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 06/20] fuse: Implement hole detection through lseek Max Reitz
` (16 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
This allows allocating areas after the (old) EOF as part of a growing
resize, writing zeroes, and discarding.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/export/fuse.c | 79 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index f3a84579ba..7dfb4eb280 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -457,6 +457,84 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
}
}
+/**
+ * Let clients perform various fallocate() operations.
+ */
+static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
+ off_t offset, off_t length,
+ struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+ int64_t blk_len;
+ int ret;
+
+ if (!exp->writable) {
+ fuse_reply_err(req, EACCES);
+ return;
+ }
+
+ blk_len = blk_getlength(exp->common.blk);
+ if (blk_len < 0) {
+ fuse_reply_err(req, -blk_len);
+ return;
+ }
+
+ if (mode & FALLOC_FL_KEEP_SIZE) {
+ length = MIN(length, blk_len - offset);
+ }
+
+ if (mode & FALLOC_FL_PUNCH_HOLE) {
+ if (!(mode & FALLOC_FL_KEEP_SIZE)) {
+ fuse_reply_err(req, EINVAL);
+ return;
+ }
+
+ do {
+ int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
+
+ ret = blk_pdiscard(exp->common.blk, offset, size);
+ length -= size;
+ } while (ret == 0 && length > 0);
+ } else if (mode & FALLOC_FL_ZERO_RANGE) {
+ if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + length > blk_len) {
+ ret = fuse_do_truncate(exp, offset + length, PREALLOC_MODE_OFF);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ }
+
+ do {
+ int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
+
+ ret = blk_pwrite_zeroes(exp->common.blk,
+ offset, size, 0);
+ length -= size;
+ } while (ret == 0 && length > 0);
+ } else if (!mode) {
+ /* We can only fallocate at the EOF with a truncate */
+ if (offset < blk_len) {
+ fuse_reply_err(req, EOPNOTSUPP);
+ return;
+ }
+
+ if (offset > blk_len) {
+ /* No preallocation needed here */
+ ret = fuse_do_truncate(exp, offset, PREALLOC_MODE_OFF);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+ }
+
+ ret = fuse_do_truncate(exp, offset + length, PREALLOC_MODE_FALLOC);
+ } else {
+ ret = -EOPNOTSUPP;
+ }
+
+ fuse_reply_err(req, ret < 0 ? -ret : 0);
+}
+
/**
* Let clients flush the exported image.
*/
@@ -477,6 +555,7 @@ static const struct fuse_lowlevel_ops fuse_ops = {
.open = fuse_open,
.read = fuse_read,
.write = fuse_write,
+ .fallocate = fuse_fallocate,
.flush = fuse_flush,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 06/20] fuse: Implement hole detection through lseek
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (4 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 05/20] fuse: (Partially) implement fallocate() Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 07/20] iotests: Do not needlessly filter _make_test_img Max Reitz
` (15 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
This is a relatively new feature in libfuse (available since 3.8.0,
which was released in November 2019), so we have to let configure check
whether it is available before making use of it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
configure | 32 +++++++++++++++++++
block/export/fuse.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
meson.build | 1 +
3 files changed, 110 insertions(+)
diff --git a/configure b/configure
index 21c31e4694..6b9184b62a 100755
--- a/configure
+++ b/configure
@@ -6226,11 +6226,39 @@ EOF
fuse_libs=$(pkg-config --libs fuse3)
if compile_prog "$fuse_cflags" "$fuse_libs"; then
fuse=yes
+
+ cat > $TMPC <<EOF
+#define FUSE_USE_VERSION 31
+#include <fuse.h>
+#include <fuse_lowlevel.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/types.h>
+static void fuse_lseek(fuse_req_t req, fuse_ino_t inode, off_t offset,
+ int whence, struct fuse_file_info *fi)
+{
+ if (whence == SEEK_DATA || whence == SEEK_HOLE) {
+ fuse_reply_lseek(req, offset);
+ } else {
+ fuse_reply_err(req, EINVAL);
+ }
+}
+const struct fuse_lowlevel_ops fuse_ops = {
+ .lseek = fuse_lseek,
+};
+int main(void) { return 0; }
+EOF
+ if compile_prog "$fuse_cflags" "$fuse_libs"; then
+ fuse_lseek=yes
+ else
+ fuse_lseek=no
+ fi
else
if test "$fuse" = "yes"; then
feature_not_found "fuse"
fi
fuse=no
+ fuse_lseek=no
fi
fi
@@ -7425,6 +7453,10 @@ if test "$fuse" = "yes"; then
echo "CONFIG_FUSE=y" >> $config_host_mak
echo "FUSE_CFLAGS=$fuse_cflags" >> $config_host_mak
echo "FUSE_LIBS=$fuse_libs" >> $config_host_mak
+
+ if test "$fuse_lseek" = "yes"; then
+ echo "CONFIG_FUSE_LSEEK=y" >> $config_host_mak
+ fi
fi
if test "$tcg_interpreter" = "yes"; then
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 7dfb4eb280..ee90f5a185 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -548,6 +548,80 @@ static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
fuse_reply_err(req, ret < 0 ? -ret : 0);
}
+#ifdef CONFIG_FUSE_LSEEK
+/**
+ * Let clients inquire allocation status.
+ */
+static void fuse_lseek(fuse_req_t req, fuse_ino_t inode, off_t offset,
+ int whence, struct fuse_file_info *fi)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+
+ if (whence != SEEK_HOLE && whence != SEEK_DATA) {
+ fuse_reply_err(req, EINVAL);
+ return;
+ }
+
+ while (true) {
+ int64_t pnum;
+ int ret;
+
+ ret = bdrv_block_status_above(blk_bs(exp->common.blk), NULL,
+ offset, INT64_MAX, &pnum, NULL, NULL);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return;
+ }
+
+ if (!pnum && (ret & BDRV_BLOCK_EOF)) {
+ int64_t blk_len;
+
+ /*
+ * If blk_getlength() rounds (e.g. by sectors), then the
+ * export length will be rounded, too. However,
+ * bdrv_block_status_above() may return EOF at unaligned
+ * offsets. We must not let this become visible and thus
+ * always simulate a hole between @offset (the real EOF)
+ * and @blk_len (the client-visible EOF).
+ */
+
+ blk_len = blk_getlength(exp->common.blk);
+ if (blk_len < 0) {
+ fuse_reply_err(req, -blk_len);
+ return;
+ }
+
+ if (offset > blk_len || whence == SEEK_DATA) {
+ fuse_reply_err(req, ENXIO);
+ } else {
+ fuse_reply_lseek(req, offset);
+ }
+ return;
+ }
+
+ if (ret & BDRV_BLOCK_DATA) {
+ if (whence == SEEK_DATA) {
+ fuse_reply_lseek(req, offset);
+ return;
+ }
+ } else {
+ if (whence == SEEK_HOLE) {
+ fuse_reply_lseek(req, offset);
+ return;
+ }
+ }
+
+ /* Safety check against infinite loops */
+ if (!pnum) {
+ fuse_reply_err(req, ENXIO);
+ return;
+ }
+
+ offset += pnum;
+ }
+}
+#endif
+
static const struct fuse_lowlevel_ops fuse_ops = {
.lookup = fuse_lookup,
.getattr = fuse_getattr,
@@ -557,6 +631,9 @@ static const struct fuse_lowlevel_ops fuse_ops = {
.write = fuse_write,
.fallocate = fuse_fallocate,
.flush = fuse_flush,
+#ifdef CONFIG_FUSE_LSEEK
+ .lseek = fuse_lseek,
+#endif
};
const BlockExportDriver blk_exp_fuse = {
diff --git a/meson.build b/meson.build
index 85addd8562..1db6a46d14 100644
--- a/meson.build
+++ b/meson.build
@@ -1537,6 +1537,7 @@ summary_info += {'thread sanitizer': config_host.has_key('CONFIG_TSAN')}
summary_info += {'rng-none': config_host.has_key('CONFIG_RNG_NONE')}
summary_info += {'Linux keyring': config_host.has_key('CONFIG_SECRET_KEYRING')}
summary_info += {'fuse exports': config_host.has_key('CONFIG_FUSE')}
+summary_info += {'fuse lseek': config_host.has_key('CONFIG_FUSE_LSEEK')}
summary(summary_info, bool_yn: true)
if not supported_cpus.contains(cpu)
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 07/20] iotests: Do not needlessly filter _make_test_img
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (5 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 06/20] fuse: Implement hole detection through lseek Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 08/20] iotests: Do not pipe _make_test_img Max Reitz
` (14 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
In most cases, _make_test_img does not need a _filter_imgfmt on top. It
does that by itself.
(The exception is when IMGFMT has been overwritten but TEST_IMG has not.
In such cases, we do need a _filter_imgfmt on top to filter the test's
original IMGFMT from TEST_IMG.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/161 | 12 ++++++------
tests/qemu-iotests/175 | 6 +++---
tests/qemu-iotests/249 | 6 +++---
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/qemu-iotests/161 b/tests/qemu-iotests/161
index e270976d87..bbf7dbbc5c 100755
--- a/tests/qemu-iotests/161
+++ b/tests/qemu-iotests/161
@@ -48,9 +48,9 @@ _supported_os Linux
IMG_SIZE=1M
# Create the images
-TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE | _filter_imgfmt
-TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT | _filter_imgfmt
-_make_test_img -b "$TEST_IMG.int" -F $IMGFMT -F $IMGFMT | _filter_imgfmt
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
+TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT
+_make_test_img -b "$TEST_IMG.int" -F $IMGFMT -F $IMGFMT
# First test: reopen $TEST.IMG changing the detect-zeroes option on
# its backing file ($TEST_IMG.int).
@@ -105,9 +105,9 @@ echo
echo "*** Commit and then change an option on the backing file"
echo
# Create the images again
-TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE | _filter_imgfmt
-TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT| _filter_imgfmt
-_make_test_img -b "$TEST_IMG.int" -F $IMGFMT | _filter_imgfmt
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
+TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT
+_make_test_img -b "$TEST_IMG.int" -F $IMGFMT
_launch_qemu -drive if=none,file="${TEST_IMG}"
_send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index 00a626aa63..c3c2aed653 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -89,20 +89,20 @@ min_blocks=$(stat -c '%b' "$TEST_DIR/empty")
echo
echo "== creating image with default preallocation =="
-_make_test_img -o extent_size_hint=0 $size | _filter_imgfmt
+_make_test_img -o extent_size_hint=0 $size
stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
for mode in off full falloc; do
echo
echo "== creating image with preallocation $mode =="
- _make_test_img -o preallocation=$mode,extent_size_hint=0 $size | _filter_imgfmt
+ _make_test_img -o preallocation=$mode,extent_size_hint=0 $size
stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
done
for new_size in 4096 1048576; do
echo
echo "== resize empty image with block_resize =="
- _make_test_img -o extent_size_hint=0 0 | _filter_imgfmt
+ _make_test_img -o extent_size_hint=0 0
_block_resize $TEST_IMG $new_size >/dev/null
stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $new_size
done
diff --git a/tests/qemu-iotests/249 b/tests/qemu-iotests/249
index 68f13ed328..a9aa9303eb 100755
--- a/tests/qemu-iotests/249
+++ b/tests/qemu-iotests/249
@@ -48,9 +48,9 @@ _supported_os Linux
IMG_SIZE=1M
# Create the images: base <- int <- active
-TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE | _filter_imgfmt
-TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT | _filter_imgfmt
-_make_test_img -b "$TEST_IMG.int" -F $IMGFMT | _filter_imgfmt
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
+TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" -F $IMGFMT
+_make_test_img -b "$TEST_IMG.int" -F $IMGFMT
# Launch QEMU with these two drives:
# none0: base (read-only)
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 08/20] iotests: Do not pipe _make_test_img
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (6 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 07/20] iotests: Do not needlessly filter _make_test_img Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 09/20] iotests: Use convert -n in some cases Max Reitz
` (13 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Executing _make_test_img as part of a pipe will undo all variable
changes it has done. As such, this could not work with FUSE (because
we want to remember all of our exports and their qemu instances).
Replace the pipe by a temporary file in 071 and 174 (the two tests that
can run on FUSE).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/071 | 19 +++++++++++++++----
tests/qemu-iotests/174 | 10 +++++++++-
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 88faebcc1d..18fe9054b0 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -61,8 +61,17 @@ echo
echo "=== Testing blkverify through filename ==="
echo
-TEST_IMG="$TEST_IMG.base" IMGFMT="raw" _make_test_img --no-opts $IMG_SIZE |\
- _filter_imgfmt
+# _make_test_img may set variables that we need to retain. Everything
+# in a pipe is executed in a subshell, so doing so would throw away
+# all changes. Therefore, we have to store the output in some temp
+# file and filter that.
+scratch_out="$TEST_DIR/img-create.out"
+
+TEST_IMG="$TEST_IMG.base" IMGFMT="raw" _make_test_img --no-opts $IMG_SIZE \
+ >"$scratch_out"
+_filter_imgfmt <"$scratch_out"
+rm -f "$scratch_out"
+
_make_test_img $IMG_SIZE
$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
-c 'read 0 512' -c 'write -P 42 0x38000 512' -c 'read -P 42 0x38000 512' | _filter_qemu_io
@@ -76,8 +85,10 @@ echo
echo "=== Testing blkverify through file blockref ==="
echo
-TEST_IMG="$TEST_IMG.base" IMGFMT="raw" _make_test_img --no-opts $IMG_SIZE |\
- _filter_imgfmt
+TEST_IMG="$TEST_IMG.base" IMGFMT="raw" _make_test_img --no-opts $IMG_SIZE \
+ >"$scratch_out"
+_filter_imgfmt <"$scratch_out"
+
_make_test_img $IMG_SIZE
$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base,file.test.driver=$IMGFMT,file.test.file.filename=$TEST_IMG" \
-c 'read 0 512' -c 'write -P 42 0x38000 512' -c 'read -P 42 0x38000 512' | _filter_qemu_io
diff --git a/tests/qemu-iotests/174 b/tests/qemu-iotests/174
index e2f14a38c6..1b0dd2e8b7 100755
--- a/tests/qemu-iotests/174
+++ b/tests/qemu-iotests/174
@@ -40,7 +40,15 @@ _unsupported_fmt raw
size=256K
-IMGFMT=raw IMGKEYSECRET= _make_test_img --no-opts $size | _filter_imgfmt
+
+# _make_test_img may set variables that we need to retain. Everything
+# in a pipe is executed in a subshell, so doing so would throw away
+# all changes. Therefore, we have to store the output in some temp
+# file and filter that.
+scratch_out="$TEST_DIR/img-create.out"
+IMGFMT=raw IMGKEYSECRET= _make_test_img --no-opts $size >"$scratch_out"
+_filter_imgfmt <"$scratch_out"
+rm -f "$scratch_out"
echo
echo "== reading wrong format should fail =="
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 09/20] iotests: Use convert -n in some cases
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (7 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 08/20] iotests: Do not pipe _make_test_img Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 10/20] iotests/046: Avoid renaming images Max Reitz
` (12 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
qemu-img convert (without -n) can often be replaced by a combination of
_make_test_img + qemu-img convert -n. Doing so allows converting to
protocols that do not allow direct file creation, such as FUSE exports.
The only problem is that for formats other than qcow2 and qed (qcow1 at
least), this may lead to high disk usage for some reason, so we cannot
do it everywhere.
But we can do it in 028 and 089, so let us do that so they can run on
FUSE exports. Also, in 028 this allows us to remove a 9-line comment
that used to explain why we cannot safely filter drive-backup's image
creation output.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/028 | 14 ++++----------
tests/qemu-iotests/028.out | 3 +++
tests/qemu-iotests/089 | 3 ++-
tests/qemu-iotests/089.out | 1 +
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028
index 6dd3ae09a3..864dc4a4e2 100755
--- a/tests/qemu-iotests/028
+++ b/tests/qemu-iotests/028
@@ -116,16 +116,10 @@ else
QEMU_COMM_TIMEOUT=1
fi
-# Silence output since it contains the disk image path and QEMU's readline
-# character echoing makes it very hard to filter the output. Plus, there
-# is no telling how many times the command will repeat before succeeding.
-# (Note that creating the image results in a "Formatting..." message over
-# stdout, which is the same channel the monitor uses. We cannot reliably
-# wait for it because the monitor output may interact with it in such a
-# way that _timed_wait_for cannot read it. However, once the block job is
-# done, we know that the "Formatting..." message must have appeared
-# already, so the output is still deterministic.)
-silent=y _send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)"
+TEST_IMG="$TEST_IMG.copy" _make_test_img $image_size
+_send_qemu_cmd $h "drive_backup -n disk ${TEST_IMG}.copy" "(qemu)" \
+ | _filter_imgfmt
+
silent=y qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs"
_send_qemu_cmd $h "info block-jobs" "No active jobs"
_send_qemu_cmd $h 'quit' ""
diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out
index 5a68de5c46..e580488216 100644
--- a/tests/qemu-iotests/028.out
+++ b/tests/qemu-iotests/028.out
@@ -468,6 +468,9 @@ No errors were found on the image.
block-backup
+Formatting 'TEST_DIR/t.IMGFMT.copy', fmt=IMGFMT size=4294968832
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) drive_backup -n disk TEST_DIR/t.IMGFMT.copy
(qemu) info block-jobs
No active jobs
=== IO: pattern 195
diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
index 66c5415abe..03a2ccf1e8 100755
--- a/tests/qemu-iotests/089
+++ b/tests/qemu-iotests/089
@@ -62,7 +62,8 @@ TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
$QEMU_IO -c 'write -P 42 0 512' -c 'write -P 23 512 512' \
-c 'write -P 66 1024 512' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IMG convert -f raw -O $IMGFMT "$TEST_IMG.base" "$TEST_IMG"
+_make_test_img $IMG_SIZE
+$QEMU_IMG convert -f raw -O $IMGFMT -n "$TEST_IMG.base" "$TEST_IMG"
$QEMU_IO_PROG --cache $CACHEMODE --aio $AIOMODE \
-c 'read -P 42 0 512' -c 'read -P 23 512 512' \
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
index 15682c2886..c53fc4823a 100644
--- a/tests/qemu-iotests/089.out
+++ b/tests/qemu-iotests/089.out
@@ -9,6 +9,7 @@ wrote 512/512 bytes at offset 512
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 512/512 bytes at offset 1024
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 512/512 bytes at offset 512
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 10/20] iotests/046: Avoid renaming images
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (8 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 09/20] iotests: Use convert -n in some cases Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 11/20] iotests: Derive image names from $TEST_IMG Max Reitz
` (11 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
This generally does not work on non-file protocols. It is better to
create the image with the final name from the start, and most tests do
this already. Let 046 follow suit.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/046 | 5 +++--
tests/qemu-iotests/046.out | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
index 88b3363c19..40a9f30087 100755
--- a/tests/qemu-iotests/046
+++ b/tests/qemu-iotests/046
@@ -47,6 +47,8 @@ size=128M
echo
echo "== creating backing file for COW tests =="
+TEST_IMG_SAVE=$TEST_IMG
+TEST_IMG="$TEST_IMG.base"
_make_test_img $size
backing_io()
@@ -67,8 +69,7 @@ backing_io()
backing_io 0 32 write | $QEMU_IO "$TEST_IMG" | _filter_qemu_io
-mv "$TEST_IMG" "$TEST_IMG.base"
-
+TEST_IMG=$TEST_IMG_SAVE
_make_test_img -b "$TEST_IMG.base" -F $IMGFMT 6G
echo
diff --git a/tests/qemu-iotests/046.out b/tests/qemu-iotests/046.out
index b022bcddd5..66ad987ab3 100644
--- a/tests/qemu-iotests/046.out
+++ b/tests/qemu-iotests/046.out
@@ -1,7 +1,7 @@
QA output created by 046
== creating backing file for COW tests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 65536
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 11/20] iotests: Derive image names from $TEST_IMG
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (9 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 10/20] iotests/046: Avoid renaming images Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 12/20] iotests/091: Use _cleanup_qemu instad of "wait" Max Reitz
` (10 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Avoid creating images with custom filenames in $TEST_DIR, because
non-file protocols may want to keep $TEST_IMG (and all other test
images) in some other directory.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/200 | 3 +--
tests/qemu-iotests/200.out | 4 ++--
tests/qemu-iotests/229 | 3 +--
tests/qemu-iotests/229.out | 6 +++---
4 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
index 59f7854b9f..a7aabbd032 100755
--- a/tests/qemu-iotests/200
+++ b/tests/qemu-iotests/200
@@ -44,8 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2 qed
_supported_proto file
-BACKING_IMG="${TEST_DIR}/backing.img"
-TEST_IMG="${TEST_DIR}/test.img"
+BACKING_IMG="$TEST_IMG.base"
TEST_IMG="$BACKING_IMG" _make_test_img 512M
_make_test_img -F $IMGFMT -b "$BACKING_IMG" 512M
diff --git a/tests/qemu-iotests/200.out b/tests/qemu-iotests/200.out
index a6776070e4..5883f16ac3 100644
--- a/tests/qemu-iotests/200.out
+++ b/tests/qemu-iotests/200.out
@@ -1,6 +1,6 @@
QA output created by 200
-Formatting 'TEST_DIR/backing.img', fmt=IMGFMT size=536870912
-Formatting 'TEST_DIR/test.img', fmt=IMGFMT size=536870912 backing_file=TEST_DIR/backing.img backing_fmt=IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=536870912
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
wrote 314572800/314572800 bytes at offset 512
300 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
index 89a5359f32..5f759fa587 100755
--- a/tests/qemu-iotests/229
+++ b/tests/qemu-iotests/229
@@ -51,8 +51,7 @@ _supported_os Linux
_unsupported_imgopts data_file
-DEST_IMG="$TEST_DIR/d.$IMGFMT"
-TEST_IMG="$TEST_DIR/b.$IMGFMT"
+DEST_IMG="$TEST_IMG.dest"
BLKDEBUG_CONF="$TEST_DIR/blkdebug.conf"
_make_test_img 2M
diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
index 4de6dfaa28..7eed393013 100644
--- a/tests/qemu-iotests/229.out
+++ b/tests/qemu-iotests/229.out
@@ -1,6 +1,6 @@
QA output created by 229
-Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=2097152
-Formatting 'TEST_DIR/d.IMGFMT', fmt=IMGFMT size=2097152
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2097152
+Formatting 'TEST_DIR/t.IMGFMT.dest', fmt=IMGFMT size=2097152
wrote 2097152/2097152 bytes at offset 0
2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
{'execute': 'qmp_capabilities'}
@@ -8,7 +8,7 @@ wrote 2097152/2097152 bytes at offset 0
=== Starting drive-mirror, causing error & stop ===
-{'execute': 'drive-mirror', 'arguments': {'device': 'testdisk', 'format': 'IMGFMT', 'target': 'blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/d.IMGFMT', 'sync': 'full', 'mode': 'existing', 'on-source-error': 'stop', 'on-target-error': 'stop' }}
+{'execute': 'drive-mirror', 'arguments': {'device': 'testdisk', 'format': 'IMGFMT', 'target': 'blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT.dest', 'sync': 'full', 'mode': 'existing', 'on-source-error': 'stop', 'on-target-error': 'stop' }}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "testdisk"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
{"return": {}}
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 12/20] iotests/091: Use _cleanup_qemu instad of "wait"
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (10 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 11/20] iotests: Derive image names from $TEST_IMG Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 13/20] iotests: Restrict some Python tests to file Max Reitz
` (9 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
If the test environment has some other child processes running (like a
storage daemon that provides a FUSE export), then "wait" will never
finish. Use wait=yes _cleanup_qemu instead.
(We need to discard the output so there is no change to the reference
output.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/091 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index 68fbfd777b..8a4ce5b7e2 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -96,7 +96,8 @@ _send_qemu_cmd $h2 'qemu-io disk flush' "(qemu)"
_send_qemu_cmd $h2 'quit' ""
_send_qemu_cmd $h1 'quit' ""
-wait
+wait=yes _cleanup_qemu >/dev/null
+
echo "Check image pattern"
${QEMU_IO} -c "read -P 0x22 0 4M" "${TEST_IMG}" | _filter_testdir | _filter_qemu_io
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 13/20] iotests: Restrict some Python tests to file
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (11 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 12/20] iotests/091: Use _cleanup_qemu instad of "wait" Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 14/20] iotests: Let _make_test_img guess $TEST_IMG_FILE Max Reitz
` (8 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Most Python tests are restricted to the file protocol (without
explicitly saying so), but these are the ones that would break
./check -fuse -qcow2.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/206 | 3 ++-
tests/qemu-iotests/242 | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index 11bc51f256..0a3ee5ef00 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -23,7 +23,8 @@
import iotests
from iotests import imgfmt
-iotests.script_initialize(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'],
+ supported_protocols=['file'])
iotests.verify_working_luks()
with iotests.FilePath('t.qcow2') as disk_path, \
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index 64f1bd95e4..a16de3085f 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -24,7 +24,8 @@ import struct
from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
file_path, img_info_log, log, filter_qemu_io
-iotests.script_initialize(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'],
+ supported_protocols=['file'])
disk = file_path('disk')
chunk = 256 * 1024
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 14/20] iotests: Let _make_test_img guess $TEST_IMG_FILE
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (12 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 13/20] iotests: Restrict some Python tests to file Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 15/20] iotests/287: Clean up subshell test image Max Reitz
` (7 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
When most iotests want to create a test image that is named differently
from the default $TEST_IMG, they do something like this:
TEST_IMG="$TEST_IMG.base" _make_test_img $options
This works fine with the "file" protocol, but not so much for anything
else: _make_test_img tries to create an image under $TEST_IMG_FILE
first, and only under $TEST_IMG if the former is not set; and on
everything but "file", $TEST_IMG_FILE is set.
There are two ways we can fix this: First, we could make all tests
adjust not only TEST_IMG, but also TEST_IMG_FILE if that is present
(e.g. with something like _set_test_img_suffix $suffix that would affect
not only TEST_IMG but also TEST_IMG_FILE, if necessary). This is a
pretty clean solution, and this is maybe what we should have done from
the start.
But it would also require changes to most existing bash tests. So the
alternative is this: Let _make_test_img see whether $TEST_IMG_FILE still
points to the original value. If so, it is possible that the caller has
adjusted $TEST_IMG but not $TEST_IMG_FILE. In such a case, we can (for
most protocols) derive the corresponding $TEST_IMG_FILE value from
$TEST_IMG value and thus work around what technically is the caller
misbehaving.
This second solution is less clean, but it is robust against people
keeping their old habit of adjusting TEST_IMG only, and requires much
less changes. So this patch implements it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/common.rc | 40 +++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 494490a272..23f46da2db 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -268,6 +268,7 @@ else
TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
fi
fi
+ORIG_TEST_IMG_FILE=$TEST_IMG_FILE
ORIG_TEST_IMG="$TEST_IMG"
if [ -z "$TEST_DIR" ]; then
@@ -330,6 +331,30 @@ _get_data_file()
| sed -e "s#\\\$TEST_IMG#$1#"
}
+# Translate a $TEST_IMG to its corresponding $TEST_IMG_FILE for
+# different protocols
+_test_img_to_test_img_file()
+{
+ case "$IMGPROTO" in
+ file)
+ echo "$1"
+ ;;
+
+ nfs)
+ echo "$1" | sed -e "s#nfs://127.0.0.1##"
+ ;;
+
+ ssh)
+ echo "$1" | \
+ sed -e "s#ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?##"
+ ;;
+
+ *)
+ return 1
+ ;;
+ esac
+}
+
_make_test_img()
{
# extra qemu-img options can be added by tests
@@ -343,10 +368,19 @@ _make_test_img()
local opts_param=false
local misc_params=()
- if [ -n "$TEST_IMG_FILE" ]; then
- img_name=$TEST_IMG_FILE
- else
+ if [ -z "$TEST_IMG_FILE" ]; then
img_name=$TEST_IMG
+ elif [ "$IMGOPTSSYNTAX" != "true" -a \
+ "$TEST_IMG_FILE" = "$ORIG_TEST_IMG_FILE" ]; then
+ # Handle cases of tests only updating TEST_IMG, but not TEST_IMG_FILE
+ img_name=$(_test_img_to_test_img_file "$TEST_IMG")
+ if [ "$?" != 0 ]; then
+ img_name=$TEST_IMG_FILE
+ fi
+ else
+ # $TEST_IMG_FILE is not the default value, so it definitely has been
+ # modified by the test
+ img_name=$TEST_IMG_FILE
fi
if [ -n "$IMGOPTS" ]; then
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 15/20] iotests/287: Clean up subshell test image
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (13 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 14/20] iotests: Let _make_test_img guess $TEST_IMG_FILE Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 16/20] storage-daemon: Call bdrv_close_all() on exit Max Reitz
` (6 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
287 creates an image in a subshell (thanks to the pipe) to see whether
that is possible with compression_type=zstd. If _make_test_img were to
modify any global state, this global state would then be lost before we
could cleanup the image.
When using FUSE as the test protocol, this global state is important, so
clean up the image before the state is lost.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/287 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287
index f98a4cadc1..036cc09e82 100755
--- a/tests/qemu-iotests/287
+++ b/tests/qemu-iotests/287
@@ -51,8 +51,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
CLUSTER_SIZE=65536
# Check if we can run this test.
-if IMGOPTS='compression_type=zstd' _make_test_img 64M |
- grep "Invalid parameter 'zstd'"; then
+output=$(_make_test_img -o 'compression_type=zstd' 64M; _cleanup_test_img)
+if echo "$output" | grep -q "Invalid parameter 'zstd'"; then
_notrun "ZSTD is disabled"
fi
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 16/20] storage-daemon: Call bdrv_close_all() on exit
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (14 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 15/20] iotests/287: Clean up subshell test image Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon Max Reitz
` (5 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Otherwise, exports and block devices are not properly shut down and
closed, unless the users explicitly issues blockdev-del and
block-export-del commands for each of them.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
storage-daemon/qemu-storage-daemon.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index e6157ff518..0b6d469751 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -324,6 +324,9 @@ int main(int argc, char *argv[])
main_loop_wait(false);
}
+ bdrv_drain_all_begin();
+ bdrv_close_all();
+
monitor_cleanup();
qemu_chr_cleanup();
user_creatable_cleanup();
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (15 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 16/20] storage-daemon: Call bdrv_close_all() on exit Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-10-15 11:27 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 18/20] iotests: Allow testing FUSE exports Max Reitz
` (4 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/check | 11 +++++++++++
tests/qemu-iotests/common.rc | 17 +++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e14a1f354d..467a7cf1b7 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -644,6 +644,17 @@ if [ -z $QEMU_NBD_PROG ]; then
fi
export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
+if [ -z "$QEMU_STGD_PROG" ]; then
+ if [ -x "$build_iotests/qemu-storage-daemon" ]; then
+ export QEMU_STGD_PROG="$build_iotests/qemu-storage-daemon"
+ elif [ -x "$build_root/storage-daemon/qemu-storage-daemon" ]; then
+ export QEMU_STGD_PROG="$build_root/storage-daemon/qemu-storage-daemon"
+ else
+ _init_error "qemu-storage-daemon not found"
+ fi
+fi
+export QEMU_STGD_PROG="$(type -p "$QEMU_STGD_PROG")"
+
if [ -x "$build_iotests/socket_scm_helper" ]
then
export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 23f46da2db..e4751d4985 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -124,6 +124,7 @@ fi
: ${VALGRIND_QEMU_IMG=$VALGRIND_QEMU}
: ${VALGRIND_QEMU_IO=$VALGRIND_QEMU}
: ${VALGRIND_QEMU_NBD=$VALGRIND_QEMU}
+: ${VALGRIND_QEMU_STGD=$VALGRIND_QEMU}
# The Valgrind own parameters may be set with
# its environment variable VALGRIND_OPTS, e.g.
@@ -211,6 +212,21 @@ _qemu_nbd_wrapper()
return $RETVAL
}
+_qemu_storage_daemon_wrapper()
+{
+ local VALGRIND_LOGFILE="${TEST_DIR}"/$$.valgrind
+ (
+ if [ -n "${QEMU_STGD_NEED_PID}" ]; then
+ echo $BASHPID > "${QEMU_TEST_DIR}/qemu-storage-daemon.pid"
+ fi
+ VALGRIND_QEMU="${VALGRIND_QEMU_STGD}" _qemu_proc_exec "${VALGRIND_LOGFILE}" \
+ "$QEMU_STGD_PROG" $QEMU_STGD_OPTIONS "$@"
+ )
+ RETVAL=$?
+ _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
+ return $RETVAL
+}
+
# Valgrind bug #409141 https://bugs.kde.org/show_bug.cgi?id=409141
# Until valgrind 3.16+ is ubiquitous, we must work around a hang in
# valgrind when issuing sigkill. Disable valgrind for this invocation.
@@ -223,6 +239,7 @@ export QEMU=_qemu_wrapper
export QEMU_IMG=_qemu_img_wrapper
export QEMU_IO=_qemu_io_wrapper
export QEMU_NBD=_qemu_nbd_wrapper
+export QEMU_STGD=_qemu_storage_daemon_wrapper
if [ "$IMGOPTSSYNTAX" = "true" ]; then
DRIVER="driver=$IMGFMT"
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 18/20] iotests: Allow testing FUSE exports
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (16 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-10-15 11:43 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 19/20] iotests: Enable fuse for many tests Max Reitz
` (3 subsequent siblings)
21 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
This pretends FUSE exports are a kind of protocol. As such, they are
always tested under the format node. This is probably the best way to
test them, actually, because this will generate more I/O load and more
varied patterns.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/check | 6 ++
tests/qemu-iotests/common.filter | 5 +-
tests/qemu-iotests/common.rc | 124 +++++++++++++++++++++++++++++++
3 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 467a7cf1b7..07232138d7 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -270,6 +270,7 @@ image protocol options
-rbd test rbd
-sheepdog test sheepdog
-nbd test nbd
+ -fuse test fuse
-ssh test ssh
-nfs test nfs
@@ -382,6 +383,11 @@ testlist options
xpand=false
;;
+ -fuse)
+ IMGPROTO=fuse
+ xpand=false
+ ;;
+
-ssh)
IMGPROTO=ssh
xpand=false
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 838ed15793..172ea5752e 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -44,7 +44,8 @@ _filter_qom_path()
_filter_testdir()
{
$SED -e "s#$TEST_DIR/#TEST_DIR/#g" \
- -e "s#$SOCK_DIR/#SOCK_DIR/#g"
+ -e "s#$SOCK_DIR/#SOCK_DIR/#g" \
+ -e "s#SOCK_DIR/fuse-#TEST_DIR/#g"
}
# replace occurrences of the actual IMGFMT value with IMGFMT
@@ -127,6 +128,7 @@ _filter_img_create_filenames()
-e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$SOCK_DIR#SOCK_DIR#g" \
+ -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
-e "s#$IMGFMT#IMGFMT#g" \
-e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
-e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
@@ -227,6 +229,7 @@ _filter_img_info()
-e "s#$IMGFMT#IMGFMT#g" \
-e 's#nbd://127.0.0.1:[0-9]\\+$#TEST_DIR/t.IMGFMT#g' \
-e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g' \
+ -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
-e "/encrypted: yes/d" \
-e "/cluster_size: [0-9]\\+/d" \
-e "/table_size: [0-9]\\+/d" \
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index e4751d4985..e17f813f06 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -257,6 +257,9 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
TEST_IMG="$DRIVER,file.driver=nbd,file.type=unix"
TEST_IMG="$TEST_IMG,file.path=$SOCK_DIR/nbd"
+ elif [ "$IMGPROTO" = "fuse" ]; then
+ TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+ TEST_IMG="$DRIVER,file.filename=$SOCK_DIR/fuse-t.$IMGFMT"
elif [ "$IMGPROTO" = "ssh" ]; then
TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE"
@@ -273,6 +276,9 @@ else
elif [ "$IMGPROTO" = "nbd" ]; then
TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
TEST_IMG="nbd+unix:///?socket=$SOCK_DIR/nbd"
+ elif [ "$IMGPROTO" = "fuse" ]; then
+ TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+ TEST_IMG="$SOCK_DIR/fuse-t.$IMGFMT"
elif [ "$IMGPROTO" = "ssh" ]; then
TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR"
@@ -288,6 +294,9 @@ fi
ORIG_TEST_IMG_FILE=$TEST_IMG_FILE
ORIG_TEST_IMG="$TEST_IMG"
+FUSE_PIDS=()
+FUSE_EXPORTS=()
+
if [ -z "$TEST_DIR" ]; then
TEST_DIR=$PWD/scratch
fi
@@ -357,6 +366,10 @@ _test_img_to_test_img_file()
echo "$1"
;;
+ fuse)
+ echo "$1" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#"
+ ;;
+
nfs)
echo "$1" | sed -e "s#nfs://127.0.0.1##"
;;
@@ -385,6 +398,11 @@ _make_test_img()
local opts_param=false
local misc_params=()
+ if [[ $IMGPROTO == fuse && $TEST_IMG == $SOCK_DIR/fuse-* ]]; then
+ # The caller may be trying to overwrite an existing image
+ _rm_test_img "$TEST_IMG"
+ fi
+
if [ -z "$TEST_IMG_FILE" ]; then
img_name=$TEST_IMG
elif [ "$IMGOPTSSYNTAX" != "true" -a \
@@ -469,11 +487,105 @@ _make_test_img()
eval "$QEMU_NBD -v -t -k '$SOCK_DIR/nbd' -f $IMGFMT -e 42 -x '' $TEST_IMG_FILE >/dev/null &"
sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
fi
+
+ if [ $IMGPROTO = "fuse" -a -f "$img_name" ]; then
+ local export_mp
+ local pid
+ local pidfile
+ local timeout
+
+ export_mp=$(echo "$img_name" | sed -e "s#$TEST_DIR/#$SOCK_DIR/fuse-#")
+ if ! echo "$export_mp" | grep -q "^$SOCK_DIR"; then
+ echo 'Cannot use FUSE exports with images outside of TEST_DIR' >&2
+ return 1
+ fi
+
+ touch "$export_mp"
+ rm -f "$SOCK_DIR/fuse-output"
+
+ # Usually, users would export formatted nodes. But we present fuse as a
+ # protocol-level driver here, so we have to leave the format to the
+ # client.
+ QEMU_STGD_NEED_PID=y $QEMU_STGD \
+ --blockdev file,node-name=export-node,filename=$img_name,discard=unmap \
+ --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on \
+ &
+
+ pidfile="$QEMU_TEST_DIR/qemu-storage-daemon.pid"
+
+ # Wait for the PID file
+ while [ ! -f "$pidfile" ]; do
+ sleep 0.5
+ done
+
+ pid=$(cat "$pidfile")
+ rm -f "$pidfile"
+
+ FUSE_PIDS+=($pid)
+ FUSE_EXPORTS+=("$export_mp")
+ fi
}
_rm_test_img()
{
local img=$1
+
+ if [[ $IMGPROTO == fuse && $img == $SOCK_DIR/fuse-* ]]; then
+ # Drop a FUSE export
+ local df_output
+ local i
+ local image_file
+ local index=''
+ local timeout
+
+ for i in "${!FUSE_EXPORTS[@]}"; do
+ if [ "${FUSE_EXPORTS[i]}" = "$img" ]; then
+ index=$i
+ break
+ fi
+ done
+
+ if [ -z "$index" ]; then
+ # Probably gone already
+ return 0
+ fi
+
+ kill "${FUSE_PIDS[index]}"
+
+ # Wait until the mount is gone
+ timeout=10 # *0.5 s
+ while true; do
+ # Will show the mount point; if the mount is still there,
+ # it will be $img.
+ df_output=$(df -T "$img" 2>/dev/null)
+
+ # But df may also show an error ("Transpoint endpoint not
+ # connected"), so retry in such cases
+ if [ -n "$df_output" ]; then
+ if ! echo "$df_output" | grep -q "$img"; then
+ break
+ fi
+ fi
+
+ sleep 0.5
+
+ timeout=$((timeout - 1))
+ if [ "$timeout" = 0 ]; then
+ echo 'Failed to take down FUSE export' >&2
+ return 1
+ fi
+ done
+
+ rm -f "$img"
+
+ unset "FUSE_PIDS[$index]"
+ unset "FUSE_EXPORTS[$index]"
+
+ image_file=$(echo "$img" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#")
+ _rm_test_img "$image_file"
+ return
+ fi
+
if [ "$IMGFMT" = "vmdk" ]; then
# Remove all the extents for vmdk
"$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \
@@ -496,6 +608,17 @@ _cleanup_test_img()
rm -f "$TEST_IMG_FILE"
;;
+ fuse)
+ local mp
+
+ for mp in "${FUSE_EXPORTS[@]}"; do
+ _rm_test_img "$mp"
+ done
+
+ FUSE_PIDS=()
+ FUSE_EXPORTS=()
+ ;;
+
file)
_rm_test_img "$TEST_DIR/t.$IMGFMT"
_rm_test_img "$TEST_DIR/t.$IMGFMT.orig"
@@ -562,6 +685,7 @@ _img_info()
sed -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
-e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
+ -e "s#$SOCK_DIR/fuse-#TEST_DIR/#g" \
-e "s#$IMGFMT#IMGFMT#g" \
-e "/^disk size:/ D" \
-e "/actual-size/ D" | \
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 19/20] iotests: Enable fuse for many tests
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (17 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 18/20] iotests: Allow testing FUSE exports Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 20/20] iotests/308: Add test for FUSE exports Max Reitz
` (2 subsequent siblings)
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
Many tests (that do not support generic protocols) can run just fine
with FUSE-exported images, so allow them to. Note that this is no
attempt at being definitely complete. There are some tests that might
be modified to run on FUSE, but this patch still skips them. This patch
only tries to pick the rather low-hanging fruits.
Note that 221 and 250 only pass when .lseek is correctly implemented,
which is only possible with a libfuse that is 3.8 or newer.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/025 | 2 +-
tests/qemu-iotests/026 | 2 +-
tests/qemu-iotests/028 | 2 +-
tests/qemu-iotests/031 | 2 +-
tests/qemu-iotests/034 | 2 +-
tests/qemu-iotests/036 | 2 +-
tests/qemu-iotests/037 | 2 +-
tests/qemu-iotests/038 | 2 +-
tests/qemu-iotests/039 | 2 +-
tests/qemu-iotests/046 | 2 +-
tests/qemu-iotests/050 | 2 +-
tests/qemu-iotests/054 | 2 +-
tests/qemu-iotests/060 | 2 +-
tests/qemu-iotests/071 | 2 +-
tests/qemu-iotests/079 | 2 +-
tests/qemu-iotests/080 | 2 +-
tests/qemu-iotests/089 | 2 +-
tests/qemu-iotests/090 | 2 +-
tests/qemu-iotests/091 | 2 +-
tests/qemu-iotests/095 | 2 +-
tests/qemu-iotests/097 | 2 +-
tests/qemu-iotests/098 | 2 +-
tests/qemu-iotests/102 | 2 +-
tests/qemu-iotests/103 | 2 +-
tests/qemu-iotests/106 | 2 +-
tests/qemu-iotests/107 | 2 +-
tests/qemu-iotests/108 | 2 +-
tests/qemu-iotests/111 | 2 +-
tests/qemu-iotests/112 | 2 +-
tests/qemu-iotests/115 | 2 +-
tests/qemu-iotests/117 | 2 +-
tests/qemu-iotests/120 | 2 +-
tests/qemu-iotests/121 | 2 +-
tests/qemu-iotests/127 | 2 +-
tests/qemu-iotests/133 | 2 +-
tests/qemu-iotests/137 | 2 +-
tests/qemu-iotests/138 | 2 +-
tests/qemu-iotests/140 | 2 +-
tests/qemu-iotests/154 | 2 +-
tests/qemu-iotests/161 | 2 +-
tests/qemu-iotests/171 | 2 +-
tests/qemu-iotests/175 | 2 +-
tests/qemu-iotests/176 | 2 +-
tests/qemu-iotests/177 | 2 +-
tests/qemu-iotests/179 | 2 +-
tests/qemu-iotests/183 | 2 +-
tests/qemu-iotests/186 | 2 +-
tests/qemu-iotests/187 | 2 +-
tests/qemu-iotests/191 | 2 +-
tests/qemu-iotests/195 | 2 +-
tests/qemu-iotests/200 | 2 +-
tests/qemu-iotests/204 | 2 +-
tests/qemu-iotests/214 | 2 +-
tests/qemu-iotests/217 | 2 +-
tests/qemu-iotests/220 | 2 +-
tests/qemu-iotests/221 | 2 +-
tests/qemu-iotests/229 | 2 +-
tests/qemu-iotests/247 | 2 +-
tests/qemu-iotests/249 | 2 +-
tests/qemu-iotests/250 | 2 +-
tests/qemu-iotests/252 | 2 +-
tests/qemu-iotests/265 | 2 +-
tests/qemu-iotests/268 | 2 +-
tests/qemu-iotests/272 | 2 +-
tests/qemu-iotests/273 | 2 +-
tests/qemu-iotests/279 | 2 +-
tests/qemu-iotests/286 | 2 +-
tests/qemu-iotests/287 | 2 +-
tests/qemu-iotests/289 | 2 +-
tests/qemu-iotests/290 | 2 +-
tests/qemu-iotests/291 | 2 +-
tests/qemu-iotests/292 | 2 +-
tests/qemu-iotests/293 | 2 +-
tests/qemu-iotests/294 | 2 +-
tests/qemu-iotests/305 | 2 +-
75 files changed, 75 insertions(+), 75 deletions(-)
diff --git a/tests/qemu-iotests/025 b/tests/qemu-iotests/025
index e05d833452..1569d912f4 100755
--- a/tests/qemu-iotests/025
+++ b/tests/qemu-iotests/025
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.pattern
_supported_fmt raw qcow2 qed luks
-_supported_proto file sheepdog rbd nfs
+_supported_proto file sheepdog rbd nfs fuse
echo "=== Creating image"
echo
diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index b9713eb591..9ecc5880b1 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Currently only qcow2 supports rebasing
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_default_cache_mode writethrough
_supported_cache_modes writethrough none
# The refcount table tests expect a certain minimum width for refcount entries
diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028
index 864dc4a4e2..57d34aae99 100755
--- a/tests/qemu-iotests/028
+++ b/tests/qemu-iotests/028
@@ -46,7 +46,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Any format supporting backing files except vmdk and qcow which do not support
# smaller backing files.
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# Choose a size that is not necessarily a cluster size multiple for image
diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031
index 646ecd593f..2bcbc5886e 100755
--- a/tests/qemu-iotests/031
+++ b/tests/qemu-iotests/031
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# We want to test compat=0.10, which does not support external data
# files or refcount widths other than 16
_unsupported_imgopts data_file 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
diff --git a/tests/qemu-iotests/034 b/tests/qemu-iotests/034
index ac2d687c71..08f7aea6d5 100755
--- a/tests/qemu-iotests/034
+++ b/tests/qemu-iotests/034
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow qcow2 vmdk qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_unsupported_imgopts "subformat=monolithicFlat" \
"subformat=twoGbMaxExtentFlat" \
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index cf522de7a1..6b82638080 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# Only qcow2v3 and later supports feature bits;
# qcow2.py does not support external data files;
# this test requires a cluster size large enough for the feature table
diff --git a/tests/qemu-iotests/037 b/tests/qemu-iotests/037
index e1187ac24a..bb893c43dc 100755
--- a/tests/qemu-iotests/037
+++ b/tests/qemu-iotests/037
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow qcow2 vmdk qed
-_supported_proto file
+_supported_proto file fuse
_unsupported_imgopts "subformat=monolithicFlat" \
"subformat=twoGbMaxExtentFlat" \
"subformat=twoGbMaxExtentSparse" \
diff --git a/tests/qemu-iotests/038 b/tests/qemu-iotests/038
index a253231f5b..30f1f73c25 100755
--- a/tests/qemu-iotests/038
+++ b/tests/qemu-iotests/038
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
CLUSTER_SIZE=2M
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 42f6503138..ad3867c3fc 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_default_cache_mode writethrough
_supported_cache_modes writethrough
diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
index 40a9f30087..ed6fae3529 100755
--- a/tests/qemu-iotests/046
+++ b/tests/qemu-iotests/046
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# data_file does not support compressed clusters
_unsupported_imgopts data_file
diff --git a/tests/qemu-iotests/050 b/tests/qemu-iotests/050
index 4b0a390c43..741bdb610e 100755
--- a/tests/qemu-iotests/050
+++ b/tests/qemu-iotests/050
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
echo
echo "== Creating images =="
diff --git a/tests/qemu-iotests/054 b/tests/qemu-iotests/054
index a8905b60d0..40922db2b1 100755
--- a/tests/qemu-iotests/054
+++ b/tests/qemu-iotests/054
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
echo
echo "creating too large image (1 EB)"
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 94c0d5accc..4b81d1aa51 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -46,7 +46,7 @@ _filter_io_error()
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# These tests only work for compat=1.1 images without an external
# data file with refcount_bits=16
diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 18fe9054b0..49faae6684 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_require_drivers blkdebug blkverify
# blkdebug can only inject errors on bs->file, not on the data_file,
# so thie test does not work with external data files
diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
index 3642b51feb..0f0d94a2ac 100755
--- a/tests/qemu-iotests/079
+++ b/tests/qemu-iotests/079
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file nfs
+_supported_proto file nfs fuse
# Some containers (e.g. non-x86 on Travis) do not allow large files
_require_large_file 4G
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 7588c63b6c..bda8617c38 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# - Internal snapshots are (currently) impossible with refcount_bits=1,
# and generally impossible with external data files
diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
index 03a2ccf1e8..f0929b64c0 100755
--- a/tests/qemu-iotests/089
+++ b/tests/qemu-iotests/089
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# Because anything other than 16 would change the output of qemu_io -c info
_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
diff --git a/tests/qemu-iotests/090 b/tests/qemu-iotests/090
index 1246e4f910..87e872ebf4 100755
--- a/tests/qemu-iotests/090
+++ b/tests/qemu-iotests/090
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file nfs
+_supported_proto file nfs fuse
# External data files do not support compressed clusters
_unsupported_imgopts data_file
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index 8a4ce5b7e2..8dee168bf6 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_supported_cache_modes writethrough none writeback
_default_cache_mode none writeback
diff --git a/tests/qemu-iotests/095 b/tests/qemu-iotests/095
index 77a5f0f3f5..7604ae6966 100755
--- a/tests/qemu-iotests/095
+++ b/tests/qemu-iotests/095
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
size_smaller=5M
size_larger=100M
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index d910a8b107..1837d4e8e0 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Any format supporting backing files and bdrv_make_empty
_supported_fmt qcow qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098
index f2ccdd7909..a35ce7205e 100755
--- a/tests/qemu-iotests/098
+++ b/tests/qemu-iotests/098
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.pattern
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# The code path we want to test here only works for compat=1.1 images;
# blkdebug can only inject errors on bs->file, so external data files
# do not work with this test
diff --git a/tests/qemu-iotests/102 b/tests/qemu-iotests/102
index b898df436f..2cc3efd1ed 100755
--- a/tests/qemu-iotests/102
+++ b/tests/qemu-iotests/102
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
IMG_SIZE=64K
diff --git a/tests/qemu-iotests/103 b/tests/qemu-iotests/103
index 8c1ebe0443..220481db4c 100755
--- a/tests/qemu-iotests/103
+++ b/tests/qemu-iotests/103
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file nfs
+_supported_proto file nfs fuse
# Internal snapshots are (currently) impossible with refcount_bits=1,
# and generally impossible with external data files
_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
diff --git a/tests/qemu-iotests/106 b/tests/qemu-iotests/106
index a20659d443..20ad7bd5a2 100755
--- a/tests/qemu-iotests/106
+++ b/tests/qemu-iotests/106
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt raw
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# in kB
diff --git a/tests/qemu-iotests/107 b/tests/qemu-iotests/107
index 268ba27688..d24829ccf9 100755
--- a/tests/qemu-iotests/107
+++ b/tests/qemu-iotests/107
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file nfs
+_supported_proto file nfs fuse
IMG_SIZE=64K
diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
index 5f7076fba4..ba67748bdf 100755
--- a/tests/qemu-iotests/108
+++ b/tests/qemu-iotests/108
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# This test directly modifies a refblock so it relies on refcount_bits being 16;
# and the low-level modification it performs are not tuned for external data
diff --git a/tests/qemu-iotests/111 b/tests/qemu-iotests/111
index 3b43d1bd83..bd839a39f4 100755
--- a/tests/qemu-iotests/111
+++ b/tests/qemu-iotests/111
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qed qcow qcow2 vmdk
-_supported_proto file
+_supported_proto file fuse
_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
_make_test_img -b "$TEST_IMG.inexistent"
diff --git a/tests/qemu-iotests/112 b/tests/qemu-iotests/112
index 20ff5c224a..6e413f5651 100755
--- a/tests/qemu-iotests/112
+++ b/tests/qemu-iotests/112
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# This test will set refcount_bits on its own which would conflict with the
# manual setting; compat will be overridden as well;
# and external data files do not work well with our refcount testing
diff --git a/tests/qemu-iotests/115 b/tests/qemu-iotests/115
index d254b18342..7f53987d1b 100755
--- a/tests/qemu-iotests/115
+++ b/tests/qemu-iotests/115
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# This test relies on refcounts being 64 bits wide (which does not work with
# compat=0.10)
_unsupported_imgopts 'refcount_bits=\([^6]\|.\([^4]\|$\)\)' 'compat=0.10'
diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117
index f37b34f8b1..9039555ac4 100755
--- a/tests/qemu-iotests/117
+++ b/tests/qemu-iotests/117
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_make_test_img 64k
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index 2931a7550f..45c55c1c01 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt generic
-_supported_proto file
+_supported_proto file fuse
_unsupported_fmt luks
_require_drivers raw
diff --git a/tests/qemu-iotests/121 b/tests/qemu-iotests/121
index 90ea0db737..8357ce089a 100755
--- a/tests/qemu-iotests/121
+++ b/tests/qemu-iotests/121
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# Refcount structures are used much differently with external data
# files
diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index e2ac2f5536..77fdfd0205 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_require_devices virtio-scsi scsi-hd
diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133
index 4070fd9457..bc82d8ebd7 100755
--- a/tests/qemu-iotests/133
+++ b/tests/qemu-iotests/133
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
TEST_IMG="$TEST_IMG.base" _make_test_img 64M
_make_test_img -b "$TEST_IMG.base" -F $IMGFMT
diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
index 7ae86892f7..de555a91c9 100755
--- a/tests/qemu-iotests/137
+++ b/tests/qemu-iotests/137
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# We are going to use lazy-refcounts
_unsupported_imgopts 'compat=0.10'
diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
index 1d5b0bed6d..e87a64eb89 100755
--- a/tests/qemu-iotests/138
+++ b/tests/qemu-iotests/138
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This tests qcow2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# With an external data file, data clusters are not refcounted
# (so qemu-img check would not do much);
diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
index 443751455e..97b6e55810 100755
--- a/tests/qemu-iotests/140
+++ b/tests/qemu-iotests/140
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt generic
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_make_test_img 64k
diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
index 7f1c0d9bd9..34a1c051b6 100755
--- a/tests/qemu-iotests/154
+++ b/tests/qemu-iotests/154
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
CLUSTER_SIZE=4k
diff --git a/tests/qemu-iotests/161 b/tests/qemu-iotests/161
index bbf7dbbc5c..4fb7d0cbf0 100755
--- a/tests/qemu-iotests/161
+++ b/tests/qemu-iotests/161
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Any format implementing BlockDriver.bdrv_change_backing_file
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
IMG_SIZE=1M
diff --git a/tests/qemu-iotests/171 b/tests/qemu-iotests/171
index 341064a1c6..f3582edb10 100755
--- a/tests/qemu-iotests/171
+++ b/tests/qemu-iotests/171
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt raw
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index c3c2aed653..21a77a2bf5 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -71,7 +71,7 @@ EOF
. ./common.filter
_supported_fmt raw
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_default_cache_mode none
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
index 2565ff12ee..5ce3b27069 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -45,7 +45,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This test is specific to qcow2
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# Persistent dirty bitmaps require compat=1.1;
# Internal snapshots forbid using an external data file
diff --git a/tests/qemu-iotests/177 b/tests/qemu-iotests/177
index 5d4a77a6ab..595bfd4236 100755
--- a/tests/qemu-iotests/177
+++ b/tests/qemu-iotests/177
@@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# tests specific to compat=1.1.
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
CLUSTER_SIZE=1M
size=128M
diff --git a/tests/qemu-iotests/179 b/tests/qemu-iotests/179
index 11a20cb1bf..7ada04c641 100755
--- a/tests/qemu-iotests/179
+++ b/tests/qemu-iotests/179
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# v2 images can't mark clusters as zero
diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index acdbefa310..d889a3b19c 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_os Linux FreeBSD NetBSD
_supported_fmt qcow2 raw qed quorum
-_supported_proto file
+_supported_proto file fuse
size=64M
_make_test_img $size
diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index 3ea0442d44..0db25b0e68 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_require_drivers null-co
if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
diff --git a/tests/qemu-iotests/187 b/tests/qemu-iotests/187
index c6e1dc57a0..f262d83e3a 100755
--- a/tests/qemu-iotests/187
+++ b/tests/qemu-iotests/187
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
size=64M
_make_test_img $size
diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191
index d17462e1e4..95a891350d 100755
--- a/tests/qemu-iotests/191
+++ b/tests/qemu-iotests/191
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# An external data file would change the query-named-block-nodes output
_unsupported_imgopts data_file
diff --git a/tests/qemu-iotests/195 b/tests/qemu-iotests/195
index 2351d55fe1..967af5b7b5 100755
--- a/tests/qemu-iotests/195
+++ b/tests/qemu-iotests/195
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
do_run_qemu()
{
diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
index a7aabbd032..046539154f 100755
--- a/tests/qemu-iotests/200
+++ b/tests/qemu-iotests/200
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
BACKING_IMG="$TEST_IMG.base"
diff --git a/tests/qemu-iotests/204 b/tests/qemu-iotests/204
index 6770fa9b96..536bb8b534 100755
--- a/tests/qemu-iotests/204
+++ b/tests/qemu-iotests/204
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# This test assumes that discard leaves zero clusters; see test 177 for
# other tests that also work in older images
_unsupported_imgopts 'compat=0.10'
diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index af677d90b8..75ae7a14b5 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# Repairing the corrupted image requires qemu-img check to store a
# refcount up to 3, which requires at least two refcount bits.
diff --git a/tests/qemu-iotests/217 b/tests/qemu-iotests/217
index d89116ccad..7385342498 100755
--- a/tests/qemu-iotests/217
+++ b/tests/qemu-iotests/217
@@ -36,7 +36,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This test is specific to qcow2
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# This test needs clusters with at least a refcount of 2 so that
# OFLAG_COPIED is not set. refcount_bits=1 is therefore unsupported.
diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220
index a9259b7127..9ba3b3fdcb 100755
--- a/tests/qemu-iotests/220
+++ b/tests/qemu-iotests/220
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.pattern
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# To use a different refcount width but 16 bits we need compat=1.1,
# and external data files do not support compressed clusters.
diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221
index 0e9096fec7..ca62b3baa1 100755
--- a/tests/qemu-iotests/221
+++ b/tests/qemu-iotests/221
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt raw
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_default_cache_mode writeback
diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
index 5f759fa587..273ac2472d 100755
--- a/tests/qemu-iotests/229
+++ b/tests/qemu-iotests/229
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Needs backing file and backing format support
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# blkdebug can only inject errors on bs->file, so external data files
# do not work with this test
diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
index 87e37b39e2..6cf2679750 100755
--- a/tests/qemu-iotests/247
+++ b/tests/qemu-iotests/247
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Requires backing files and .bdrv_change_backing_file support
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
size=128M
diff --git a/tests/qemu-iotests/249 b/tests/qemu-iotests/249
index a9aa9303eb..29453b8c90 100755
--- a/tests/qemu-iotests/249
+++ b/tests/qemu-iotests/249
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Any format implementing BlockDriver.bdrv_change_backing_file
_supported_fmt qcow2 qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
IMG_SIZE=1M
diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
index 9bb6b94d74..3df275c76b 100755
--- a/tests/qemu-iotests/250
+++ b/tests/qemu-iotests/250
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# This test does not make much sense with external data files
_unsupported_imgopts data_file
diff --git a/tests/qemu-iotests/252 b/tests/qemu-iotests/252
index 6662f4c9de..1d74afff99 100755
--- a/tests/qemu-iotests/252
+++ b/tests/qemu-iotests/252
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# zero cluster support
_supported_fmt qcow2
_unsupported_imgopts 'compat=0.10'
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
CLUSTER_SIZE=65536
diff --git a/tests/qemu-iotests/265 b/tests/qemu-iotests/265
index 00f2ec769e..0e800fb524 100755
--- a/tests/qemu-iotests/265
+++ b/tests/qemu-iotests/265
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# qcow2-specific test
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
echo '--- Writing to the image ---'
diff --git a/tests/qemu-iotests/268 b/tests/qemu-iotests/268
index 78c3f4db3a..ddf4312284 100755
--- a/tests/qemu-iotests/268
+++ b/tests/qemu-iotests/268
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
echo
echo "== Required alignment larger than cluster size =="
diff --git a/tests/qemu-iotests/272 b/tests/qemu-iotests/272
index c2f782d47b..de475bf6f0 100755
--- a/tests/qemu-iotests/272
+++ b/tests/qemu-iotests/272
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This is a qcow2 regression test
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# External data files do not support compression;
# We need an exact cluster size (2M) and refcount width (2) so we can
diff --git a/tests/qemu-iotests/273 b/tests/qemu-iotests/273
index 41ffbf39c4..79b4ab4b05 100755
--- a/tests/qemu-iotests/273
+++ b/tests/qemu-iotests/273
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# This is a qcow2 regression test
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# External data files would add nodes to the block graph, so it would
# not match the reference output
diff --git a/tests/qemu-iotests/279 b/tests/qemu-iotests/279
index 5515d4ed01..2a6315cf17 100755
--- a/tests/qemu-iotests/279
+++ b/tests/qemu-iotests/279
@@ -36,7 +36,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
# Backing files are required...
_supported_fmt qcow qcow2 vmdk qed
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_unsupported_imgopts "subformat=monolithicFlat" \
"subformat=twoGbMaxExtentFlat" \
diff --git a/tests/qemu-iotests/286 b/tests/qemu-iotests/286
index f14445ba4a..f64e0eccea 100755
--- a/tests/qemu-iotests/286
+++ b/tests/qemu-iotests/286
@@ -35,7 +35,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.qemu
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# Internal snapshots are (currently) impossible with refcount_bits=1,
# and generally impossible with external data files
_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287
index 036cc09e82..3bb383fd4b 100755
--- a/tests/qemu-iotests/287
+++ b/tests/qemu-iotests/287
@@ -32,7 +32,7 @@ status=1 # failure is the default!
# This tests qocw2-specific low-level functionality
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_unsupported_imgopts 'compat=0.10' data_file
diff --git a/tests/qemu-iotests/289 b/tests/qemu-iotests/289
index 1c11d4030e..fe69bde1eb 100755
--- a/tests/qemu-iotests/289
+++ b/tests/qemu-iotests/289
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.pattern
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
# This is a v3-exclusive test;
# As for data_file, error paths often very much depend on whether
# there is an external data file or not; so we create one exactly when
diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
index 01ee14dcfb..35c38d4f80 100755
--- a/tests/qemu-iotests/290
+++ b/tests/qemu-iotests/290
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_unsupported_imgopts 'compat=0.10' refcount_bits data_file
diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
index 1e0bb76959..34ad5f0c30 100755
--- a/tests/qemu-iotests/291
+++ b/tests/qemu-iotests/291
@@ -36,7 +36,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.nbd
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_require_command QEMU_NBD
# compat=0.10 does not support bitmaps
diff --git a/tests/qemu-iotests/292 b/tests/qemu-iotests/292
index 83ab19231d..3ae2772e3b 100755
--- a/tests/qemu-iotests/292
+++ b/tests/qemu-iotests/292
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
# We need qemu-img map to show the file where the data is allocated,
# but with an external data file, it will show that instead of the
diff --git a/tests/qemu-iotests/293 b/tests/qemu-iotests/293
index f86fe3b413..3363bf07f0 100755
--- a/tests/qemu-iotests/293
+++ b/tests/qemu-iotests/293
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2 luks
-_supported_proto file #TODO
+_supported_proto file fuse #TODO
_require_working_luks
QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
diff --git a/tests/qemu-iotests/294 b/tests/qemu-iotests/294
index 9c95ed8c9a..87da35db49 100755
--- a/tests/qemu-iotests/294
+++ b/tests/qemu-iotests/294
@@ -34,7 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt luks
-_supported_proto file #TODO
+_supported_proto file fuse #TODO
QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
diff --git a/tests/qemu-iotests/305 b/tests/qemu-iotests/305
index 768818af4a..5a415eb2a4 100755
--- a/tests/qemu-iotests/305
+++ b/tests/qemu-iotests/305
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.filter
_supported_fmt qcow2
-_supported_proto file
+_supported_proto file fuse
_supported_os Linux
_unsupported_imgopts cluster_size refcount_bits extended_l2 compat=0.10 data_file
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v2 20/20] iotests/308: Add test for FUSE exports
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (18 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 19/20] iotests: Enable fuse for many tests Max Reitz
@ 2020-09-22 10:49 ` Max Reitz
2020-09-22 15:58 ` [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Daniel P. Berrangé
2020-10-15 12:01 ` Kevin Wolf
21 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 10:49 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Max Reitz
We have good coverage of the normal I/O paths now, but what remains is a
test that tests some more special cases: Exporting an image on itself
(thus turning a formatted image into a raw one), some error cases, and
non-writable and non-growable exports.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/308 | 339 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/308.out | 97 +++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 437 insertions(+)
create mode 100755 tests/qemu-iotests/308
create mode 100644 tests/qemu-iotests/308.out
diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308
new file mode 100755
index 0000000000..b30f4400f6
--- /dev/null
+++ b/tests/qemu-iotests/308
@@ -0,0 +1,339 @@
+#!/usr/bin/env bash
+#
+# Test FUSE exports (in ways that are not captured by the generic
+# tests)
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=$(basename "$0")
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_qemu
+ _cleanup_test_img
+ rmdir "$EXT_MP" 2>/dev/null
+ rm -f "$EXT_MP"
+ rm -f "$COPIED_IMG"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# Generic format, but needs a plain filename
+_supported_fmt generic
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+ _unsupported_fmt $IMGFMT
+fi
+# We need the image to have exactly the specified size, and VPC does
+# not allow that by default
+_unsupported_fmt vpc
+
+_supported_proto file # We create the FUSE export manually
+_supported_os Linux # We need /dev/urandom
+
+# $1: Export ID
+# $2: Options (beyond the node-name and ID)
+# $3: Expected return value (defaults to 'return')
+# $4: Node to export (defaults to 'node-format')
+fuse_export_add()
+{
+ _send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'block-export-add',
+ 'arguments': {
+ 'type': 'fuse',
+ 'id': '$1',
+ 'node-name': '${4:-node-format}',
+ $2
+ } }" \
+ "${3:-return}" \
+ | _filter_imgfmt
+}
+
+# $1: Export ID
+fuse_export_del()
+{
+ _send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'block-export-del',
+ 'arguments': {
+ 'id': '$1'
+ } }" \
+ 'return'
+
+ _send_qemu_cmd $QEMU_HANDLE \
+ '' \
+ 'BLOCK_EXPORT_DELETED'
+}
+
+# Return the length of the protocol file
+# $1: Protocol node export mount point
+# $2: Original file (to compare)
+get_proto_len()
+{
+ len1=$(stat -c '%s' "$1")
+ len2=$(stat -c '%s' "$2")
+
+ if [ "$len1" != "$len2" ]; then
+ echo 'ERROR: Length of export and original differ:' >&2
+ echo "$len1 != $len2" >&2
+ else
+ echo '(OK: Lengths of export and original are the same)' >&2
+ fi
+
+ echo "$len1"
+}
+
+COPIED_IMG="$TEST_IMG.copy"
+EXT_MP="$TEST_IMG.fuse"
+
+echo '=== Set up ==='
+
+# Create image with random data
+_make_test_img 64M
+$QEMU_IO -c 'write -s /dev/urandom 0 64M' "$TEST_IMG" | _filter_qemu_io
+
+_launch_qemu
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'qmp_capabilities'}" \
+ 'return'
+
+# Separate blockdev-add calls for format and protocol so we can remove
+# the format layer later on
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': 'file',
+ 'node-name': 'node-protocol',
+ 'filename': '$TEST_IMG'
+ } }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': '$IMGFMT',
+ 'node-name': 'node-format',
+ 'file': 'node-protocol'
+ } }" \
+ 'return'
+
+echo
+echo '=== Mountpoint not present ==='
+
+rmdir "$EXT_MP" 2>/dev/null
+rm -f "$EXT_MP"
+output=$(fuse_export_add 'export-err' "'mountpoint': '$EXT_MP'" error)
+
+if echo "$output" | grep -q "Invalid parameter 'fuse'"; then
+ _notrun 'No FUSE support'
+fi
+
+echo "$output"
+
+echo
+echo '=== Mountpoint is a directory ==='
+
+mkdir "$EXT_MP"
+fuse_export_add 'export-err' "'mountpoint': '$EXT_MP'" error
+rmdir "$EXT_MP"
+
+echo
+echo '=== Mountpoint is a regular file ==='
+
+touch "$EXT_MP"
+fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP'"
+
+# Check that the export presents the same data as the original image
+$QEMU_IMG compare -f raw -F $IMGFMT -U "$EXT_MP" "$TEST_IMG"
+
+echo
+echo '=== Mount over existing file ==='
+
+# This is the coolest feature of FUSE exports: You can transparently
+# make images in any format appear as raw images
+fuse_export_add 'export-img' "'mountpoint': '$TEST_IMG'"
+
+# Accesses both exports at the same time, so we get a concurrency test
+$QEMU_IMG compare -f raw -F raw -U "$EXT_MP" "$TEST_IMG"
+
+# Just to be sure, we later want to compare the data offline. Also,
+# this allows us to see that cp works without complaining.
+# (This is not a given, because cp will expect a short read at EOF.
+# Internally, qemu does not allow short reads, so we have to check
+# whether the FUSE export driver lets them work.)
+cp "$TEST_IMG" "$COPIED_IMG"
+
+# $TEST_IMG will be in mode 0400 because it is read-only; we are going
+# to write to the copy, so make it writable
+chmod 0600 "$COPIED_IMG"
+
+echo
+echo '=== Double export ==='
+
+# We have already seen that exporting a node twice works fine, but you
+# cannot export anything twice on the same mount point. The reason is
+# that qemu has to stat the given mount point, and this would have to
+# be answered by the same qemu instance if it already has an export
+# there. However, it cannot answer the stat because it is itself
+# caught up in that same stat.
+fuse_export_add 'export-err' "'mountpoint': '$EXT_MP'" error
+
+echo
+echo '=== Remove export ==='
+
+# Double-check that $EXT_MP appears as a non-empty file (the raw image)
+$QEMU_IMG info -f raw "$EXT_MP" | grep 'virtual size'
+
+fuse_export_del 'export-mp'
+
+# See that the file appears empty again
+$QEMU_IMG info -f raw "$EXT_MP" | grep 'virtual size'
+
+echo
+echo '=== Writable export ==='
+
+fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP', 'writable': true"
+
+# Check that writing to the read-only export fails
+$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" | _filter_qemu_io
+
+# But here it should work
+$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$EXT_MP" | _filter_qemu_io
+
+# (Adjust the copy, too)
+$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$COPIED_IMG" | _filter_qemu_io
+
+echo
+echo '=== Resizing exports ==='
+
+# Here, we need to export the protocol node -- the format layer may
+# not be growable, simply because the format does not support it.
+
+# Remove all exports and the format node first so permissions will not
+# get in the way
+fuse_export_del 'export-mp'
+fuse_export_del 'export-img'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-del',
+ 'arguments': {
+ 'node-name': 'node-format'
+ } }" \
+ 'return'
+
+# Now export the protocol node
+fuse_export_add \
+ 'export-mp' \
+ "'mountpoint': '$EXT_MP', 'writable': true" \
+ 'return' \
+ 'node-protocol'
+
+echo
+echo '--- Try growing non-growable export ---'
+
+# Get the current size so we can write beyond the EOF
+orig_len=$(get_proto_len "$EXT_MP" "$TEST_IMG")
+orig_disk_usage=$(stat -c '%b' "$TEST_IMG")
+
+# Should fail (exports are non-growable by default)
+# (Note that qemu-io can never write beyond the EOF, so we have to use
+# dd here)
+dd if=/dev/zero of="$EXT_MP" bs=1 count=64k seek=$orig_len 2>&1 \
+ | _filter_testdir | _filter_imgfmt
+
+echo
+echo '--- Resize export ---'
+
+# But we can truncate it explicitly; even with fallocate
+fallocate -o "$orig_len" -l 64k "$EXT_MP"
+
+new_len=$(get_proto_len "$EXT_MP" "$TEST_IMG")
+if [ "$new_len" != "$((orig_len + 65536))" ]; then
+ echo 'ERROR: Unexpected post-truncate image size:'
+ echo "$new_len != $((orig_len + 65536))"
+else
+ echo 'OK: Post-truncate image size is as expected'
+fi
+
+new_disk_usage=$(stat -c '%b' "$TEST_IMG")
+if [ "$new_disk_usage" -gt "$orig_disk_usage" ]; then
+ echo 'OK: Disk usage grew with fallocate'
+else
+ echo 'ERROR: Disk usage did not grow despite fallocate:'
+ echo "$orig_disk_usage => $new_disk_usage"
+fi
+
+echo
+echo '--- Try growing growable export ---'
+
+# Now export as growable
+fuse_export_del 'export-mp'
+fuse_export_add \
+ 'export-mp' \
+ "'mountpoint': '$EXT_MP', 'writable': true, 'growable': true" \
+ 'return' \
+ 'node-protocol'
+
+# Now we should be able to write beyond the EOF
+dd if=/dev/zero of="$EXT_MP" bs=1 count=64k seek=$new_len 2>&1 \
+ | _filter_testdir | _filter_imgfmt
+
+new_len=$(get_proto_len "$EXT_MP" "$TEST_IMG")
+if [ "$new_len" != "$((orig_len + 131072))" ]; then
+ echo 'ERROR: Unexpected post-grow image size:'
+ echo "$new_len != $((orig_len + 131072))"
+else
+ echo 'OK: Post-grow image size is as expected'
+fi
+
+echo
+echo '--- Shrink export ---'
+
+# Now go back to the original size
+truncate -s "$orig_len" "$EXT_MP"
+
+new_len=$(get_proto_len "$EXT_MP" "$TEST_IMG")
+if [ "$new_len" != "$orig_len" ]; then
+ echo 'ERROR: Unexpected post-truncate image size:'
+ echo "$new_len != $orig_len"
+else
+ echo 'OK: Post-truncate image size is as expected'
+fi
+
+echo
+echo '=== Tear down ==='
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'quit'}" \
+ 'return'
+
+wait=yes _cleanup_qemu
+
+echo
+echo '=== Compare copy with original ==='
+
+$QEMU_IMG compare -f raw -F $IMGFMT "$COPIED_IMG" "$TEST_IMG"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out
new file mode 100644
index 0000000000..b93aceed2e
--- /dev/null
+++ b/tests/qemu-iotests/308.out
@@ -0,0 +1,97 @@
+QA output created by 308
+=== Set up ===
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{'execute': 'qmp_capabilities'}
+{"return": {}}
+{'execute': 'blockdev-add', 'arguments': { 'driver': 'file', 'node-name': 'node-protocol', 'filename': 'TEST_DIR/t.IMGFMT' } }
+{"return": {}}
+{'execute': 'blockdev-add', 'arguments': { 'driver': 'IMGFMT', 'node-name': 'node-format', 'file': 'node-protocol' } }
+{"return": {}}
+
+=== Mountpoint not present ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-err', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse' } }
+{"error": {"class": "GenericError", "desc": "Failed to stat 'TEST_DIR/t.IMGFMT.fuse': No such file or directory"}}
+
+=== Mountpoint is a directory ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-err', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse' } }
+{"error": {"class": "GenericError", "desc": "'TEST_DIR/t.IMGFMT.fuse' is not a regular file"}}
+
+=== Mountpoint is a regular file ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-mp', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse' } }
+{"return": {}}
+Images are identical.
+
+=== Mount over existing file ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-img', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT' } }
+{"return": {}}
+Images are identical.
+
+=== Double export ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-err', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse' } }
+{"error": {"class": "GenericError", "desc": "There already is a FUSE export on 'TEST_DIR/t.IMGFMT.fuse'"}}
+
+=== Remove export ===
+virtual size: 64 MiB (67108864 bytes)
+{'execute': 'block-export-del', 'arguments': { 'id': 'export-mp' } }
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export-mp"}}
+virtual size: 0 B (0 bytes)
+
+=== Writable export ===
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-mp', 'node-name': 'node-format', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true } }
+{"return": {}}
+write failed: Permission denied
+wrote 65536/65536 bytes at offset 1048576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 1048576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Resizing exports ===
+{'execute': 'block-export-del', 'arguments': { 'id': 'export-mp' } }
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export-mp"}}
+{'execute': 'block-export-del', 'arguments': { 'id': 'export-img' } }
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export-img"}}
+{'execute': 'blockdev-del', 'arguments': { 'node-name': 'node-format' } }
+{"return": {}}
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-mp', 'node-name': 'node-protocol', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true } }
+{"return": {}}
+
+--- Try growing non-growable export ---
+(OK: Lengths of export and original are the same)
+dd: error writing 'TEST_DIR/t.IMGFMT.fuse': Input/output error
+1+0 records in
+0+0 records out
+
+--- Resize export ---
+(OK: Lengths of export and original are the same)
+OK: Post-truncate image size is as expected
+OK: Disk usage grew with fallocate
+
+--- Try growing growable export ---
+{'execute': 'block-export-del', 'arguments': { 'id': 'export-mp' } }
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export-mp"}}
+{'execute': 'block-export-add', 'arguments': { 'type': 'fuse', 'id': 'export-mp', 'node-name': 'node-protocol', 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true, 'growable': true } }
+{"return": {}}
+65536+0 records in
+65536+0 records out
+(OK: Lengths of export and original are the same)
+OK: Post-grow image size is as expected
+
+--- Shrink export ---
+(OK: Lengths of export and original are the same)
+OK: Post-truncate image size is as expected
+
+=== Tear down ===
+{'execute': 'quit'}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export-mp"}}
+
+=== Compare copy with original ===
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9e4f7c0153..8c5f622393 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -315,3 +315,4 @@
304 rw quick
305 rw quick
307 rw quick export
+308 rw
--
2.26.2
^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 10:49 ` [PATCH v2 01/20] configure: Detect libfuse Max Reitz
@ 2020-09-22 11:14 ` Thomas Huth
2020-09-22 11:21 ` Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 46+ messages in thread
From: Thomas Huth @ 2020-09-22 11:14 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi
On 22/09/2020 12.49, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> configure | 34 ++++++++++++++++++++++++++++++++++
> meson.build | 6 ++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/configure b/configure
> index ce27eafb0a..21c31e4694 100755
> --- a/configure
> +++ b/configure
> @@ -538,6 +538,7 @@ meson=""
> ninja=""
> skip_meson=no
> gettext=""
> +fuse=""
>
> bogus_os="no"
> malloc_trim=""
> @@ -1621,6 +1622,10 @@ for opt do
> ;;
> --disable-libdaxctl) libdaxctl=no
> ;;
> + --enable-fuse) fuse=yes
> + ;;
> + --disable-fuse) fuse=no
> + ;;
> *)
> echo "ERROR: unknown option $opt"
> echo "Try '$0 --help' for more information"
> @@ -1945,6 +1950,7 @@ disabled with --disable-FEATURE, default is enabled if available:
> xkbcommon xkbcommon support
> rng-none dummy RNG, avoid using /dev/(u)random and getrandom()
> libdaxctl libdaxctl support
> + fuse fuse block device export
>
> NOTE: The object files are built at the place where configure is launched
> EOF
> @@ -6206,6 +6212,28 @@ but not implemented on your system"
> fi
> fi
>
> +##########################################
> +# FUSE support
> +
> +if test "$fuse" != "no"; then
> + cat > $TMPC <<EOF
> +#define FUSE_USE_VERSION 31
> +#include <fuse.h>
> +#include <fuse_lowlevel.h>
> +int main(void) { return 0; }
> +EOF
> + fuse_cflags=$(pkg-config --cflags fuse3)
> + fuse_libs=$(pkg-config --libs fuse3)
> + if compile_prog "$fuse_cflags" "$fuse_libs"; then
> + fuse=yes
> + else
> + if test "$fuse" = "yes"; then
> + feature_not_found "fuse"
> + fi
> + fuse=no
> + fi
> +fi
Could you turn this immediately into a meson test instead? See e.g.
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07112.html as
an example for how to do this.
Thomas
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 11:14 ` Thomas Huth
@ 2020-09-22 11:21 ` Paolo Bonzini
2020-09-22 11:46 ` Max Reitz
2020-09-22 15:37 ` Max Reitz
2 siblings, 0 replies; 46+ messages in thread
From: Paolo Bonzini @ 2020-09-22 11:21 UTC (permalink / raw)
To: Thomas Huth, Max Reitz, qemu-block
Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 22/09/20 13:14, Thomas Huth wrote:
> On 22/09/2020 12.49, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> configure | 34 ++++++++++++++++++++++++++++++++++
>> meson.build | 6 ++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/configure b/configure
>> index ce27eafb0a..21c31e4694 100755
>> --- a/configure
>> +++ b/configure
>> @@ -538,6 +538,7 @@ meson=""
>> ninja=""
>> skip_meson=no
>> gettext=""
>> +fuse=""
>>
>> bogus_os="no"
>> malloc_trim=""
>> @@ -1621,6 +1622,10 @@ for opt do
>> ;;
>> --disable-libdaxctl) libdaxctl=no
>> ;;
>> + --enable-fuse) fuse=yes
>> + ;;
>> + --disable-fuse) fuse=no
>> + ;;
>> *)
>> echo "ERROR: unknown option $opt"
>> echo "Try '$0 --help' for more information"
>> @@ -1945,6 +1950,7 @@ disabled with --disable-FEATURE, default is enabled if available:
>> xkbcommon xkbcommon support
>> rng-none dummy RNG, avoid using /dev/(u)random and getrandom()
>> libdaxctl libdaxctl support
>> + fuse fuse block device export
>>
>> NOTE: The object files are built at the place where configure is launched
>> EOF
>> @@ -6206,6 +6212,28 @@ but not implemented on your system"
>> fi
>> fi
>>
>> +##########################################
>> +# FUSE support
>> +
>> +if test "$fuse" != "no"; then
>> + cat > $TMPC <<EOF
>> +#define FUSE_USE_VERSION 31
>> +#include <fuse.h>
>> +#include <fuse_lowlevel.h>
>> +int main(void) { return 0; }
>> +EOF
>> + fuse_cflags=$(pkg-config --cflags fuse3)
>> + fuse_libs=$(pkg-config --libs fuse3)
>> + if compile_prog "$fuse_cflags" "$fuse_libs"; then
>> + fuse=yes
>> + else
>> + if test "$fuse" = "yes"; then
>> + feature_not_found "fuse"
>> + fi
>> + fuse=no
>> + fi
>> +fi
>
> Could you turn this immediately into a meson test instead? See e.g.
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07112.html as
> an example for how to do this.
For pkg-config in fact it's even simpler and documented in
docs/devel/build-system.rst.
Paolo
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 11:14 ` Thomas Huth
2020-09-22 11:21 ` Paolo Bonzini
@ 2020-09-22 11:46 ` Max Reitz
2020-09-22 15:37 ` Max Reitz
2 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-22 11:46 UTC (permalink / raw)
To: Thomas Huth, qemu-block
Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 247 bytes --]
On 22.09.20 13:14, Thomas Huth wrote:
[...]
> Could you turn this immediately into a meson test instead? See e.g.
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07112.html as
> an example for how to do this.
Sure!
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 11:14 ` Thomas Huth
2020-09-22 11:21 ` Paolo Bonzini
2020-09-22 11:46 ` Max Reitz
@ 2020-09-22 15:37 ` Max Reitz
2020-09-22 15:45 ` Paolo Bonzini
2 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-09-22 15:37 UTC (permalink / raw)
To: Thomas Huth, qemu-block
Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 573 bytes --]
On 22.09.20 13:14, Thomas Huth wrote:
[...]
> Could you turn this immediately into a meson test instead? See e.g.
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07112.html as
> an example for how to do this.
Done (I think). Until I send v3, the updated version lives here:
https://git.xanclic.moe/XanClic/qemu/commits/branch/fuse-exports-next
https://github.com/XanClic/qemu/commits/fuse-exports-next
I’ve replaced the compile checks (on test programs) by simpler version
checks (>=3.1 for libfuse itself, >=3.8 for fuse-lseek).
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 01/20] configure: Detect libfuse
2020-09-22 15:37 ` Max Reitz
@ 2020-09-22 15:45 ` Paolo Bonzini
0 siblings, 0 replies; 46+ messages in thread
From: Paolo Bonzini @ 2020-09-22 15:45 UTC (permalink / raw)
To: Max Reitz, Thomas Huth, qemu-block
Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 514 bytes --]
On 22/09/20 17:37, Max Reitz wrote:
> On 22.09.20 13:14, Thomas Huth wrote:
>
> [...]
>
>> Could you turn this immediately into a meson test instead? See e.g.
>> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07112.html as
>> an example for how to do this.
>
> Done (I think). Until I send v3, the updated version lives here:
>
> https://git.xanclic.moe/XanClic/qemu/commits/branch/fuse-exports-next
> https://github.com/XanClic/qemu/commits/fuse-exports-next
Looks good.
Paolo
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (19 preceding siblings ...)
2020-09-22 10:49 ` [PATCH v2 20/20] iotests/308: Add test for FUSE exports Max Reitz
@ 2020-09-22 15:58 ` Daniel P. Berrangé
2020-09-23 7:21 ` Max Reitz
2020-09-23 9:08 ` Stefan Hajnoczi
2020-10-15 12:01 ` Kevin Wolf
21 siblings, 2 replies; 46+ messages in thread
From: Daniel P. Berrangé @ 2020-09-22 15:58 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, Stefan Hajnoczi, qemu-devel, qemu-block
On Tue, Sep 22, 2020 at 12:49:12PM +0200, Max Reitz wrote:
> Based-on: <20200907182011.521007-1-kwolf@redhat.com>
> (“block/export: Add infrastructure and QAPI for block exports”)
>
> (Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
>
> v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
>
> Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
> Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
>
>
> Hi,
>
> Ever since I found out that you can mount FUSE filesystems on regular
> files (not just directories), I had the idea of adding FUSE block
> exports to qemu where you can export block nodes as raw images. The
> best thing is that you’d be able to mount an image on itself, so
> whatever format it may be in, qemu lets it appear as a raw image (and
> you can then use regular tools like dd on it).
>
> The performance is quite bad so far, but we can always try to improve it
> if the need arises. For now I consider it mostly a cute feature to get
> easy access to the raw contents of image files in any format (without
> requiring root rights).
Aside from the iotests, so you forsee any particular use cases
where this feature is desirable / important ?
Looking at it from a security POV, I'm not thrilled about the
idea of granting QEMU permission to use the mount syscall for
seccomp or SELinux. IOW, I expect this feature won't be something
we want to expose in QEMU guests managed by libvirt, which would
limit how widely it can be used.
QEMU can export NBD. Would it make sense to do this as an NBD
client ? There's already https://libguestfs.org/nbdfuse.1.html
but IIUC that exposes it as a file within a dir. Presumably
it is not too hard to make it support exposing it directly as
a file too.
I wonder how performance compares between your native FUSE
impl in QEMU vs NBD FUSE ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
2020-09-22 15:58 ` [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Daniel P. Berrangé
@ 2020-09-23 7:21 ` Max Reitz
2020-09-23 9:08 ` Stefan Hajnoczi
1 sibling, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-09-23 7:21 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 3761 bytes --]
On 22.09.20 17:58, Daniel P. Berrangé wrote:
> On Tue, Sep 22, 2020 at 12:49:12PM +0200, Max Reitz wrote:
>> Based-on: <20200907182011.521007-1-kwolf@redhat.com>
>> (“block/export: Add infrastructure and QAPI for block exports”)
>>
>> (Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
>>
>> v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
>>
>> Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
>> Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
>>
>>
>> Hi,
>>
>> Ever since I found out that you can mount FUSE filesystems on regular
>> files (not just directories), I had the idea of adding FUSE block
>> exports to qemu where you can export block nodes as raw images. The
>> best thing is that you’d be able to mount an image on itself, so
>> whatever format it may be in, qemu lets it appear as a raw image (and
>> you can then use regular tools like dd on it).
>>
>> The performance is quite bad so far, but we can always try to improve it
>> if the need arises. For now I consider it mostly a cute feature to get
>> easy access to the raw contents of image files in any format (without
>> requiring root rights).
>
> Aside from the iotests, so you forsee any particular use cases
> where this feature is desirable / important ?
No.
I implemented this feature for fun last year (when I realized that FUSE
allows regular files to be mount points), and I got positive reactions.
I assumed others would find it as nice as me to be able to quickly
access an image file without requiring root rights (and then device file
accesses), or setting up an NBD chain.
(Though it should be noted that when I first came up with the feature,
nbdfuse did not exist yet.)
(It should also be noted that my original idea was to have a new
executable qemu-blkfuse that would basically allow you to invoke
“qemu-blkfuse $img”, and then $img would appear as a raw image. To me,
that appeared very useful because it was so simple. I admit that the
current proposal, which relies on the storage-daemon, has none of that
simplicity. But if that’s the problem that prevents this from being
considered useful, I’m sure we (I) can figure something out. Perhaps a
simple script, bundled with qemu, that can generate -blockdev
invocations based on the result of file(1).)
> Looking at it from a security POV, I'm not thrilled about the
> idea of granting QEMU permission to use the mount syscall for
> seccomp or SELinux. IOW, I expect this feature won't be something
> we want to expose in QEMU guests managed by libvirt, which would
> limit how widely it can be used.
I don’t expect this to be used through QEMU very much, but through the
storage daemon. I assume that for the storage daemon, the permissions
can effectively be fine-tuned for each export, because you can “just”
launch another instance.
> QEMU can export NBD. Would it make sense to do this as an NBD
> client ? There's already https://libguestfs.org/nbdfuse.1.html
> but IIUC that exposes it as a file within a dir. Presumably
> it is not too hard to make it support exposing it directly as
> a file too.
I don’t like that idea very much, because my main gripe with the current
state of my proposal is that it’s more cumbersome than
“qemu-blkfuse $img”. Adding more indirections won’t make it simpler.
> I wonder how performance compares between your native FUSE
> impl in QEMU vs NBD FUSE ?
Last year, I tried various ways of improving performance and nothing
really amounted to much. So in the end I settled for a simple and naive
implementation, for it to be improved in case anyone cares for it.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
2020-09-22 15:58 ` [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Daniel P. Berrangé
2020-09-23 7:21 ` Max Reitz
@ 2020-09-23 9:08 ` Stefan Hajnoczi
1 sibling, 0 replies; 46+ messages in thread
From: Stefan Hajnoczi @ 2020-09-23 9:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, mszeredi, qemu-block, afrosi, qemu-devel, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]
On Tue, Sep 22, 2020 at 04:58:38PM +0100, Daniel P. Berrangé wrote:
> On Tue, Sep 22, 2020 at 12:49:12PM +0200, Max Reitz wrote:
> > Based-on: <20200907182011.521007-1-kwolf@redhat.com>
> > (“block/export: Add infrastructure and QAPI for block exports”)
> >
> > (Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
> >
> > v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
> >
> > Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
> > Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
> >
> >
> > Hi,
> >
> > Ever since I found out that you can mount FUSE filesystems on regular
> > files (not just directories), I had the idea of adding FUSE block
> > exports to qemu where you can export block nodes as raw images. The
> > best thing is that you’d be able to mount an image on itself, so
> > whatever format it may be in, qemu lets it appear as a raw image (and
> > you can then use regular tools like dd on it).
> >
> > The performance is quite bad so far, but we can always try to improve it
> > if the need arises. For now I consider it mostly a cute feature to get
> > easy access to the raw contents of image files in any format (without
> > requiring root rights).
>
> Aside from the iotests, so you forsee any particular use cases
> where this feature is desirable / important ?
Alice Frosi is working on a qemu-storage-daemon-based project where the
FUSE export type is useful. In this case qemu-storage-daemon is used
stand-alone without a guest or libvirt directly involved. The goal is
just to export disk images and how they are consumed is the user's
responsibility (processes, containers, guests).
> Looking at it from a security POV, I'm not thrilled about the
> idea of granting QEMU permission to use the mount syscall for
> seccomp or SELinux. IOW, I expect this feature won't be something
> we want to expose in QEMU guests managed by libvirt, which would
> limit how widely it can be used.
I have CCed Miklos Szeredi, the Linux FUSE maintainer, to check what the
options are for unprivileged mounting of a FUSE file system:
1. libfuse invokes open("/dev/fuse") + mount()
2. libfuse spawns the fusermount3 suid root helper
3. Any other options? D-Bus? etc
> QEMU can export NBD. Would it make sense to do this as an NBD
> client ? There's already https://libguestfs.org/nbdfuse.1.html
> but IIUC that exposes it as a file within a dir. Presumably
> it is not too hard to make it support exposing it directly as
> a file too.
>
> I wonder how performance compares between your native FUSE
> impl in QEMU vs NBD FUSE ?
NBD exports are useful for networks but shouldn't preclude other export
types from being merged just because they can be implemented on top of
NBD. Native export types are simpler to manage and have less performance
overhead than stacking additional tools on top of NBD.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-09-22 10:49 ` [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE Max Reitz
@ 2020-10-15 8:57 ` Kevin Wolf
2020-10-15 14:46 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 8:57 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> block-export-add type=fuse allows mounting block graph nodes via FUSE on
> some existing regular file. That file should then appears like a raw
> disk image, and accesses to it result in accesses to the exported BDS.
>
> Right now, we only implement the necessary block export functions to set
> it up and shut it down. We do not implement any access functions, so
> accessing the mount point only results in errors. This will be
> addressed by a followup patch.
>
> We keep a hash table of exported mount points, because we want to be
> able to detect when users try to use a mount point twice. This is
> because we invoke stat() to check whether the given mount point is a
> regular file, but if that file is served by ourselves (because it is
> already used as a mount point), then this stat() would have to be served
> by ourselves, too, which is impossible to do while we (as the caller)
> are waiting for it to settle. Therefore, keep track of mount point
> paths to at least catch the most obvious instances of that problem.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qapi/block-export.json | 24 +++-
> include/block/fuse.h | 30 +++++
> block.c | 1 +
> block/export/export.c | 4 +
> block/export/fuse.c | 253 +++++++++++++++++++++++++++++++++++++++
> block/export/meson.build | 1 +
> 6 files changed, 311 insertions(+), 2 deletions(-)
> create mode 100644 include/block/fuse.h
> create mode 100644 block/export/fuse.c
>
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index 0c045dfe7b..cb5bd54cbf 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -174,6 +174,21 @@
> ##
> { 'command': 'nbd-server-stop' }
>
> +##
> +# @BlockExportOptionsFuse:
> +#
> +# Options for exporting a block graph node on some (file) mountpoint
> +# as a raw image.
> +#
> +# @mountpoint: Path on which to export the block device via FUSE.
> +# This must point to an existing regular file.
> +#
> +# Since: 5.2
> +##
> +{ 'struct': 'BlockExportOptionsFuse',
> + 'data': { 'mountpoint': 'str' },
> + 'if': 'defined(CONFIG_FUSE)' }
> +
> ##
> # @BlockExportType:
> #
> @@ -181,10 +196,13 @@
> #
> # @nbd: NBD export
> #
> +# @fuse: Fuse export (since: 5.2)
s/Fuse/FUSE/?
> +#
> # Since: 4.2
> ##
> { 'enum': 'BlockExportType',
> - 'data': [ 'nbd' ] }
> + 'data': [ 'nbd',
> + { 'name': 'fuse', 'if': 'defined(CONFIG_FUSE)' } ] }
>
> ##
> # @BlockExportOptions:
> @@ -213,7 +231,9 @@
> '*writethrough': 'bool' },
> 'discriminator': 'type',
> 'data': {
> - 'nbd': 'BlockExportOptionsNbd'
> + 'nbd': 'BlockExportOptionsNbd',
> + 'fuse': { 'type': 'BlockExportOptionsFuse',
> + 'if': 'defined(CONFIG_FUSE)' }
> } }
>
> ##
> diff --git a/include/block/fuse.h b/include/block/fuse.h
> new file mode 100644
> index 0000000000..ffa91fe364
> --- /dev/null
> +++ b/include/block/fuse.h
> @@ -0,0 +1,30 @@
> +/*
> + * Present a block device as a raw image through FUSE
> + *
> + * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 or later of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef BLOCK_FUSE_H
> +#define BLOCK_FUSE_H
> +
> +#ifdef CONFIG_FUSE
> +
> +#include "block/export.h"
> +
> +extern const BlockExportDriver blk_exp_fuse;
> +
> +#endif /* CONFIG_FUSE */
> +
> +#endif
> diff --git a/block.c b/block.c
> index acde53f92a..7bf45f6ede 100644
> --- a/block.c
> +++ b/block.c
> @@ -26,6 +26,7 @@
> #include "block/trace.h"
> #include "block/block_int.h"
> #include "block/blockjob.h"
> +#include "block/fuse.h"
> #include "block/nbd.h"
> #include "block/qdict.h"
> #include "qemu/error-report.h"
> diff --git a/block/export/export.c b/block/export/export.c
> index 64d39a6934..ba866d6ba7 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -16,6 +16,7 @@
> #include "block/block.h"
> #include "sysemu/block-backend.h"
> #include "block/export.h"
> +#include "block/fuse.h"
> #include "block/nbd.h"
> #include "qapi/error.h"
> #include "qapi/qapi-commands-block-export.h"
> @@ -24,6 +25,9 @@
>
> static const BlockExportDriver *blk_exp_drivers[] = {
> &blk_exp_nbd,
> +#ifdef CONFIG_FUSE
> + &blk_exp_fuse,
> +#endif
> };
>
> /* Only accessed from the main thread */
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> new file mode 100644
> index 0000000000..75f11d2514
> --- /dev/null
> +++ b/block/export/fuse.c
> @@ -0,0 +1,253 @@
> +/*
> + * Present a block device as a raw image through FUSE
> + *
> + * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 or later of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define FUSE_USE_VERSION 31
> +
> +#include "qemu/osdep.h"
> +#include "block/aio.h"
> +#include "block/block.h"
> +#include "block/export.h"
> +#include "block/fuse.h"
> +#include "block/qapi.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-block.h"
> +#include "sysemu/block-backend.h"
> +
> +#include <fuse.h>
> +#include <fuse_lowlevel.h>
> +
> +
> +typedef struct FuseExport {
> + BlockExport common;
> +
> + struct fuse_session *fuse_session;
> + struct fuse_buf fuse_buf;
> + bool mounted, fd_handler_set_up;
> +
> + char *mountpoint;
> + bool writable;
> +} FuseExport;
> +
> +static GHashTable *exports;
> +static const struct fuse_lowlevel_ops fuse_ops;
> +
> +static void fuse_export_shutdown(BlockExport *exp);
> +static void fuse_export_delete(BlockExport *exp);
> +
> +static void init_fuse(void);
> +static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
> + Error **errp);
> +static void read_from_fuse_export(void *opaque);
> +
> +static bool is_regular_file(const char *path, Error **errp);
> +
> +
> +static int fuse_export_create(BlockExport *blk_exp,
> + BlockExportOptions *blk_exp_args,
> + Error **errp)
> +{
> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
> + BlockExportOptionsFuse *args = &blk_exp_args->u.fuse;
> + int ret;
> +
> + assert(blk_exp_args->type == BLOCK_EXPORT_TYPE_FUSE);
> +
> + init_fuse();
> +
> + /*
> + * It is important to do this check before calling is_regular_file() --
> + * that function will do a stat(), which we would have to handle if we
> + * already exported something on @mountpoint. But we cannot, because
> + * we are currently caught up here.
> + * (Note that ideally we would want to resolve relative paths here,
> + * but bdrv_make_absolute_filename() might do the wrong thing for
> + * paths that contain colons, and realpath() would resolve symlinks,
> + * which we do not want: The mount point is not going to be the
> + * symlink's destination, but the link itself.)
> + * So this will not catch all potential clashes, but hopefully at
> + * least the most common one of specifying exactly the same path
> + * string twice.
> + */
> + if (g_hash_table_contains(exports, args->mountpoint)) {
> + error_setg(errp, "There already is a FUSE export on '%s'",
> + args->mountpoint);
> + ret = -EEXIST;
> + goto fail;
> + }
I guess the proper solution would be to make block export creation
asynchronous so that we can actually serve that request in the
background.
Something for another day...
> +
> + if (!is_regular_file(args->mountpoint, errp)) {
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + exp->mountpoint = g_strdup(args->mountpoint);
> + exp->writable = blk_exp_args->writable;
> +
> + ret = setup_fuse_export(exp, args->mountpoint, errp);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + g_hash_table_insert(exports, g_strdup(args->mountpoint), NULL);
> + return 0;
> +
> +fail:
> + fuse_export_shutdown(blk_exp);
> + fuse_export_delete(blk_exp);
Why fuse_export_shutdown()? The only thing to possibly undo at
this point is allocating exp->mountpoint, which is freed in
fuse_export_delete().
Or can setup_fuse_export() fail without cleaning up? I hope not.
> + return ret;
> +}
> +
> +/**
> + * Ensure that the global FUSE context is set up.
> + */
> +static void init_fuse(void)
> +{
> + if (exports) {
> + return;
> + }
> +
> + exports = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
> +}
> +
> +/**
> + * Create exp->fuse_session and mount it.
> + */
> +static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
> + Error **errp)
> +{
> + const char *fuse_argv[2];
> + struct fuse_args fuse_args;
> + int ret;
> +
> + fuse_argv[0] = ""; /* Dummy program name */
> + fuse_argv[1] = NULL;
> + fuse_args = (struct fuse_args)FUSE_ARGS_INIT(1, (char **)fuse_argv);
> +
> + exp->fuse_session = fuse_session_new(&fuse_args, &fuse_ops,
> + sizeof(fuse_ops), exp);
> + if (!exp->fuse_session) {
> + error_setg(errp, "Failed to set up FUSE session");
> + return -EIO;
> + }
> +
> + ret = fuse_session_mount(exp->fuse_session, mountpoint);
> + if (ret < 0) {
> + error_setg(errp, "Failed to mount FUSE session to export");
> + return -EIO;
Oh, it can. :-(
I think it would be better to call fuse_export_shutdown() here so that
the function doesn't change the state when it returns failure.
> + }
> + exp->mounted = true;
> +
> + aio_set_fd_handler(exp->common.ctx,
> + fuse_session_fd(exp->fuse_session), true,
> + read_from_fuse_export, NULL, NULL, exp);
> + exp->fd_handler_set_up = true;
> +
> + return 0;
> +}
> +
> +/**
> + * Callback to be invoked when the FUSE session FD can be read from.
> + * (This is basically the FUSE event loop.)
> + */
> +static void read_from_fuse_export(void *opaque)
> +{
> + FuseExport *exp = opaque;
> + int ret;
> +
> + blk_exp_ref(&exp->common);
> +
> + ret = fuse_session_receive_buf(exp->fuse_session, &exp->fuse_buf);
The fuse_session_loop() implementation seems to imply that we should
retry on EINTR.
> + if (ret < 0) {
> + goto out;
> + }
> +
> + fuse_session_process_buf(exp->fuse_session, &exp->fuse_buf);
> +
> +out:
> + blk_exp_unref(&exp->common);
> +}
> +
> +static void fuse_export_shutdown(BlockExport *blk_exp)
> +{
> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
> +
> + if (exp->fuse_session) {
> + fuse_session_exit(exp->fuse_session);
> +
> + if (exp->mounted) {
> + fuse_session_unmount(exp->fuse_session);
> + exp->mounted = false;
> + }
> +
> + if (exp->fd_handler_set_up) {
> + aio_set_fd_handler(exp->common.ctx,
> + fuse_session_fd(exp->fuse_session), true,
> + NULL, NULL, NULL, NULL);
> + exp->fd_handler_set_up = false;
> + }
> +
> + fuse_session_destroy(exp->fuse_session);
> + exp->fuse_session = NULL;
What happens if a request is still in flight?
Oh, can't happen because the driver is fully synchronous after this
series. Fair enough, making it asynchronous can come on top of it.
However, I think we should move fuse_session_unmount() and
fuse_session_destroy() to .delete instead of .shutdown to avoid setting
up a trap for whoever introduces async requests to the FUSE export.
> + }
> +
> + if (exp->mountpoint) {
> + g_hash_table_remove(exports, exp->mountpoint);
> + }
> +}
> +
> +static void fuse_export_delete(BlockExport *blk_exp)
> +{
> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
> +
> + free(exp->fuse_buf.mem);
> + g_free(exp->mountpoint);
> +}
> +
> +/**
> + * Check whether @path points to a regular file. If not, put an
> + * appropriate message into *errp.
> + */
> +static bool is_regular_file(const char *path, Error **errp)
> +{
> + struct stat statbuf;
> + int ret;
> +
> + ret = stat(path, &statbuf);
> + if (ret < 0) {
> + error_setg_errno(errp, errno, "Failed to stat '%s'", path);
> + return false;
> + }
> +
> + if (!S_ISREG(statbuf.st_mode)) {
> + error_setg(errp, "'%s' is not a regular file", path);
> + return false;
> + }
> +
> + return true;
> +}
This is obviously racy (TOCTOU), but if I understand correctly, we use
it only to provide a nicer error message than if fuse_session_mount()
fails, so this is fine.
> +static const struct fuse_lowlevel_ops fuse_ops = {
> +};
> +
> +const BlockExportDriver blk_exp_fuse = {
> + .type = BLOCK_EXPORT_TYPE_FUSE,
> + .instance_size = sizeof(FuseExport),
> + .create = fuse_export_create,
> + .delete = fuse_export_delete,
> + .request_shutdown = fuse_export_shutdown,
> +};
Kevin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 03/20] fuse: Implement standard FUSE operations
2020-09-22 10:49 ` [PATCH v2 03/20] fuse: Implement standard FUSE operations Max Reitz
@ 2020-10-15 9:46 ` Kevin Wolf
2020-10-15 15:18 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 9:46 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> This makes the export actually useful instead of only producing errors
> whenever it is accessed.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/export/fuse.c | 226 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 226 insertions(+)
>
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 75f11d2514..8fc667231d 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -32,6 +32,10 @@
> #include <fuse_lowlevel.h>
>
>
> +/* Prevent overly long bounce buffer allocations */
> +#define FUSE_MAX_BOUNCE_BYTES (MIN(BDRV_REQUEST_MAX_BYTES, 64 * 1024 * 1024))
> +
> +
> typedef struct FuseExport {
> BlockExport common;
>
> @@ -241,7 +245,229 @@ static bool is_regular_file(const char *path, Error **errp)
> return true;
> }
>
> +
> +/**
> + * Let clients look up files. Always return ENOENT because we only
> + * care about the mountpoint itself.
> + */
> +static void fuse_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
> +{
> + fuse_reply_err(req, ENOENT);
> +}
> +
> +/**
> + * Let clients get file attributes (i.e., stat() the file).
> + */
> +static void fuse_getattr(fuse_req_t req, fuse_ino_t inode,
> + struct fuse_file_info *fi)
> +{
> + struct stat statbuf;
> + int64_t length, allocated_blocks;
> + time_t now = time(NULL);
> + ImageInfo *info = NULL;
> + FuseExport *exp = fuse_req_userdata(req);
> + mode_t mode;
> + Error *local_error = NULL;
> +
> + length = blk_getlength(exp->common.blk);
> + if (length < 0) {
> + fuse_reply_err(req, -length);
> + return;
> + }
> +
> + bdrv_query_image_info(blk_bs(exp->common.blk), &info, &local_error);
> + if (local_error) {
> + allocated_blocks = DIV_ROUND_UP(length, 512);
> + } else {
> + allocated_blocks = DIV_ROUND_UP(info->actual_size, 512);
> + }
> +
> + qapi_free_ImageInfo(info);
> + error_free(local_error);
> + local_error = NULL;
If you only use info->actual_size, why not directly call
bdrv_get_allocated_file_size()?
> +
> + mode = S_IFREG | S_IRUSR;
> + if (exp->writable) {
> + mode |= S_IWUSR;
> + }
> +
> + statbuf = (struct stat) {
> + .st_ino = inode,
> + .st_mode = mode,
> + .st_nlink = 1,
> + .st_uid = getuid(),
> + .st_gid = getgid(),
> + .st_size = length,
> + .st_blksize = blk_bs(exp->common.blk)->bl.request_alignment,
> + .st_blocks = allocated_blocks,
> + .st_atime = now,
> + .st_mtime = now,
> + .st_ctime = now,
> + };
> +
> + fuse_reply_attr(req, &statbuf, 1.);
> +}
> +
> +static int fuse_do_truncate(const FuseExport *exp, int64_t size,
> + PreallocMode prealloc)
> +{
> + uint64_t blk_perm, blk_shared_perm;
> + int ret;
> +
> + blk_get_perm(exp->common.blk, &blk_perm, &blk_shared_perm);
> +
> + ret = blk_set_perm(exp->common.blk, blk_perm | BLK_PERM_RESIZE,
> + blk_shared_perm, NULL);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + ret = blk_truncate(exp->common.blk, size, true, prealloc, 0, NULL);
> +
> + /* Must succeed, because we are only giving up the RESIZE permission */
> + blk_set_perm(exp->common.blk, blk_perm, blk_shared_perm, &error_abort);
> +
> + return ret;
> +}
> +
> +/**
> + * Let clients set file attributes. Only resizing is supported.
> + */
> +static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf,
> + int to_set, struct fuse_file_info *fi)
> +{
> + FuseExport *exp = fuse_req_userdata(req);
> + int ret;
> +
> + if (!exp->writable) {
> + fuse_reply_err(req, EACCES);
> + return;
> + }
> +
> + if (to_set & ~FUSE_SET_ATTR_SIZE) {
> + fuse_reply_err(req, ENOTSUP);
> + return;
> + }
> +
> + ret = fuse_do_truncate(exp, statbuf->st_size, PREALLOC_MODE_OFF);
> + if (ret < 0) {
> + fuse_reply_err(req, -ret);
> + return;
> + }
> +
> + fuse_getattr(req, inode, fi);
> +}
> +
> +/**
> + * Let clients open a file (i.e., the exported image).
> + */
> +static void fuse_open(fuse_req_t req, fuse_ino_t inode,
> + struct fuse_file_info *fi)
> +{
> + fuse_reply_open(req, fi);
> +}
> +
> +/**
> + * Handle client reads from the exported image.
> + */
> +static void fuse_read(fuse_req_t req, fuse_ino_t inode,
> + size_t size, off_t offset, struct fuse_file_info *fi)
> +{
> + FuseExport *exp = fuse_req_userdata(req);
> + int64_t length;
> + void *buf;
> + int ret;
> +
> + /**
> + * Clients will expect short reads at EOF, so we have to limit
> + * offset+size to the image length.
> + */
> + length = blk_getlength(exp->common.blk);
> + if (length < 0) {
> + fuse_reply_err(req, -length);
> + return;
> + }
> +
> + size = MIN(size, FUSE_MAX_BOUNCE_BYTES);
"Read should send exactly the number of bytes requested except on EOF or
error, otherwise the rest of the data will be substituted with zeroes."
Do we corrupt huge reads with this, so that read() succeeds, but the
buffer is zeroed above 64M instead of containing the correct data? Maybe
we should return an error instead?
(It's kind of sad that we need a bounce buffer from which data is later
copied instead of being provided the right memory by the kernel.)
> + if (offset + size > length) {
> + size = length - offset;
> + }
> +
> + buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
> + if (!buf) {
> + fuse_reply_err(req, ENOMEM);
> + return;
> + }
> +
> + ret = blk_pread(exp->common.blk, offset, buf, size);
> + if (ret >= 0) {
> + fuse_reply_buf(req, buf, size);
> + } else {
> + fuse_reply_err(req, -ret);
> + }
> +
> + qemu_vfree(buf);
> +}
> +
> +/**
> + * Handle client writes to the exported image.
> + */
> +static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
> + size_t size, off_t offset, struct fuse_file_info *fi)
> +{
> + FuseExport *exp = fuse_req_userdata(req);
> + int64_t length;
> + int ret;
> +
> + if (!exp->writable) {
> + fuse_reply_err(req, EACCES);
> + return;
> + }
> +
> + /**
> + * Clients will expect short writes at EOF, so we have to limit
> + * offset+size to the image length.
> + */
> + length = blk_getlength(exp->common.blk);
> + if (length < 0) {
> + fuse_reply_err(req, -length);
> + return;
> + }
> +
> + size = MIN(size, BDRV_REQUEST_MAX_BYTES);
We're only supposed to do short writes on EOF, so this has a similar
problem as in fuse_read, except that BDRV_REQUEST_MAX_BYTES is much
higher and it's not specified what the resulting misbehaviour could be
(possibly the kernel not retrying write for the rest of the buffer?)
> + if (offset + size > length) {
> + size = length - offset;
> + }
> +
> + ret = blk_pwrite(exp->common.blk, offset, buf, size, 0);
> + if (ret >= 0) {
> + fuse_reply_write(req, size);
> + } else {
> + fuse_reply_err(req, -ret);
> + }
> +}
> +
> +/**
> + * Let clients flush the exported image.
> + */
> +static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
> + struct fuse_file_info *fi)
> +{
> + FuseExport *exp = fuse_req_userdata(req);
> + int ret;
> +
> + ret = blk_flush(exp->common.blk);
> + fuse_reply_err(req, ret < 0 ? -ret : 0);
> +}
This seems to be an implementation for .fsync rather than for .flush.
Hmm, or maybe actually for both? We usually do bdrv_flush() during
close, so it would be consistent to do the same here. It's our last
chance to report an error to the user before the file is closed.
> static const struct fuse_lowlevel_ops fuse_ops = {
> + .lookup = fuse_lookup,
> + .getattr = fuse_getattr,
> + .setattr = fuse_setattr,
> + .open = fuse_open,
> + .read = fuse_read,
> + .write = fuse_write,
> + .flush = fuse_flush,
> };
Kevin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 04/20] fuse: Allow growable exports
2020-09-22 10:49 ` [PATCH v2 04/20] fuse: Allow growable exports Max Reitz
@ 2020-10-15 10:41 ` Kevin Wolf
2020-10-15 15:20 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 10:41 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> These will behave more like normal files in that writes beyond the EOF
> will automatically grow the export size.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qapi/block-export.json | 6 +++++-
> block/export/fuse.c | 12 +++++++++++-
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index cb5bd54cbf..cb26daa98b 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -183,10 +183,14 @@
> # @mountpoint: Path on which to export the block device via FUSE.
> # This must point to an existing regular file.
> #
> +# @growable: Whether writes beyond the EOF should grow the block node
> +# accordingly. (default: false)
> +#
> # Since: 5.2
> ##
> { 'struct': 'BlockExportOptionsFuse',
> - 'data': { 'mountpoint': 'str' },
> + 'data': { 'mountpoint': 'str',
> + '*growable': 'bool' },
> 'if': 'defined(CONFIG_FUSE)' }
>
> ##
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 8fc667231d..f3a84579ba 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -45,6 +45,7 @@ typedef struct FuseExport {
>
> char *mountpoint;
> bool writable;
> + bool growable;
> } FuseExport;
>
> static GHashTable *exports;
> @@ -101,6 +102,7 @@ static int fuse_export_create(BlockExport *blk_exp,
>
> exp->mountpoint = g_strdup(args->mountpoint);
> exp->writable = blk_exp_args->writable;
> + exp->growable = args->growable;
>
> ret = setup_fuse_export(exp, args->mountpoint, errp);
> if (ret < 0) {
> @@ -436,7 +438,15 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
>
> size = MIN(size, BDRV_REQUEST_MAX_BYTES);
> if (offset + size > length) {
> - size = length - offset;
> + if (exp->growable) {
> + ret = fuse_do_truncate(exp, offset + size, PREALLOC_MODE_OFF);
Do we need BDRV_REQ_ZERO_WRITE for blk_truncate() in this case?
Actually, since we export a regular file, it would probably be good for
the setattr case, too.
If someone actually uses this to sequentially write past the end of the
file, it will be quite inefficient because fuse_do_truncate() temporarily
acquires locks for each write request. It might be a good idea to
acquire BLK_PERM_RESIZE from the start for growable exports (like image
formats do for bs->file).
> + if (ret < 0) {
> + fuse_reply_err(req, -ret);
> + return;
> + }
> + } else {
> + size = length - offset;
> + }
> }
Kevin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon
2020-09-22 10:49 ` [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon Max Reitz
@ 2020-10-15 11:27 ` Kevin Wolf
2020-10-15 15:22 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 11:27 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/check | 11 +++++++++++
> tests/qemu-iotests/common.rc | 17 +++++++++++++++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index e14a1f354d..467a7cf1b7 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -644,6 +644,17 @@ if [ -z $QEMU_NBD_PROG ]; then
> fi
> export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
>
> +if [ -z "$QEMU_STGD_PROG" ]; then
No series without some bikeshedding:
This is the first time I see "QEMU_STGD" as a short form. Without the
subject line of the patch, I wouldn't be able to guess what it is.
If the full name ($QEMU_STORAGE_DAEMON_PROG) is too long, I think simply
$QSD_PROG would be an alternative, because "qsd" is what people already
use an as abbreviation.
Kevin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 18/20] iotests: Allow testing FUSE exports
2020-09-22 10:49 ` [PATCH v2 18/20] iotests: Allow testing FUSE exports Max Reitz
@ 2020-10-15 11:43 ` Kevin Wolf
2020-10-15 15:27 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 11:43 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> This pretends FUSE exports are a kind of protocol. As such, they are
> always tested under the format node. This is probably the best way to
> test them, actually, because this will generate more I/O load and more
> varied patterns.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/check | 6 ++
> tests/qemu-iotests/common.filter | 5 +-
> tests/qemu-iotests/common.rc | 124 +++++++++++++++++++++++++++++++
> 3 files changed, 134 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 467a7cf1b7..07232138d7 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -270,6 +270,7 @@ image protocol options
> -rbd test rbd
> -sheepdog test sheepdog
> -nbd test nbd
> + -fuse test fuse
> -ssh test ssh
> -nfs test nfs
>
> @@ -382,6 +383,11 @@ testlist options
> xpand=false
> ;;
>
> + -fuse)
> + IMGPROTO=fuse
> + xpand=false
> + ;;
> +
> -ssh)
> IMGPROTO=ssh
> xpand=false
> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index 838ed15793..172ea5752e 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -44,7 +44,8 @@ _filter_qom_path()
> _filter_testdir()
> {
> $SED -e "s#$TEST_DIR/#TEST_DIR/#g" \
> - -e "s#$SOCK_DIR/#SOCK_DIR/#g"
> + -e "s#$SOCK_DIR/#SOCK_DIR/#g" \
> + -e "s#SOCK_DIR/fuse-#TEST_DIR/#g"
> }
>
> # replace occurrences of the actual IMGFMT value with IMGFMT
> @@ -127,6 +128,7 @@ _filter_img_create_filenames()
> -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
> -e "s#$TEST_DIR#TEST_DIR#g" \
> -e "s#$SOCK_DIR#SOCK_DIR#g" \
> + -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
> -e "s#$IMGFMT#IMGFMT#g" \
> -e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
> -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
> @@ -227,6 +229,7 @@ _filter_img_info()
> -e "s#$IMGFMT#IMGFMT#g" \
> -e 's#nbd://127.0.0.1:[0-9]\\+$#TEST_DIR/t.IMGFMT#g' \
> -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g' \
> + -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
> -e "/encrypted: yes/d" \
> -e "/cluster_size: [0-9]\\+/d" \
> -e "/table_size: [0-9]\\+/d" \
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index e4751d4985..e17f813f06 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -257,6 +257,9 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> TEST_IMG="$DRIVER,file.driver=nbd,file.type=unix"
> TEST_IMG="$TEST_IMG,file.path=$SOCK_DIR/nbd"
> + elif [ "$IMGPROTO" = "fuse" ]; then
> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> + TEST_IMG="$DRIVER,file.filename=$SOCK_DIR/fuse-t.$IMGFMT"
> elif [ "$IMGPROTO" = "ssh" ]; then
> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE"
> @@ -273,6 +276,9 @@ else
> elif [ "$IMGPROTO" = "nbd" ]; then
> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> TEST_IMG="nbd+unix:///?socket=$SOCK_DIR/nbd"
> + elif [ "$IMGPROTO" = "fuse" ]; then
> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> + TEST_IMG="$SOCK_DIR/fuse-t.$IMGFMT"
> elif [ "$IMGPROTO" = "ssh" ]; then
> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR"
> @@ -288,6 +294,9 @@ fi
> ORIG_TEST_IMG_FILE=$TEST_IMG_FILE
> ORIG_TEST_IMG="$TEST_IMG"
>
> +FUSE_PIDS=()
> +FUSE_EXPORTS=()
> +
> if [ -z "$TEST_DIR" ]; then
> TEST_DIR=$PWD/scratch
> fi
> @@ -357,6 +366,10 @@ _test_img_to_test_img_file()
> echo "$1"
> ;;
>
> + fuse)
> + echo "$1" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#"
> + ;;
> +
> nfs)
> echo "$1" | sed -e "s#nfs://127.0.0.1##"
> ;;
> @@ -385,6 +398,11 @@ _make_test_img()
> local opts_param=false
> local misc_params=()
>
> + if [[ $IMGPROTO == fuse && $TEST_IMG == $SOCK_DIR/fuse-* ]]; then
Given that you sent this series, I assume the test cases pass, but I
don't understand how this works with more than one image. Shouldn't you
get an syntax error then because $SOCK_DIR/fuse-* will evaluate to
multiple words?
> + # The caller may be trying to overwrite an existing image
> + _rm_test_img "$TEST_IMG"
> + fi
> +
> if [ -z "$TEST_IMG_FILE" ]; then
> img_name=$TEST_IMG
> elif [ "$IMGOPTSSYNTAX" != "true" -a \
> @@ -469,11 +487,105 @@ _make_test_img()
> eval "$QEMU_NBD -v -t -k '$SOCK_DIR/nbd' -f $IMGFMT -e 42 -x '' $TEST_IMG_FILE >/dev/null &"
> sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
> fi
> +
> + if [ $IMGPROTO = "fuse" -a -f "$img_name" ]; then
> + local export_mp
> + local pid
> + local pidfile
> + local timeout
> +
> + export_mp=$(echo "$img_name" | sed -e "s#$TEST_DIR/#$SOCK_DIR/fuse-#")
> + if ! echo "$export_mp" | grep -q "^$SOCK_DIR"; then
> + echo 'Cannot use FUSE exports with images outside of TEST_DIR' >&2
> + return 1
> + fi
> +
> + touch "$export_mp"
> + rm -f "$SOCK_DIR/fuse-output"
> +
> + # Usually, users would export formatted nodes. But we present fuse as a
> + # protocol-level driver here, so we have to leave the format to the
> + # client.
> + QEMU_STGD_NEED_PID=y $QEMU_STGD \
> + --blockdev file,node-name=export-node,filename=$img_name,discard=unmap \
> + --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on \
> + &
> +
> + pidfile="$QEMU_TEST_DIR/qemu-storage-daemon.pid"
> +
> + # Wait for the PID file
> + while [ ! -f "$pidfile" ]; do
> + sleep 0.5
> + done
> +
> + pid=$(cat "$pidfile")
> + rm -f "$pidfile"
> +
> + FUSE_PIDS+=($pid)
> + FUSE_EXPORTS+=("$export_mp")
> + fi
> }
>
> _rm_test_img()
> {
> local img=$1
> +
> + if [[ $IMGPROTO == fuse && $img == $SOCK_DIR/fuse-* ]]; then
> + # Drop a FUSE export
> + local df_output
> + local i
> + local image_file
> + local index=''
> + local timeout
> +
> + for i in "${!FUSE_EXPORTS[@]}"; do
> + if [ "${FUSE_EXPORTS[i]}" = "$img" ]; then
> + index=$i
> + break
> + fi
> + done
> +
> + if [ -z "$index" ]; then
> + # Probably gone already
> + return 0
> + fi
> +
> + kill "${FUSE_PIDS[index]}"
> +
> + # Wait until the mount is gone
> + timeout=10 # *0.5 s
> + while true; do
> + # Will show the mount point; if the mount is still there,
> + # it will be $img.
> + df_output=$(df -T "$img" 2>/dev/null)
'df -T' doesn't seem to be portable.
Well, neither is FUSE, so I guess it doesn't matter?
> +
> + # But df may also show an error ("Transpoint endpoint not
> + # connected"), so retry in such cases
> + if [ -n "$df_output" ]; then
> + if ! echo "$df_output" | grep -q "$img"; then
> + break
> + fi
> + fi
> +
> + sleep 0.5
> +
> + timeout=$((timeout - 1))
> + if [ "$timeout" = 0 ]; then
> + echo 'Failed to take down FUSE export' >&2
> + return 1
> + fi
> + done
> +
> + rm -f "$img"
> +
> + unset "FUSE_PIDS[$index]"
> + unset "FUSE_EXPORTS[$index]"
> +
> + image_file=$(echo "$img" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#")
> + _rm_test_img "$image_file"
> + return
> + fi
> +
> if [ "$IMGFMT" = "vmdk" ]; then
> # Remove all the extents for vmdk
> "$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \
Kevin
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
` (20 preceding siblings ...)
2020-09-22 15:58 ` [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Daniel P. Berrangé
@ 2020-10-15 12:01 ` Kevin Wolf
2020-10-15 16:47 ` Max Reitz
21 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 12:01 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> Based-on: <20200907182011.521007-1-kwolf@redhat.com>
> (“block/export: Add infrastructure and QAPI for block exports”)
>
> (Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
>
> v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
>
> Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
> Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
>
>
> Hi,
>
> Ever since I found out that you can mount FUSE filesystems on regular
> files (not just directories), I had the idea of adding FUSE block
> exports to qemu where you can export block nodes as raw images. The
> best thing is that you’d be able to mount an image on itself, so
> whatever format it may be in, qemu lets it appear as a raw image (and
> you can then use regular tools like dd on it).
>
> The performance is quite bad so far, but we can always try to improve it
> if the need arises. For now I consider it mostly a cute feature to get
> easy access to the raw contents of image files in any format (without
> requiring root rights).
>
> In this version (as opposed to v1 linked above), I integrated the FUSE
> export code into Kevin’s proposed common infrastructure for block
> exports.
Patches 5-16, 19 and 20:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-10-15 8:57 ` Kevin Wolf
@ 2020-10-15 14:46 ` Max Reitz
2020-10-15 15:41 ` Kevin Wolf
0 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-10-15 14:46 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 16004 bytes --]
On 15.10.20 10:57, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> block-export-add type=fuse allows mounting block graph nodes via FUSE on
>> some existing regular file. That file should then appears like a raw
>> disk image, and accesses to it result in accesses to the exported BDS.
>>
>> Right now, we only implement the necessary block export functions to set
>> it up and shut it down. We do not implement any access functions, so
>> accessing the mount point only results in errors. This will be
>> addressed by a followup patch.
>>
>> We keep a hash table of exported mount points, because we want to be
>> able to detect when users try to use a mount point twice. This is
>> because we invoke stat() to check whether the given mount point is a
>> regular file, but if that file is served by ourselves (because it is
>> already used as a mount point), then this stat() would have to be served
>> by ourselves, too, which is impossible to do while we (as the caller)
>> are waiting for it to settle. Therefore, keep track of mount point
>> paths to at least catch the most obvious instances of that problem.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qapi/block-export.json | 24 +++-
>> include/block/fuse.h | 30 +++++
>> block.c | 1 +
>> block/export/export.c | 4 +
>> block/export/fuse.c | 253 +++++++++++++++++++++++++++++++++++++++
>> block/export/meson.build | 1 +
>> 6 files changed, 311 insertions(+), 2 deletions(-)
>> create mode 100644 include/block/fuse.h
>> create mode 100644 block/export/fuse.c
>>
>> diff --git a/qapi/block-export.json b/qapi/block-export.json
>> index 0c045dfe7b..cb5bd54cbf 100644
>> --- a/qapi/block-export.json
>> +++ b/qapi/block-export.json
>> @@ -174,6 +174,21 @@
>> ##
>> { 'command': 'nbd-server-stop' }
>>
>> +##
>> +# @BlockExportOptionsFuse:
>> +#
>> +# Options for exporting a block graph node on some (file) mountpoint
>> +# as a raw image.
>> +#
>> +# @mountpoint: Path on which to export the block device via FUSE.
>> +# This must point to an existing regular file.
>> +#
>> +# Since: 5.2
>> +##
>> +{ 'struct': 'BlockExportOptionsFuse',
>> + 'data': { 'mountpoint': 'str' },
>> + 'if': 'defined(CONFIG_FUSE)' }
>> +
>> ##
>> # @BlockExportType:
>> #
>> @@ -181,10 +196,13 @@
>> #
>> # @nbd: NBD export
>> #
>> +# @fuse: Fuse export (since: 5.2)
>
> s/Fuse/FUSE/?
Tomato, TOMATO.
Sure. :)
>> +#
>> # Since: 4.2
>> ##
>> { 'enum': 'BlockExportType',
>> - 'data': [ 'nbd' ] }
>> + 'data': [ 'nbd',
>> + { 'name': 'fuse', 'if': 'defined(CONFIG_FUSE)' } ] }
>>
>> ##
>> # @BlockExportOptions:
>> @@ -213,7 +231,9 @@
>> '*writethrough': 'bool' },
>> 'discriminator': 'type',
>> 'data': {
>> - 'nbd': 'BlockExportOptionsNbd'
>> + 'nbd': 'BlockExportOptionsNbd',
>> + 'fuse': { 'type': 'BlockExportOptionsFuse',
>> + 'if': 'defined(CONFIG_FUSE)' }
>> } }
>>
>> ##
>> diff --git a/include/block/fuse.h b/include/block/fuse.h
>> new file mode 100644
>> index 0000000000..ffa91fe364
>> --- /dev/null
>> +++ b/include/block/fuse.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Present a block device as a raw image through FUSE
>> + *
>> + * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; under version 2 or later of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef BLOCK_FUSE_H
>> +#define BLOCK_FUSE_H
>> +
>> +#ifdef CONFIG_FUSE
>> +
>> +#include "block/export.h"
>> +
>> +extern const BlockExportDriver blk_exp_fuse;
>> +
>> +#endif /* CONFIG_FUSE */
>> +
>> +#endif
>> diff --git a/block.c b/block.c
>> index acde53f92a..7bf45f6ede 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -26,6 +26,7 @@
>> #include "block/trace.h"
>> #include "block/block_int.h"
>> #include "block/blockjob.h"
>> +#include "block/fuse.h"
>> #include "block/nbd.h"
>> #include "block/qdict.h"
>> #include "qemu/error-report.h"
>> diff --git a/block/export/export.c b/block/export/export.c
>> index 64d39a6934..ba866d6ba7 100644
>> --- a/block/export/export.c
>> +++ b/block/export/export.c
>> @@ -16,6 +16,7 @@
>> #include "block/block.h"
>> #include "sysemu/block-backend.h"
>> #include "block/export.h"
>> +#include "block/fuse.h"
>> #include "block/nbd.h"
>> #include "qapi/error.h"
>> #include "qapi/qapi-commands-block-export.h"
>> @@ -24,6 +25,9 @@
>>
>> static const BlockExportDriver *blk_exp_drivers[] = {
>> &blk_exp_nbd,
>> +#ifdef CONFIG_FUSE
>> + &blk_exp_fuse,
>> +#endif
>> };
>>
>> /* Only accessed from the main thread */
>> diff --git a/block/export/fuse.c b/block/export/fuse.c
>> new file mode 100644
>> index 0000000000..75f11d2514
>> --- /dev/null
>> +++ b/block/export/fuse.c
>> @@ -0,0 +1,253 @@
>> +/*
>> + * Present a block device as a raw image through FUSE
>> + *
>> + * Copyright (c) 2020 Max Reitz <mreitz@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; under version 2 or later of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#define FUSE_USE_VERSION 31
>> +
>> +#include "qemu/osdep.h"
>> +#include "block/aio.h"
>> +#include "block/block.h"
>> +#include "block/export.h"
>> +#include "block/fuse.h"
>> +#include "block/qapi.h"
>> +#include "qapi/error.h"
>> +#include "qapi/qapi-commands-block.h"
>> +#include "sysemu/block-backend.h"
>> +
>> +#include <fuse.h>
>> +#include <fuse_lowlevel.h>
>> +
>> +
>> +typedef struct FuseExport {
>> + BlockExport common;
>> +
>> + struct fuse_session *fuse_session;
>> + struct fuse_buf fuse_buf;
>> + bool mounted, fd_handler_set_up;
>> +
>> + char *mountpoint;
>> + bool writable;
>> +} FuseExport;
>> +
>> +static GHashTable *exports;
>> +static const struct fuse_lowlevel_ops fuse_ops;
>> +
>> +static void fuse_export_shutdown(BlockExport *exp);
>> +static void fuse_export_delete(BlockExport *exp);
>> +
>> +static void init_fuse(void);
>> +static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
>> + Error **errp);
>> +static void read_from_fuse_export(void *opaque);
>> +
>> +static bool is_regular_file(const char *path, Error **errp);
>> +
>> +
>> +static int fuse_export_create(BlockExport *blk_exp,
>> + BlockExportOptions *blk_exp_args,
>> + Error **errp)
>> +{
>> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
>> + BlockExportOptionsFuse *args = &blk_exp_args->u.fuse;
>> + int ret;
>> +
>> + assert(blk_exp_args->type == BLOCK_EXPORT_TYPE_FUSE);
>> +
>> + init_fuse();
>> +
>> + /*
>> + * It is important to do this check before calling is_regular_file() --
>> + * that function will do a stat(), which we would have to handle if we
>> + * already exported something on @mountpoint. But we cannot, because
>> + * we are currently caught up here.
>> + * (Note that ideally we would want to resolve relative paths here,
>> + * but bdrv_make_absolute_filename() might do the wrong thing for
>> + * paths that contain colons, and realpath() would resolve symlinks,
>> + * which we do not want: The mount point is not going to be the
>> + * symlink's destination, but the link itself.)
>> + * So this will not catch all potential clashes, but hopefully at
>> + * least the most common one of specifying exactly the same path
>> + * string twice.
>> + */
>> + if (g_hash_table_contains(exports, args->mountpoint)) {
>> + error_setg(errp, "There already is a FUSE export on '%s'",
>> + args->mountpoint);
>> + ret = -EEXIST;
>> + goto fail;
>> + }
>
> I guess the proper solution would be to make block export creation
> asynchronous so that we can actually serve that request in the
> background.
>
> Something for another day...
>
>> +
>> + if (!is_regular_file(args->mountpoint, errp)) {
>> + ret = -EINVAL;
>> + goto fail;
>> + }
>> +
>> + exp->mountpoint = g_strdup(args->mountpoint);
>> + exp->writable = blk_exp_args->writable;
>> +
>> + ret = setup_fuse_export(exp, args->mountpoint, errp);
>> + if (ret < 0) {
>> + goto fail;
>> + }
>> +
>> + g_hash_table_insert(exports, g_strdup(args->mountpoint), NULL);
>> + return 0;
>> +
>> +fail:
>> + fuse_export_shutdown(blk_exp);
>> + fuse_export_delete(blk_exp);
>
> Why fuse_export_shutdown()? The only thing to possibly undo at
> this point is allocating exp->mountpoint, which is freed in
> fuse_export_delete().
I just realized it’s not only a question of style, but even technically
wrong to do so. fuse_export_shutdown() drops the mount point path from
the @exports hash table, which shouldn’t be done if we got here from the
g_hash_table_contains() condition.
That could be fixed by introducing a new label that won’t execute
fuse_export_shutdown(), but we can also just let setup_fuse_export()
call that.
> Or can setup_fuse_export() fail without cleaning up? I hope not.
>
>> + return ret;
>> +}
>> +
>> +/**
>> + * Ensure that the global FUSE context is set up.
>> + */
>> +static void init_fuse(void)
>> +{
>> + if (exports) {
>> + return;
>> + }
>> +
>> + exports = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
>> +}
>> +
>> +/**
>> + * Create exp->fuse_session and mount it.
>> + */
>> +static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
>> + Error **errp)
>> +{
>> + const char *fuse_argv[2];
>> + struct fuse_args fuse_args;
>> + int ret;
>> +
>> + fuse_argv[0] = ""; /* Dummy program name */
>> + fuse_argv[1] = NULL;
>> + fuse_args = (struct fuse_args)FUSE_ARGS_INIT(1, (char **)fuse_argv);
>> +
>> + exp->fuse_session = fuse_session_new(&fuse_args, &fuse_ops,
>> + sizeof(fuse_ops), exp);
>> + if (!exp->fuse_session) {
>> + error_setg(errp, "Failed to set up FUSE session");
>> + return -EIO;
>> + }
>> +
>> + ret = fuse_session_mount(exp->fuse_session, mountpoint);
>> + if (ret < 0) {
>> + error_setg(errp, "Failed to mount FUSE session to export");
>> + return -EIO;
>
> Oh, it can. :-(
>
> I think it would be better to call fuse_export_shutdown() here so that
> the function doesn't change the state when it returns failure.
I suppose then this function (setup_fuse_export()) should also add the
mount point name from the @exports hash table, so it makes sense to drop
it via fuse_export_shutdown().
But then yeah, why not. Makes sense.
>> + }
>> + exp->mounted = true;
>> +
>> + aio_set_fd_handler(exp->common.ctx,
>> + fuse_session_fd(exp->fuse_session), true,
>> + read_from_fuse_export, NULL, NULL, exp);
>> + exp->fd_handler_set_up = true;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * Callback to be invoked when the FUSE session FD can be read from.
>> + * (This is basically the FUSE event loop.)
>> + */
>> +static void read_from_fuse_export(void *opaque)
>> +{
>> + FuseExport *exp = opaque;
>> + int ret;
>> +
>> + blk_exp_ref(&exp->common);
>> +
>> + ret = fuse_session_receive_buf(exp->fuse_session, &exp->fuse_buf);
>
> The fuse_session_loop() implementation seems to imply that we should
> retry on EINTR.
OK. I see you’re digging into libfuse already. :)
>> + if (ret < 0) {
>> + goto out;
>> + }
>> +
>> + fuse_session_process_buf(exp->fuse_session, &exp->fuse_buf);
>> +
>> +out:
>> + blk_exp_unref(&exp->common);
>> +}
>> +
>> +static void fuse_export_shutdown(BlockExport *blk_exp)
>> +{
>> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
>> +
>> + if (exp->fuse_session) {
>> + fuse_session_exit(exp->fuse_session);
>> +
>> + if (exp->mounted) {
>> + fuse_session_unmount(exp->fuse_session);
>> + exp->mounted = false;
>> + }
>> +
>> + if (exp->fd_handler_set_up) {
>> + aio_set_fd_handler(exp->common.ctx,
>> + fuse_session_fd(exp->fuse_session), true,
>> + NULL, NULL, NULL, NULL);
>> + exp->fd_handler_set_up = false;
>> + }
>> +
>> + fuse_session_destroy(exp->fuse_session);
>> + exp->fuse_session = NULL;
>
> What happens if a request is still in flight?
>
> Oh, can't happen because the driver is fully synchronous after this
> series. Fair enough, making it asynchronous can come on top of it.
(I had multiple approaches of handling parallel requests, but none made
a substantial performance difference, which is why I left the driver in
the most simple form for this first proposal.)
> However, I think we should move fuse_session_unmount() and
> fuse_session_destroy() to .delete instead of .shutdown to avoid setting
> up a trap for whoever introduces async requests to the FUSE export.
Yes, makes sense.
>> + }
>> +
>> + if (exp->mountpoint) {
>> + g_hash_table_remove(exports, exp->mountpoint);
>> + }
>> +}
>> +
>> +static void fuse_export_delete(BlockExport *blk_exp)
>> +{
>> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
>> +
>> + free(exp->fuse_buf.mem);
>> + g_free(exp->mountpoint);
>> +}
>> +
>> +/**
>> + * Check whether @path points to a regular file. If not, put an
>> + * appropriate message into *errp.
>> + */
>> +static bool is_regular_file(const char *path, Error **errp)
>> +{
>> + struct stat statbuf;
>> + int ret;
>> +
>> + ret = stat(path, &statbuf);
>> + if (ret < 0) {
>> + error_setg_errno(errp, errno, "Failed to stat '%s'", path);
>> + return false;
>> + }
>> +
>> + if (!S_ISREG(statbuf.st_mode)) {
>> + error_setg(errp, "'%s' is not a regular file", path);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>
> This is obviously racy (TOCTOU), but if I understand correctly, we use
> it only to provide a nicer error message than if fuse_session_mount()
> fails, so this is fine.
It’s a check provided for convenience, yes. More important is perhaps
that users might provide a directory instead of a file as the mount
point, which wouldn’t work. (You’ll just get errors back. Which isn’t
a catastrophe, it’s just nice to tell users early and explicitly that
that won’t work.)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 03/20] fuse: Implement standard FUSE operations
2020-10-15 9:46 ` Kevin Wolf
@ 2020-10-15 15:18 ` Max Reitz
2020-10-15 15:58 ` Kevin Wolf
0 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-10-15 15:18 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 9985 bytes --]
On 15.10.20 11:46, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> This makes the export actually useful instead of only producing errors
>> whenever it is accessed.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/export/fuse.c | 226 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 226 insertions(+)
>>
>> diff --git a/block/export/fuse.c b/block/export/fuse.c
>> index 75f11d2514..8fc667231d 100644
>> --- a/block/export/fuse.c
>> +++ b/block/export/fuse.c
>> @@ -32,6 +32,10 @@
>> #include <fuse_lowlevel.h>
>>
>>
>> +/* Prevent overly long bounce buffer allocations */
>> +#define FUSE_MAX_BOUNCE_BYTES (MIN(BDRV_REQUEST_MAX_BYTES, 64 * 1024 * 1024))
>> +
>> +
>> typedef struct FuseExport {
>> BlockExport common;
>>
>> @@ -241,7 +245,229 @@ static bool is_regular_file(const char *path, Error **errp)
>> return true;
>> }
>>
>> +
>> +/**
>> + * Let clients look up files. Always return ENOENT because we only
>> + * care about the mountpoint itself.
>> + */
>> +static void fuse_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
>> +{
>> + fuse_reply_err(req, ENOENT);
>> +}
>> +
>> +/**
>> + * Let clients get file attributes (i.e., stat() the file).
>> + */
>> +static void fuse_getattr(fuse_req_t req, fuse_ino_t inode,
>> + struct fuse_file_info *fi)
>> +{
>> + struct stat statbuf;
>> + int64_t length, allocated_blocks;
>> + time_t now = time(NULL);
>> + ImageInfo *info = NULL;
>> + FuseExport *exp = fuse_req_userdata(req);
>> + mode_t mode;
>> + Error *local_error = NULL;
>> +
>> + length = blk_getlength(exp->common.blk);
>> + if (length < 0) {
>> + fuse_reply_err(req, -length);
>> + return;
>> + }
>> +
>> + bdrv_query_image_info(blk_bs(exp->common.blk), &info, &local_error);
>> + if (local_error) {
>> + allocated_blocks = DIV_ROUND_UP(length, 512);
>> + } else {
>> + allocated_blocks = DIV_ROUND_UP(info->actual_size, 512);
>> + }
>> +
>> + qapi_free_ImageInfo(info);
>> + error_free(local_error);
>> + local_error = NULL;
>
> If you only use info->actual_size, why not directly call
> bdrv_get_allocated_file_size()?
🤔
Sometimes one doesn’t think of the simplest things.
>> +
>> + mode = S_IFREG | S_IRUSR;
>> + if (exp->writable) {
>> + mode |= S_IWUSR;
>> + }
>> +
>> + statbuf = (struct stat) {
>> + .st_ino = inode,
>> + .st_mode = mode,
>> + .st_nlink = 1,
>> + .st_uid = getuid(),
>> + .st_gid = getgid(),
>> + .st_size = length,
>> + .st_blksize = blk_bs(exp->common.blk)->bl.request_alignment,
>> + .st_blocks = allocated_blocks,
>> + .st_atime = now,
>> + .st_mtime = now,
>> + .st_ctime = now,
>> + };
>> +
>> + fuse_reply_attr(req, &statbuf, 1.);
>> +}
>> +
>> +static int fuse_do_truncate(const FuseExport *exp, int64_t size,
>> + PreallocMode prealloc)
>> +{
>> + uint64_t blk_perm, blk_shared_perm;
>> + int ret;
>> +
>> + blk_get_perm(exp->common.blk, &blk_perm, &blk_shared_perm);
>> +
>> + ret = blk_set_perm(exp->common.blk, blk_perm | BLK_PERM_RESIZE,
>> + blk_shared_perm, NULL);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> + ret = blk_truncate(exp->common.blk, size, true, prealloc, 0, NULL);
>> +
>> + /* Must succeed, because we are only giving up the RESIZE permission */
>> + blk_set_perm(exp->common.blk, blk_perm, blk_shared_perm, &error_abort);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * Let clients set file attributes. Only resizing is supported.
>> + */
>> +static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf,
>> + int to_set, struct fuse_file_info *fi)
>> +{
>> + FuseExport *exp = fuse_req_userdata(req);
>> + int ret;
>> +
>> + if (!exp->writable) {
>> + fuse_reply_err(req, EACCES);
>> + return;
>> + }
>> +
>> + if (to_set & ~FUSE_SET_ATTR_SIZE) {
>> + fuse_reply_err(req, ENOTSUP);
>> + return;
>> + }
>> +
>> + ret = fuse_do_truncate(exp, statbuf->st_size, PREALLOC_MODE_OFF);
>> + if (ret < 0) {
>> + fuse_reply_err(req, -ret);
>> + return;
>> + }
>> +
>> + fuse_getattr(req, inode, fi);
>> +}
>> +
>> +/**
>> + * Let clients open a file (i.e., the exported image).
>> + */
>> +static void fuse_open(fuse_req_t req, fuse_ino_t inode,
>> + struct fuse_file_info *fi)
>> +{
>> + fuse_reply_open(req, fi);
>> +}
>> +
>> +/**
>> + * Handle client reads from the exported image.
>> + */
>> +static void fuse_read(fuse_req_t req, fuse_ino_t inode,
>> + size_t size, off_t offset, struct fuse_file_info *fi)
>> +{
>> + FuseExport *exp = fuse_req_userdata(req);
>> + int64_t length;
>> + void *buf;
>> + int ret;
>> +
>> + /**
>> + * Clients will expect short reads at EOF, so we have to limit
>> + * offset+size to the image length.
>> + */
>> + length = blk_getlength(exp->common.blk);
>> + if (length < 0) {
>> + fuse_reply_err(req, -length);
>> + return;
>> + }
>> +
>> + size = MIN(size, FUSE_MAX_BOUNCE_BYTES);
>
> "Read should send exactly the number of bytes requested except on EOF or
> error, otherwise the rest of the data will be substituted with zeroes."
:(
> Do we corrupt huge reads with this, so that read() succeeds, but the
> buffer is zeroed above 64M instead of containing the correct data? Maybe
> we should return an error instead?
Hm. It looks like there is a max_read option obeyed by the kernel
driver, and it appears it’s set by implementing .init() and setting
fuse_conn_info.max_read (also, “for the time being” it has to also set
in the mount options passed to fuse_session_new()).
I’m not sure whether that does anything, though. It appears that
whenever I do a cached read, it caps out at 128k (which is what
cuse_prep_data() in libfuse sets; though increasing that number there
doesn’t change anything, so I think that’s just a coincidence), and with
O_DIRECT, it caps out at 1M.
But at least that would be grounds to return an error for >64M requests.
(Limiting max_read to 64k does limit all read requests to 64k.)
Further investigation is needed.
> (It's kind of sad that we need a bounce buffer from which data is later
> copied instead of being provided the right memory by the kernel.)
Yes, it is.
>> + if (offset + size > length) {
>> + size = length - offset;
>> + }
>> +
>> + buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
>> + if (!buf) {
>> + fuse_reply_err(req, ENOMEM);
>> + return;
>> + }
>> +
>> + ret = blk_pread(exp->common.blk, offset, buf, size);
>> + if (ret >= 0) {
>> + fuse_reply_buf(req, buf, size);
>> + } else {
>> + fuse_reply_err(req, -ret);
>> + }
>> +
>> + qemu_vfree(buf);
>> +}
>> +
>> +/**
>> + * Handle client writes to the exported image.
>> + */
>> +static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
>> + size_t size, off_t offset, struct fuse_file_info *fi)
>> +{
>> + FuseExport *exp = fuse_req_userdata(req);
>> + int64_t length;
>> + int ret;
>> +
>> + if (!exp->writable) {
>> + fuse_reply_err(req, EACCES);
>> + return;
>> + }
>> +
>> + /**
>> + * Clients will expect short writes at EOF, so we have to limit
>> + * offset+size to the image length.
>> + */
>> + length = blk_getlength(exp->common.blk);
>> + if (length < 0) {
>> + fuse_reply_err(req, -length);
>> + return;
>> + }
>> +
>> + size = MIN(size, BDRV_REQUEST_MAX_BYTES);
>
> We're only supposed to do short writes on EOF, so this has a similar
> problem as in fuse_read, except that BDRV_REQUEST_MAX_BYTES is much
> higher and it's not specified what the resulting misbehaviour could be
> (possibly the kernel not retrying write for the rest of the buffer?)
As for reading above, we can (and should) probably set max_write.
>> + if (offset + size > length) {
>> + size = length - offset;
>> + }
>> +
>> + ret = blk_pwrite(exp->common.blk, offset, buf, size, 0);
>> + if (ret >= 0) {
>> + fuse_reply_write(req, size);
>> + } else {
>> + fuse_reply_err(req, -ret);
>> + }
>> +}
>> +
>> +/**
>> + * Let clients flush the exported image.
>> + */
>> +static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
>> + struct fuse_file_info *fi)
>> +{
>> + FuseExport *exp = fuse_req_userdata(req);
>> + int ret;
>> +
>> + ret = blk_flush(exp->common.blk);
>> + fuse_reply_err(req, ret < 0 ? -ret : 0);
>> +}
>
> This seems to be an implementation for .fsync rather than for .flush.
Wouldn’t fsync entail a drain?
Or is it the opposite, flush should just drain and not invoke
blk_flush()? (Sorry, this all gets me confused every time.)
(Though I do think .fsync should both flush and drain.)
> Hmm, or maybe actually for both? We usually do bdrv_flush() during
> close, so it would be consistent to do the same here. It's our last
> chance to report an error to the user before the file is closed.
I don’t understand what you mean. What is “the same”? Closing the
image? Or indeed having .flush() be implemented with blk_flush()?
Do you mean that other parties may do the same as qemu does, i.e. flush
files before they are closed, which is why we should anticipate the same
and give users a chance to see errors?
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 04/20] fuse: Allow growable exports
2020-10-15 10:41 ` Kevin Wolf
@ 2020-10-15 15:20 ` Max Reitz
0 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-10-15 15:20 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]
On 15.10.20 12:41, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> These will behave more like normal files in that writes beyond the EOF
>> will automatically grow the export size.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qapi/block-export.json | 6 +++++-
>> block/export/fuse.c | 12 +++++++++++-
>> 2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/block-export.json b/qapi/block-export.json
>> index cb5bd54cbf..cb26daa98b 100644
>> --- a/qapi/block-export.json
>> +++ b/qapi/block-export.json
>> @@ -183,10 +183,14 @@
>> # @mountpoint: Path on which to export the block device via FUSE.
>> # This must point to an existing regular file.
>> #
>> +# @growable: Whether writes beyond the EOF should grow the block node
>> +# accordingly. (default: false)
>> +#
>> # Since: 5.2
>> ##
>> { 'struct': 'BlockExportOptionsFuse',
>> - 'data': { 'mountpoint': 'str' },
>> + 'data': { 'mountpoint': 'str',
>> + '*growable': 'bool' },
>> 'if': 'defined(CONFIG_FUSE)' }
>>
>> ##
>> diff --git a/block/export/fuse.c b/block/export/fuse.c
>> index 8fc667231d..f3a84579ba 100644
>> --- a/block/export/fuse.c
>> +++ b/block/export/fuse.c
>> @@ -45,6 +45,7 @@ typedef struct FuseExport {
>>
>> char *mountpoint;
>> bool writable;
>> + bool growable;
>> } FuseExport;
>>
>> static GHashTable *exports;
>> @@ -101,6 +102,7 @@ static int fuse_export_create(BlockExport *blk_exp,
>>
>> exp->mountpoint = g_strdup(args->mountpoint);
>> exp->writable = blk_exp_args->writable;
>> + exp->growable = args->growable;
>>
>> ret = setup_fuse_export(exp, args->mountpoint, errp);
>> if (ret < 0) {
>> @@ -436,7 +438,15 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
>>
>> size = MIN(size, BDRV_REQUEST_MAX_BYTES);
>> if (offset + size > length) {
>> - size = length - offset;
>> + if (exp->growable) {
>> + ret = fuse_do_truncate(exp, offset + size, PREALLOC_MODE_OFF);
>
> Do we need BDRV_REQ_ZERO_WRITE for blk_truncate() in this case?
Ah, yes. (Sorry, code is a bit old and I forgot when I revised it...)
> Actually, since we export a regular file, it would probably be good for
> the setattr case, too.
Yes.
> If someone actually uses this to sequentially write past the end of the
> file, it will be quite inefficient because fuse_do_truncate() temporarily
> acquires locks for each write request. It might be a good idea to
> acquire BLK_PERM_RESIZE from the start for growable exports (like image
> formats do for bs->file).
Oh, yes, that makes sense.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon
2020-10-15 11:27 ` Kevin Wolf
@ 2020-10-15 15:22 ` Max Reitz
0 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-10-15 15:22 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 1176 bytes --]
On 15.10.20 13:27, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/check | 11 +++++++++++
>> tests/qemu-iotests/common.rc | 17 +++++++++++++++++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index e14a1f354d..467a7cf1b7 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -644,6 +644,17 @@ if [ -z $QEMU_NBD_PROG ]; then
>> fi
>> export QEMU_NBD_PROG="$(type -p "$QEMU_NBD_PROG")"
>>
>> +if [ -z "$QEMU_STGD_PROG" ]; then
>
> No series without some bikeshedding:
>
> This is the first time I see "QEMU_STGD" as a short form. Without the
> subject line of the patch, I wouldn't be able to guess what it is.
>
> If the full name ($QEMU_STORAGE_DAEMON_PROG) is too long, I think simply
> $QSD_PROG would be an alternative, because "qsd" is what people already
> use an as abbreviation.
I asked myself for a second what this QEMU Street Day you’re referring
to is (I blame the [k] at the beginning of both ?SDs), but sure, why not.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 18/20] iotests: Allow testing FUSE exports
2020-10-15 11:43 ` Kevin Wolf
@ 2020-10-15 15:27 ` Max Reitz
0 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-10-15 15:27 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 9364 bytes --]
On 15.10.20 13:43, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> This pretends FUSE exports are a kind of protocol. As such, they are
>> always tested under the format node. This is probably the best way to
>> test them, actually, because this will generate more I/O load and more
>> varied patterns.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/check | 6 ++
>> tests/qemu-iotests/common.filter | 5 +-
>> tests/qemu-iotests/common.rc | 124 +++++++++++++++++++++++++++++++
>> 3 files changed, 134 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 467a7cf1b7..07232138d7 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -270,6 +270,7 @@ image protocol options
>> -rbd test rbd
>> -sheepdog test sheepdog
>> -nbd test nbd
>> + -fuse test fuse
>> -ssh test ssh
>> -nfs test nfs
>>
>> @@ -382,6 +383,11 @@ testlist options
>> xpand=false
>> ;;
>>
>> + -fuse)
>> + IMGPROTO=fuse
>> + xpand=false
>> + ;;
>> +
>> -ssh)
>> IMGPROTO=ssh
>> xpand=false
>> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
>> index 838ed15793..172ea5752e 100644
>> --- a/tests/qemu-iotests/common.filter
>> +++ b/tests/qemu-iotests/common.filter
>> @@ -44,7 +44,8 @@ _filter_qom_path()
>> _filter_testdir()
>> {
>> $SED -e "s#$TEST_DIR/#TEST_DIR/#g" \
>> - -e "s#$SOCK_DIR/#SOCK_DIR/#g"
>> + -e "s#$SOCK_DIR/#SOCK_DIR/#g" \
>> + -e "s#SOCK_DIR/fuse-#TEST_DIR/#g"
>> }
>>
>> # replace occurrences of the actual IMGFMT value with IMGFMT
>> @@ -127,6 +128,7 @@ _filter_img_create_filenames()
>> -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
>> -e "s#$TEST_DIR#TEST_DIR#g" \
>> -e "s#$SOCK_DIR#SOCK_DIR#g" \
>> + -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
>> -e "s#$IMGFMT#IMGFMT#g" \
>> -e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
>> -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g'
>> @@ -227,6 +229,7 @@ _filter_img_info()
>> -e "s#$IMGFMT#IMGFMT#g" \
>> -e 's#nbd://127.0.0.1:[0-9]\\+$#TEST_DIR/t.IMGFMT#g' \
>> -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g' \
>> + -e 's#SOCK_DIR/fuse-#TEST_DIR/#g' \
>> -e "/encrypted: yes/d" \
>> -e "/cluster_size: [0-9]\\+/d" \
>> -e "/table_size: [0-9]\\+/d" \
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index e4751d4985..e17f813f06 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -257,6 +257,9 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
>> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> TEST_IMG="$DRIVER,file.driver=nbd,file.type=unix"
>> TEST_IMG="$TEST_IMG,file.path=$SOCK_DIR/nbd"
>> + elif [ "$IMGPROTO" = "fuse" ]; then
>> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> + TEST_IMG="$DRIVER,file.filename=$SOCK_DIR/fuse-t.$IMGFMT"
>> elif [ "$IMGPROTO" = "ssh" ]; then
>> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE"
>> @@ -273,6 +276,9 @@ else
>> elif [ "$IMGPROTO" = "nbd" ]; then
>> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> TEST_IMG="nbd+unix:///?socket=$SOCK_DIR/nbd"
>> + elif [ "$IMGPROTO" = "fuse" ]; then
>> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> + TEST_IMG="$SOCK_DIR/fuse-t.$IMGFMT"
>> elif [ "$IMGPROTO" = "ssh" ]; then
>> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR"
>> @@ -288,6 +294,9 @@ fi
>> ORIG_TEST_IMG_FILE=$TEST_IMG_FILE
>> ORIG_TEST_IMG="$TEST_IMG"
>>
>> +FUSE_PIDS=()
>> +FUSE_EXPORTS=()
>> +
>> if [ -z "$TEST_DIR" ]; then
>> TEST_DIR=$PWD/scratch
>> fi
>> @@ -357,6 +366,10 @@ _test_img_to_test_img_file()
>> echo "$1"
>> ;;
>>
>> + fuse)
>> + echo "$1" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#"
>> + ;;
>> +
>> nfs)
>> echo "$1" | sed -e "s#nfs://127.0.0.1##"
>> ;;
>> @@ -385,6 +398,11 @@ _make_test_img()
>> local opts_param=false
>> local misc_params=()
>>
>> + if [[ $IMGPROTO == fuse && $TEST_IMG == $SOCK_DIR/fuse-* ]]; then
>
> Given that you sent this series, I assume the test cases pass, but I
> don't understand how this works with more than one image. Shouldn't you
> get an syntax error then because $SOCK_DIR/fuse-* will evaluate to
> multiple words?
Note the [[ ]] – this is a bash condition. You can do pattern matching
within them, as you see here. “$TEST_IMG == $SOCK_DIR/fuse-*” checks
whether $TEST_IMG matches the glob pattern given as the second operand.
>> + # The caller may be trying to overwrite an existing image
>> + _rm_test_img "$TEST_IMG"
>> + fi
>> +
>> if [ -z "$TEST_IMG_FILE" ]; then
>> img_name=$TEST_IMG
>> elif [ "$IMGOPTSSYNTAX" != "true" -a \
>> @@ -469,11 +487,105 @@ _make_test_img()
>> eval "$QEMU_NBD -v -t -k '$SOCK_DIR/nbd' -f $IMGFMT -e 42 -x '' $TEST_IMG_FILE >/dev/null &"
>> sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>> fi
>> +
>> + if [ $IMGPROTO = "fuse" -a -f "$img_name" ]; then
>> + local export_mp
>> + local pid
>> + local pidfile
>> + local timeout
>> +
>> + export_mp=$(echo "$img_name" | sed -e "s#$TEST_DIR/#$SOCK_DIR/fuse-#")
>> + if ! echo "$export_mp" | grep -q "^$SOCK_DIR"; then
>> + echo 'Cannot use FUSE exports with images outside of TEST_DIR' >&2
>> + return 1
>> + fi
>> +
>> + touch "$export_mp"
>> + rm -f "$SOCK_DIR/fuse-output"
>> +
>> + # Usually, users would export formatted nodes. But we present fuse as a
>> + # protocol-level driver here, so we have to leave the format to the
>> + # client.
>> + QEMU_STGD_NEED_PID=y $QEMU_STGD \
>> + --blockdev file,node-name=export-node,filename=$img_name,discard=unmap \
>> + --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on \
>> + &
>> +
>> + pidfile="$QEMU_TEST_DIR/qemu-storage-daemon.pid"
>> +
>> + # Wait for the PID file
>> + while [ ! -f "$pidfile" ]; do
>> + sleep 0.5
>> + done
>> +
>> + pid=$(cat "$pidfile")
>> + rm -f "$pidfile"
>> +
>> + FUSE_PIDS+=($pid)
>> + FUSE_EXPORTS+=("$export_mp")
>> + fi
>> }
>>
>> _rm_test_img()
>> {
>> local img=$1
>> +
>> + if [[ $IMGPROTO == fuse && $img == $SOCK_DIR/fuse-* ]]; then
>> + # Drop a FUSE export
>> + local df_output
>> + local i
>> + local image_file
>> + local index=''
>> + local timeout
>> +
>> + for i in "${!FUSE_EXPORTS[@]}"; do
>> + if [ "${FUSE_EXPORTS[i]}" = "$img" ]; then
>> + index=$i
>> + break
>> + fi
>> + done
>> +
>> + if [ -z "$index" ]; then
>> + # Probably gone already
>> + return 0
>> + fi
>> +
>> + kill "${FUSE_PIDS[index]}"
>> +
>> + # Wait until the mount is gone
>> + timeout=10 # *0.5 s
>> + while true; do
>> + # Will show the mount point; if the mount is still there,
>> + # it will be $img.
>> + df_output=$(df -T "$img" 2>/dev/null)
>
> 'df -T' doesn't seem to be portable.
>
> Well, neither is FUSE, so I guess it doesn't matter?
What a nice coincidence. :)
Though then again I have no idea what the -T is for, I don’t grep
anywhere for “fuse”, so we can just drop that -T.
Max
>> +
>> + # But df may also show an error ("Transpoint endpoint not
>> + # connected"), so retry in such cases
>> + if [ -n "$df_output" ]; then
>> + if ! echo "$df_output" | grep -q "$img"; then
>> + break
>> + fi
>> + fi
>> +
>> + sleep 0.5
>> +
>> + timeout=$((timeout - 1))
>> + if [ "$timeout" = 0 ]; then
>> + echo 'Failed to take down FUSE export' >&2
>> + return 1
>> + fi
>> + done
>> +
>> + rm -f "$img"
>> +
>> + unset "FUSE_PIDS[$index]"
>> + unset "FUSE_EXPORTS[$index]"
>> +
>> + image_file=$(echo "$img" | sed -e "s#$SOCK_DIR/fuse-#$TEST_DIR/#")
>> + _rm_test_img "$image_file"
>> + return
>> + fi
>> +
>> if [ "$IMGFMT" = "vmdk" ]; then
>> # Remove all the extents for vmdk
>> "$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \
>
> Kevin
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-10-15 14:46 ` Max Reitz
@ 2020-10-15 15:41 ` Kevin Wolf
2020-10-15 15:59 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 15:41 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3775 bytes --]
Am 15.10.2020 um 16:46 hat Max Reitz geschrieben:
> On 15.10.20 10:57, Kevin Wolf wrote:
> > Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> >> block-export-add type=fuse allows mounting block graph nodes via FUSE on
> >> some existing regular file. That file should then appears like a raw
> >> disk image, and accesses to it result in accesses to the exported BDS.
> >>
> >> Right now, we only implement the necessary block export functions to set
> >> it up and shut it down. We do not implement any access functions, so
> >> accessing the mount point only results in errors. This will be
> >> addressed by a followup patch.
> >>
> >> We keep a hash table of exported mount points, because we want to be
> >> able to detect when users try to use a mount point twice. This is
> >> because we invoke stat() to check whether the given mount point is a
> >> regular file, but if that file is served by ourselves (because it is
> >> already used as a mount point), then this stat() would have to be served
> >> by ourselves, too, which is impossible to do while we (as the caller)
> >> are waiting for it to settle. Therefore, keep track of mount point
> >> paths to at least catch the most obvious instances of that problem.
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> +/**
> >> + * Callback to be invoked when the FUSE session FD can be read from.
> >> + * (This is basically the FUSE event loop.)
> >> + */
> >> +static void read_from_fuse_export(void *opaque)
> >> +{
> >> + FuseExport *exp = opaque;
> >> + int ret;
> >> +
> >> + blk_exp_ref(&exp->common);
> >> +
> >> + ret = fuse_session_receive_buf(exp->fuse_session, &exp->fuse_buf);
> >
> > The fuse_session_loop() implementation seems to imply that we should
> > retry on EINTR.
>
> OK. I see you’re digging into libfuse already. :)
Well, I have to review against something, and the documentation tends to
be terse...
> >> + if (ret < 0) {
> >> + goto out;
> >> + }
> >> +
> >> + fuse_session_process_buf(exp->fuse_session, &exp->fuse_buf);
> >> +
> >> +out:
> >> + blk_exp_unref(&exp->common);
> >> +}
> >> +
> >> +static void fuse_export_shutdown(BlockExport *blk_exp)
> >> +{
> >> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
> >> +
> >> + if (exp->fuse_session) {
> >> + fuse_session_exit(exp->fuse_session);
> >> +
> >> + if (exp->mounted) {
> >> + fuse_session_unmount(exp->fuse_session);
> >> + exp->mounted = false;
> >> + }
> >> +
> >> + if (exp->fd_handler_set_up) {
> >> + aio_set_fd_handler(exp->common.ctx,
> >> + fuse_session_fd(exp->fuse_session), true,
> >> + NULL, NULL, NULL, NULL);
> >> + exp->fd_handler_set_up = false;
> >> + }
> >> +
> >> + fuse_session_destroy(exp->fuse_session);
> >> + exp->fuse_session = NULL;
> >
> > What happens if a request is still in flight?
> >
> > Oh, can't happen because the driver is fully synchronous after this
> > series. Fair enough, making it asynchronous can come on top of it.
>
> (I had multiple approaches of handling parallel requests, but none made
> a substantial performance difference, which is why I left the driver in
> the most simple form for this first proposal.)
I think the more relevant part is that we'd block the guest or anything
else running in the main loop while doing I/O.
Not a problem if you spawn a new qemu-storage-daemon just for this FUSE
export, but if you want to have multiple exports, or export from the
system emulator, you probably don't want to have synchronous operations.
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 03/20] fuse: Implement standard FUSE operations
2020-10-15 15:18 ` Max Reitz
@ 2020-10-15 15:58 ` Kevin Wolf
2020-10-15 16:04 ` Max Reitz
0 siblings, 1 reply; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 15:58 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 6972 bytes --]
Am 15.10.2020 um 17:18 hat Max Reitz geschrieben:
> On 15.10.20 11:46, Kevin Wolf wrote:
> > Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
> >> This makes the export actually useful instead of only producing errors
> >> whenever it is accessed.
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> +/**
> >> + * Handle client reads from the exported image.
> >> + */
> >> +static void fuse_read(fuse_req_t req, fuse_ino_t inode,
> >> + size_t size, off_t offset, struct fuse_file_info *fi)
> >> +{
> >> + FuseExport *exp = fuse_req_userdata(req);
> >> + int64_t length;
> >> + void *buf;
> >> + int ret;
> >> +
> >> + /**
> >> + * Clients will expect short reads at EOF, so we have to limit
> >> + * offset+size to the image length.
> >> + */
> >> + length = blk_getlength(exp->common.blk);
> >> + if (length < 0) {
> >> + fuse_reply_err(req, -length);
> >> + return;
> >> + }
> >> +
> >> + size = MIN(size, FUSE_MAX_BOUNCE_BYTES);
> >
> > "Read should send exactly the number of bytes requested except on EOF or
> > error, otherwise the rest of the data will be substituted with zeroes."
>
> :(
>
> > Do we corrupt huge reads with this, so that read() succeeds, but the
> > buffer is zeroed above 64M instead of containing the correct data? Maybe
> > we should return an error instead?
>
> Hm. It looks like there is a max_read option obeyed by the kernel
> driver, and it appears it’s set by implementing .init() and setting
> fuse_conn_info.max_read (also, “for the time being” it has to also set
> in the mount options passed to fuse_session_new()).
>
> I’m not sure whether that does anything, though. It appears that
> whenever I do a cached read, it caps out at 128k (which is what
> cuse_prep_data() in libfuse sets; though increasing that number there
> doesn’t change anything, so I think that’s just a coincidence), and with
> O_DIRECT, it caps out at 1M.
>
> But at least that would be grounds to return an error for >64M requests.
> (Limiting max_read to 64k does limit all read requests to 64k.)
Yes, setting max_read and making larger requests an error seems like a
good solution.
> Further investigation is needed.
If you want :-)
> > (It's kind of sad that we need a bounce buffer from which data is later
> > copied instead of being provided the right memory by the kernel.)
>
> Yes, it is.
>
> >> + if (offset + size > length) {
> >> + size = length - offset;
> >> + }
> >> +
> >> + buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
> >> + if (!buf) {
> >> + fuse_reply_err(req, ENOMEM);
> >> + return;
> >> + }
> >> +
> >> + ret = blk_pread(exp->common.blk, offset, buf, size);
> >> + if (ret >= 0) {
> >> + fuse_reply_buf(req, buf, size);
> >> + } else {
> >> + fuse_reply_err(req, -ret);
> >> + }
> >> +
> >> + qemu_vfree(buf);
> >> +}
> >> +
> >> +/**
> >> + * Handle client writes to the exported image.
> >> + */
> >> +static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
> >> + size_t size, off_t offset, struct fuse_file_info *fi)
> >> +{
> >> + FuseExport *exp = fuse_req_userdata(req);
> >> + int64_t length;
> >> + int ret;
> >> +
> >> + if (!exp->writable) {
> >> + fuse_reply_err(req, EACCES);
> >> + return;
> >> + }
> >> +
> >> + /**
> >> + * Clients will expect short writes at EOF, so we have to limit
> >> + * offset+size to the image length.
> >> + */
> >> + length = blk_getlength(exp->common.blk);
> >> + if (length < 0) {
> >> + fuse_reply_err(req, -length);
> >> + return;
> >> + }
> >> +
> >> + size = MIN(size, BDRV_REQUEST_MAX_BYTES);
> >
> > We're only supposed to do short writes on EOF, so this has a similar
> > problem as in fuse_read, except that BDRV_REQUEST_MAX_BYTES is much
> > higher and it's not specified what the resulting misbehaviour could be
> > (possibly the kernel not retrying write for the rest of the buffer?)
>
> As for reading above, we can (and should) probably set max_write.
Yes.
> >> + if (offset + size > length) {
> >> + size = length - offset;
> >> + }
> >> +
> >> + ret = blk_pwrite(exp->common.blk, offset, buf, size, 0);
> >> + if (ret >= 0) {
> >> + fuse_reply_write(req, size);
> >> + } else {
> >> + fuse_reply_err(req, -ret);
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * Let clients flush the exported image.
> >> + */
> >> +static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
> >> + struct fuse_file_info *fi)
> >> +{
> >> + FuseExport *exp = fuse_req_userdata(req);
> >> + int ret;
> >> +
> >> + ret = blk_flush(exp->common.blk);
> >> + fuse_reply_err(req, ret < 0 ? -ret : 0);
> >> +}
> >
> > This seems to be an implementation for .fsync rather than for .flush.
>
> Wouldn’t fsync entail a drain?
Hm, the libfuse documentation doesn't say anything about draining. I
suppose this is because if requests need to be drained, it will do so in
the kernel. But I haven't checked the code.
So I expect that calling fsync() on the lower layer does everything that
is needed. Which is bdrv_flush() in QEMU.
> Or is it the opposite, flush should just drain and not invoke
> blk_flush()? (Sorry, this all gets me confused every time.)
I'm just relying on the libfuse documentation there for flush:
"This is called on each close() of the opened file."
and
"NOTE: the name of the method is misleading, since (unlike fsync) the
filesystem is not forced to flush pending writes. One reason to flush
data is if the filesystem wants to return write errors during close.
However, such use is non-portable because POSIX does not require close
to wait for delayed I/O to complete."
> (Though I do think .fsync should both flush and drain.)
>
> > Hmm, or maybe actually for both? We usually do bdrv_flush() during
> > close, so it would be consistent to do the same here. It's our last
> > chance to report an error to the user before the file is closed.
>
> I don’t understand what you mean. What is “the same”? Closing the
> image? Or indeed having .flush() be implemented with blk_flush()?
Implementing .flush(), which will be called when the image is closed,
with blk_flush().
And still doing blk_flush() for .fsync, of course.
> Do you mean that other parties may do the same as qemu does, i.e.
> flush files before they are closed, which is why we should anticipate
> the same and give users a chance to see errors?
I'm not exactly sure what failure of .flush() would look like for users.
Probably close() failing, so they don't have to do anything special,
just check their return values.
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-10-15 15:41 ` Kevin Wolf
@ 2020-10-15 15:59 ` Max Reitz
2020-10-15 17:01 ` Kevin Wolf
0 siblings, 1 reply; 46+ messages in thread
From: Max Reitz @ 2020-10-15 15:59 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 1916 bytes --]
On 15.10.20 17:41, Kevin Wolf wrote:
> Am 15.10.2020 um 16:46 hat Max Reitz geschrieben:
>> On 15.10.20 10:57, Kevin Wolf wrote:
>>> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
[...]
>>>> +static void fuse_export_shutdown(BlockExport *blk_exp)
>>>> +{
>>>> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
>>>> +
>>>> + if (exp->fuse_session) {
>>>> + fuse_session_exit(exp->fuse_session);
>>>> +
>>>> + if (exp->mounted) {
>>>> + fuse_session_unmount(exp->fuse_session);
>>>> + exp->mounted = false;
>>>> + }
>>>> +
>>>> + if (exp->fd_handler_set_up) {
>>>> + aio_set_fd_handler(exp->common.ctx,
>>>> + fuse_session_fd(exp->fuse_session), true,
>>>> + NULL, NULL, NULL, NULL);
>>>> + exp->fd_handler_set_up = false;
>>>> + }
>>>> +
>>>> + fuse_session_destroy(exp->fuse_session);
>>>> + exp->fuse_session = NULL;
>>>
>>> What happens if a request is still in flight?
>>>
>>> Oh, can't happen because the driver is fully synchronous after this
>>> series. Fair enough, making it asynchronous can come on top of it.
>>
>> (I had multiple approaches of handling parallel requests, but none made
>> a substantial performance difference, which is why I left the driver in
>> the most simple form for this first proposal.)
>
> I think the more relevant part is that we'd block the guest or anything
> else running in the main loop while doing I/O.
>
> Not a problem if you spawn a new qemu-storage-daemon just for this FUSE
> export, but if you want to have multiple exports, or export from the
> system emulator, you probably don't want to have synchronous operations.
Ah, hm. Hmm. O:)
Does NBD work any different, though? I had always assumed it runs in
the BB’s context.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 03/20] fuse: Implement standard FUSE operations
2020-10-15 15:58 ` Kevin Wolf
@ 2020-10-15 16:04 ` Max Reitz
0 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-10-15 16:04 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 5046 bytes --]
On 15.10.20 17:58, Kevin Wolf wrote:
> Am 15.10.2020 um 17:18 hat Max Reitz geschrieben:
>> On 15.10.20 11:46, Kevin Wolf wrote:
>>> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>>>> This makes the export actually useful instead of only producing errors
>>>> whenever it is accessed.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>
>>>> +/**
>>>> + * Handle client reads from the exported image.
>>>> + */
>>>> +static void fuse_read(fuse_req_t req, fuse_ino_t inode,
>>>> + size_t size, off_t offset, struct fuse_file_info *fi)
>>>> +{
>>>> + FuseExport *exp = fuse_req_userdata(req);
>>>> + int64_t length;
>>>> + void *buf;
>>>> + int ret;
>>>> +
>>>> + /**
>>>> + * Clients will expect short reads at EOF, so we have to limit
>>>> + * offset+size to the image length.
>>>> + */
>>>> + length = blk_getlength(exp->common.blk);
>>>> + if (length < 0) {
>>>> + fuse_reply_err(req, -length);
>>>> + return;
>>>> + }
>>>> +
>>>> + size = MIN(size, FUSE_MAX_BOUNCE_BYTES);
>>>
>>> "Read should send exactly the number of bytes requested except on EOF or
>>> error, otherwise the rest of the data will be substituted with zeroes."
>>
>> :(
>>
>>> Do we corrupt huge reads with this, so that read() succeeds, but the
>>> buffer is zeroed above 64M instead of containing the correct data? Maybe
>>> we should return an error instead?
>>
>> Hm. It looks like there is a max_read option obeyed by the kernel
>> driver, and it appears it’s set by implementing .init() and setting
>> fuse_conn_info.max_read (also, “for the time being” it has to also set
>> in the mount options passed to fuse_session_new()).
>>
>> I’m not sure whether that does anything, though. It appears that
>> whenever I do a cached read, it caps out at 128k (which is what
>> cuse_prep_data() in libfuse sets; though increasing that number there
>> doesn’t change anything, so I think that’s just a coincidence), and with
>> O_DIRECT, it caps out at 1M.
>>
>> But at least that would be grounds to return an error for >64M requests.
>> (Limiting max_read to 64k does limit all read requests to 64k.)
>
> Yes, setting max_read and making larger requests an error seems like a
> good solution.
>
>> Further investigation is needed.
>
> If you want :-)
Not really, but 128k per request is a bit sad.
[...]
>>>> +/**
>>>> + * Let clients flush the exported image.
>>>> + */
>>>> +static void fuse_flush(fuse_req_t req, fuse_ino_t inode,
>>>> + struct fuse_file_info *fi)
>>>> +{
>>>> + FuseExport *exp = fuse_req_userdata(req);
>>>> + int ret;
>>>> +
>>>> + ret = blk_flush(exp->common.blk);
>>>> + fuse_reply_err(req, ret < 0 ? -ret : 0);
>>>> +}
>>>
>>> This seems to be an implementation for .fsync rather than for .flush.
>>
>> Wouldn’t fsync entail a drain?
>
> Hm, the libfuse documentation doesn't say anything about draining. I
> suppose this is because if requests need to be drained, it will do so in
> the kernel. But I haven't checked the code.
Hmm, well, yeah... A sync doesn’t necessarily mean settling all
in-flight requests.
> So I expect that calling fsync() on the lower layer does everything that
> is needed. Which is bdrv_flush() in QEMU.
>
>> Or is it the opposite, flush should just drain and not invoke
>> blk_flush()? (Sorry, this all gets me confused every time.)
>
> I'm just relying on the libfuse documentation there for flush:
>
> "This is called on each close() of the opened file."
Ah, OK.
> and
>
> "NOTE: the name of the method is misleading, since (unlike fsync) the
> filesystem is not forced to flush pending writes. One reason to flush
> data is if the filesystem wants to return write errors during close.
> However, such use is non-portable because POSIX does not require close
> to wait for delayed I/O to complete."
>
>> (Though I do think .fsync should both flush and drain.)
>>
>>> Hmm, or maybe actually for both? We usually do bdrv_flush() during
>>> close, so it would be consistent to do the same here. It's our last
>>> chance to report an error to the user before the file is closed.
>>
>> I don’t understand what you mean. What is “the same”? Closing the
>> image? Or indeed having .flush() be implemented with blk_flush()?
>
> Implementing .flush(), which will be called when the image is closed,
> with blk_flush().
>
> And still doing blk_flush() for .fsync, of course.
OK.
>> Do you mean that other parties may do the same as qemu does, i.e.
>> flush files before they are closed, which is why we should anticipate
>> the same and give users a chance to see errors?
>
> I'm not exactly sure what failure of .flush() would look like for users.
> Probably close() failing, so they don't have to do anything special,
> just check their return values.
Checking return values on close()? Sounds special to me. O:)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE
2020-10-15 12:01 ` Kevin Wolf
@ 2020-10-15 16:47 ` Max Reitz
0 siblings, 0 replies; 46+ messages in thread
From: Max Reitz @ 2020-10-15 16:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1.1: Type: text/plain, Size: 2124 bytes --]
On 15.10.20 14:01, Kevin Wolf wrote:
> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>> Based-on: <20200907182011.521007-1-kwolf@redhat.com>
>> (“block/export: Add infrastructure and QAPI for block exports”)
>>
>> (Though its patch 16 needs a s/= \&blk_exp_nbd/= drv/ on top.)
>>
>> v1: https://lists.nongnu.org/archive/html/qemu-block/2019-12/msg00451.html
>>
>> Branch: https://github.com/XanClic/qemu.git fuse-exports-v2
>> Branch: https://git.xanclic.moe/XanClic/qemu.git fuse-exports-v2
>>
>>
>> Hi,
>>
>> Ever since I found out that you can mount FUSE filesystems on regular
>> files (not just directories), I had the idea of adding FUSE block
>> exports to qemu where you can export block nodes as raw images. The
>> best thing is that you’d be able to mount an image on itself, so
>> whatever format it may be in, qemu lets it appear as a raw image (and
>> you can then use regular tools like dd on it).
>>
>> The performance is quite bad so far, but we can always try to improve it
>> if the need arises. For now I consider it mostly a cute feature to get
>> easy access to the raw contents of image files in any format (without
>> requiring root rights).
>>
>> In this version (as opposed to v1 linked above), I integrated the FUSE
>> export code into Kevin’s proposed common infrastructure for block
>> exports.
>
> Patches 5-16, 19 and 20:
>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Thanks!
So far, (just FYI,) I’ve tried to address your remarks, gathered them in
a single fat commit, and pushed everything here:
https://github.com/XanClic/qemu/tree/fuse-exports-next
(There’s also something about two bugs in fuse_fallocate(), where there
are these two I/O loops (one for blk_pwrite_zeroes(), one for
blk_pdiscard()), where I both forgot to increment @offset (alongside
decrementing @length). Oops.)
(And I’ve renamed init_fuse() to init_exports_table(), because that got
really confusing with fuse_init().)
I’ll split that up, squash it into the respective patches, and send v3
when I get back from PTO.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE
2020-10-15 15:59 ` Max Reitz
@ 2020-10-15 17:01 ` Kevin Wolf
0 siblings, 0 replies; 46+ messages in thread
From: Kevin Wolf @ 2020-10-15 17:01 UTC (permalink / raw)
To: Max Reitz; +Cc: Stefan Hajnoczi, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2262 bytes --]
Am 15.10.2020 um 17:59 hat Max Reitz geschrieben:
> On 15.10.20 17:41, Kevin Wolf wrote:
> > Am 15.10.2020 um 16:46 hat Max Reitz geschrieben:
> >> On 15.10.20 10:57, Kevin Wolf wrote:
> >>> Am 22.09.2020 um 12:49 hat Max Reitz geschrieben:
>
> [...]
>
> >>>> +static void fuse_export_shutdown(BlockExport *blk_exp)
> >>>> +{
> >>>> + FuseExport *exp = container_of(blk_exp, FuseExport, common);
> >>>> +
> >>>> + if (exp->fuse_session) {
> >>>> + fuse_session_exit(exp->fuse_session);
> >>>> +
> >>>> + if (exp->mounted) {
> >>>> + fuse_session_unmount(exp->fuse_session);
> >>>> + exp->mounted = false;
> >>>> + }
> >>>> +
> >>>> + if (exp->fd_handler_set_up) {
> >>>> + aio_set_fd_handler(exp->common.ctx,
> >>>> + fuse_session_fd(exp->fuse_session), true,
> >>>> + NULL, NULL, NULL, NULL);
> >>>> + exp->fd_handler_set_up = false;
> >>>> + }
> >>>> +
> >>>> + fuse_session_destroy(exp->fuse_session);
> >>>> + exp->fuse_session = NULL;
> >>>
> >>> What happens if a request is still in flight?
> >>>
> >>> Oh, can't happen because the driver is fully synchronous after this
> >>> series. Fair enough, making it asynchronous can come on top of it.
> >>
> >> (I had multiple approaches of handling parallel requests, but none made
> >> a substantial performance difference, which is why I left the driver in
> >> the most simple form for this first proposal.)
> >
> > I think the more relevant part is that we'd block the guest or anything
> > else running in the main loop while doing I/O.
> >
> > Not a problem if you spawn a new qemu-storage-daemon just for this FUSE
> > export, but if you want to have multiple exports, or export from the
> > system emulator, you probably don't want to have synchronous operations.
>
> Ah, hm. Hmm. O:)
>
> Does NBD work any different, though? I had always assumed it runs in
> the BB’s context.
Yes, it runs in the BB's AioContext, but it's coroutine based, so
instead of blocking, it just yields and lets other stuff in the same
event loop make progress while it's waiting for its I/O.
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2020-10-15 17:08 UTC | newest]
Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 10:49 [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Max Reitz
2020-09-22 10:49 ` [PATCH v2 01/20] configure: Detect libfuse Max Reitz
2020-09-22 11:14 ` Thomas Huth
2020-09-22 11:21 ` Paolo Bonzini
2020-09-22 11:46 ` Max Reitz
2020-09-22 15:37 ` Max Reitz
2020-09-22 15:45 ` Paolo Bonzini
2020-09-22 10:49 ` [PATCH v2 02/20] fuse: Allow exporting BDSs via FUSE Max Reitz
2020-10-15 8:57 ` Kevin Wolf
2020-10-15 14:46 ` Max Reitz
2020-10-15 15:41 ` Kevin Wolf
2020-10-15 15:59 ` Max Reitz
2020-10-15 17:01 ` Kevin Wolf
2020-09-22 10:49 ` [PATCH v2 03/20] fuse: Implement standard FUSE operations Max Reitz
2020-10-15 9:46 ` Kevin Wolf
2020-10-15 15:18 ` Max Reitz
2020-10-15 15:58 ` Kevin Wolf
2020-10-15 16:04 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 04/20] fuse: Allow growable exports Max Reitz
2020-10-15 10:41 ` Kevin Wolf
2020-10-15 15:20 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 05/20] fuse: (Partially) implement fallocate() Max Reitz
2020-09-22 10:49 ` [PATCH v2 06/20] fuse: Implement hole detection through lseek Max Reitz
2020-09-22 10:49 ` [PATCH v2 07/20] iotests: Do not needlessly filter _make_test_img Max Reitz
2020-09-22 10:49 ` [PATCH v2 08/20] iotests: Do not pipe _make_test_img Max Reitz
2020-09-22 10:49 ` [PATCH v2 09/20] iotests: Use convert -n in some cases Max Reitz
2020-09-22 10:49 ` [PATCH v2 10/20] iotests/046: Avoid renaming images Max Reitz
2020-09-22 10:49 ` [PATCH v2 11/20] iotests: Derive image names from $TEST_IMG Max Reitz
2020-09-22 10:49 ` [PATCH v2 12/20] iotests/091: Use _cleanup_qemu instad of "wait" Max Reitz
2020-09-22 10:49 ` [PATCH v2 13/20] iotests: Restrict some Python tests to file Max Reitz
2020-09-22 10:49 ` [PATCH v2 14/20] iotests: Let _make_test_img guess $TEST_IMG_FILE Max Reitz
2020-09-22 10:49 ` [PATCH v2 15/20] iotests/287: Clean up subshell test image Max Reitz
2020-09-22 10:49 ` [PATCH v2 16/20] storage-daemon: Call bdrv_close_all() on exit Max Reitz
2020-09-22 10:49 ` [PATCH v2 17/20] iotests: Give access to the qemu-storage-daemon Max Reitz
2020-10-15 11:27 ` Kevin Wolf
2020-10-15 15:22 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 18/20] iotests: Allow testing FUSE exports Max Reitz
2020-10-15 11:43 ` Kevin Wolf
2020-10-15 15:27 ` Max Reitz
2020-09-22 10:49 ` [PATCH v2 19/20] iotests: Enable fuse for many tests Max Reitz
2020-09-22 10:49 ` [PATCH v2 20/20] iotests/308: Add test for FUSE exports Max Reitz
2020-09-22 15:58 ` [PATCH v2 00/20] block/export: Allow exporting BDSs via FUSE Daniel P. Berrangé
2020-09-23 7:21 ` Max Reitz
2020-09-23 9:08 ` Stefan Hajnoczi
2020-10-15 12:01 ` Kevin Wolf
2020-10-15 16:47 ` Max Reitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.