[PULL 05/13] io: follow coroutine AioContext in qio_channel_yield()
From: Stefan Hajnoczi The ongoing QEMU multi-queue block layer effort makes it possible for multiple threads to process I/O in parallel. The nbd block driver is not compatible with the multi-queue block layer yet because QIOChannel cannot be used easily from coroutines running in multiple threads. This series changes the QIOChannel API to make that possible. In the current API, calling qio_channel_attach_aio_context() sets the AioContext where qio_channel_yield() installs an fd handler prior to yielding: qio_channel_attach_aio_context(ioc, my_ctx); ... qio_channel_yield(ioc); // my_ctx is used here ... qio_channel_detach_aio_context(ioc); This API design has limitations: reading and writing must be done in the same AioContext and moving between AioContexts involves a cumbersome sequence of API calls that is not suitable for doing on a per-request basis. There is no fundamental reason why a QIOChannel needs to run within the same AioContext every time qio_channel_yield() is called. QIOChannel only uses the AioContext while inside qio_channel_yield(). The rest of the time, QIOChannel is independent of any AioContext. In the new API, qio_channel_yield() queries the AioContext from the current coroutine using qemu_coroutine_get_aio_context(). There is no need to explicitly attach/detach AioContexts anymore and qio_channel_attach_aio_context() and qio_channel_detach_aio_context() are gone. One coroutine can read from the QIOChannel while another coroutine writes from a different AioContext. This API change allows the nbd block driver to use QIOChannel from any thread. It's important to keep in mind that the block driver already synchronizes QIOChannel access and ensures that two coroutines never read simultaneously or write simultaneously. This patch updates all users of qio_channel_attach_aio_context() to the new API. Most conversions are simple, but vhost-user-server requires a new qemu_coroutine_yield() call to quiesce the vu_client_trip() coroutine when not attached to any AioContext. While the API is has become simpler, there is one wart: QIOChannel has a special case for the iohandler AioContext (used for handlers that must not run in nested event loops). I didn't find an elegant way preserve that behavior, so I added a new API called qio_channel_set_follow_coroutine_ctx(ioc, true|false) for opting in to the new AioContext model. By default QIOChannel uses the iohandler AioHandler. Code that formerly called qio_channel_attach_aio_context() now calls qio_channel_set_follow_coroutine_ctx(ioc, true) once after the QIOChannel is created. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Acked-by: Daniel P. Berrangé Message-ID: <20230830224802.493686-5-stefa...@redhat.com> [eblake: also fix migration/rdma.c] Signed-off-by: Eric Blake --- include/io/channel-util.h| 23 ++ include/io/channel.h | 69 -- include/qemu/vhost-user-server.h | 1 + block/nbd.c | 11 +-- io/channel-command.c | 10 ++- io/channel-file.c| 9 ++- io/channel-null.c| 3 +- io/channel-socket.c | 9 ++- io/channel-tls.c | 6 +- io/channel-util.c| 24 +++ io/channel.c | 120 ++- migration/channel-block.c| 3 +- migration/rdma.c | 25 +++ nbd/server.c | 14 +--- scsi/qemu-pr-helper.c| 4 +- util/vhost-user-server.c | 27 +-- 16 files changed, 229 insertions(+), 129 deletions(-) diff --git a/include/io/channel-util.h b/include/io/channel-util.h index a5d720d9a04..fa18a3756d8 100644 --- a/include/io/channel-util.h +++ b/include/io/channel-util.h @@ -49,4 +49,27 @@ QIOChannel *qio_channel_new_fd(int fd, Error **errp); +/** + * qio_channel_util_set_aio_fd_handler: + * @read_fd: the file descriptor for the read handler + * @read_ctx: the AioContext for the read handler + * @io_read: the read handler + * @write_fd: the file descriptor for the write handler + * @write_ctx: the AioContext for the write handler + * @io_write: the write handler + * @opaque: the opaque argument to the read and write handler + * + * Set the read and write handlers when @read_ctx and @write_ctx are non-NULL, + * respectively. To leave a handler in its current state, pass a NULL + * AioContext. To clear a handler, pass a non-NULL AioContext and a NULL + * handler. + */ +void qio_channel_util_set_aio_fd_handler(int read_fd, + AioContext *read_ctx, + IOHandler *io_read, + int write_fd, + AioContext *write_ctx, + IOHandler *io_write, + void *opaque); + #endif /* QIO_CHANNEL_UTIL_H */
[PULL 07/13] qemu-nbd: improve error message for dup2 error
From: "Denis V. Lunev"
This error happens if we are not able to close the pipe to the
parent (to trace errors in the child process) and assign stderr to
/dev/null as required by the daemonizing convention.
Signed-off-by: Denis V. Lunev
Suggested-by: Eric Blake
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
Message-ID: <20230906093210.339585-2-...@openvz.org>
Reviewed-by: Eric Blake
[eblake: commit message grammar]
Signed-off-by: Eric Blake
---
qemu-nbd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index b47459f781d..e2480061a16 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -324,7 +324,7 @@ static void *nbd_client_thread(void *arg)
} else {
/* Close stderr so that the qemu-nbd process exits. */
if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
-error_report("Could not set stderr to /dev/null: %s",
+error_report("Could not release pipe to parent: %s",
strerror(errno));
exit(EXIT_FAILURE);
}
@@ -1181,7 +1181,7 @@ int main(int argc, char **argv)
if (fork_process) {
if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
-error_report("Could not set stderr to /dev/null: %s",
+error_report("Could not release pipe to parent: %s",
strerror(errno));
exit(EXIT_FAILURE);
}
--
2.41.0
[PULL 11/13] qemu-nbd: invent nbd_client_release_pipe() helper
From: "Denis V. Lunev"
Move the code from main() and nbd_client_thread() into the specific
helper. This code is going to be grown.
Signed-off-by: Denis V. Lunev
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
Message-ID: <20230906093210.339585-6-...@openvz.org>
Reviewed-by: Eric Blake
Signed-off-by: Eric Blake
---
qemu-nbd.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 86bb2f04e24..7c4e22def17 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -259,6 +259,16 @@ struct NbdClientOpts {
bool verbose;
};
+static void nbd_client_release_pipe(void)
+{
+/* Close stderr so that the qemu-nbd process exits. */
+if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
+error_report("Could not release pipe to parent: %s",
+ strerror(errno));
+exit(EXIT_FAILURE);
+}
+}
+
#if HAVE_NBD_DEVICE
static void *show_parts(void *arg)
{
@@ -322,12 +332,7 @@ static void *nbd_client_thread(void *arg)
fprintf(stderr, "NBD device %s is now connected to %s\n",
opts->device, opts->srcpath);
} else {
-/* Close stderr so that the qemu-nbd process exits. */
-if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
-error_report("Could not release pipe to parent: %s",
- strerror(errno));
-exit(EXIT_FAILURE);
-}
+nbd_client_release_pipe();
}
if (nbd_client(fd) < 0) {
@@ -1176,11 +1181,7 @@ int main(int argc, char **argv)
}
if (opts.fork_process) {
-if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
-error_report("Could not release pipe to parent: %s",
- strerror(errno));
-exit(EXIT_FAILURE);
-}
+nbd_client_release_pipe();
}
state = RUNNING;
--
2.41.0
[PULL 02/13] nbd: drop unused nbd_receive_negotiate() aio_context argument
From: Stefan Hajnoczi
aio_context is always NULL, so drop it.
Suggested-by: Fabiano Rosas
Signed-off-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
Message-ID: <20230830224802.493686-2-stefa...@redhat.com>
Signed-off-by: Eric Blake
---
include/block/nbd.h | 3 +--
nbd/client-connection.c | 3 +--
nbd/client.c| 5 ++---
qemu-nbd.c | 4 ++--
4 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 4428bcffbb9..f672b76173b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -324,8 +324,7 @@ typedef struct NBDExportInfo {
char **contexts;
} NBDExportInfo;
-int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
- QCryptoTLSCreds *tlscreds,
+int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
const char *hostname, QIOChannel **outioc,
NBDExportInfo *info, Error **errp);
void nbd_free_export_list(NBDExportInfo *info, int count);
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 3d14296c042..aafb3d0fb43 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -146,8 +146,7 @@ static int nbd_connect(QIOChannelSocket *sioc,
SocketAddress *addr,
return 0;
}
-ret = nbd_receive_negotiate(NULL, QIO_CHANNEL(sioc), tlscreds,
-tlshostname,
+ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), tlscreds, tlshostname,
outioc, info, errp);
if (ret < 0) {
/*
diff --git a/nbd/client.c b/nbd/client.c
index 479208d5d9d..16ec10c8a91 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1014,8 +1014,7 @@ static int nbd_negotiate_finish_oldstyle(QIOChannel *ioc,
NBDExportInfo *info,
* Returns: negative errno: failure talking to server
* 0: server is connected
*/
-int nbd_receive_negotiate(AioContext *aio_context, QIOChannel *ioc,
- QCryptoTLSCreds *tlscreds,
+int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
const char *hostname, QIOChannel **outioc,
NBDExportInfo *info, Error **errp)
{
@@ -1027,7 +1026,7 @@ int nbd_receive_negotiate(AioContext *aio_context,
QIOChannel *ioc,
assert(info->name && strlen(info->name) <= NBD_MAX_STRING_SIZE);
trace_nbd_receive_negotiate_name(info->name);
-result = nbd_start_negotiate(aio_context, ioc, tlscreds, hostname, outioc,
+result = nbd_start_negotiate(NULL, ioc, tlscreds, hostname, outioc,
info->structured_reply, , errp);
if (result < 0) {
return result;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index aaccaa33184..b47459f781d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -295,8 +295,8 @@ static void *nbd_client_thread(void *arg)
goto out;
}
-if (nbd_receive_negotiate(NULL, QIO_CHANNEL(sioc),
- NULL, NULL, NULL, , _error) < 0) {
+if (nbd_receive_negotiate(QIO_CHANNEL(sioc), NULL, NULL, NULL,
+ , _error) < 0) {
if (local_error) {
error_report_err(local_error);
}
--
2.41.0
[PULL 13/13] qemu-nbd: document -v behavior in respect to --fork in man
From: "Denis V. Lunev" Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy Message-ID: <20230906093210.339585-8-...@openvz.org> Reviewed-by: Eric Blake [eblake: Wording improvement] Signed-off-by: Eric Blake --- docs/tools/qemu-nbd.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst index faf6349ea51..329f44d9895 100644 --- a/docs/tools/qemu-nbd.rst +++ b/docs/tools/qemu-nbd.rst @@ -197,7 +197,9 @@ driver options if :option:`--image-opts` is specified. .. option:: -v, --verbose - Display extra debugging information. + Display extra debugging information. This option also keeps the original + *STDERR* stream open if the ``qemu-nbd`` process is daemonized due to + other options like :option:`--fork` or :option:`-c`. .. option:: -h, --help -- 2.41.0
[PULL 10/13] qemu-nbd: put saddr into into struct NbdClientOpts
From: "Denis V. Lunev"
We pass other parameters into nbd_client_thread() in this way. This patch
makes the code more consistent.
Signed-off-by: Denis V. Lunev
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
Message-ID: <20230906093210.339585-5-...@openvz.org>
Reviewed-by: Eric Blake
Signed-off-by: Eric Blake
---
qemu-nbd.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 16c59424f13..86bb2f04e24 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -73,7 +73,6 @@
#define MBR_SIZE 512
-static SocketAddress *saddr;
static int persistent = 0;
static enum { RUNNING, TERMINATE, TERMINATED } state;
static int shared = 1;
@@ -255,6 +254,7 @@ static int qemu_nbd_client_list(SocketAddress *saddr,
QCryptoTLSCreds *tls,
struct NbdClientOpts {
char *device;
char *srcpath;
+SocketAddress *saddr;
bool fork_process;
bool verbose;
};
@@ -289,7 +289,7 @@ static void *nbd_client_thread(void *arg)
sioc = qio_channel_socket_new();
if (qio_channel_socket_connect_sync(sioc,
-saddr,
+opts->saddr,
_error) < 0) {
error_report_err(local_error);
goto out;
@@ -591,6 +591,7 @@ int main(int argc, char **argv)
.verbose = false,
.device = NULL,
.srcpath = NULL,
+.saddr = NULL,
};
#ifdef CONFIG_POSIX
@@ -892,8 +893,8 @@ int main(int argc, char **argv)
}
if (list) {
-saddr = nbd_build_socket_address(sockpath, bindto, port);
-return qemu_nbd_client_list(saddr, tlscreds,
+opts.saddr = nbd_build_socket_address(sockpath, bindto, port);
+return qemu_nbd_client_list(opts.saddr, tlscreds,
tlshostname ? tlshostname : bindto);
}
@@ -1024,8 +1025,8 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
#endif
-saddr = nbd_build_socket_address(sockpath, bindto, port);
-if (qio_net_listener_open_sync(server, saddr, backlog,
+opts.saddr = nbd_build_socket_address(sockpath, bindto, port);
+if (qio_net_listener_open_sync(server, opts.saddr, backlog,
_err) < 0) {
object_unref(OBJECT(server));
error_report_err(local_err);
--
2.41.0
[PULL 01/13] qemu-iotests/197: use more generic commands for formats other than qcow2
From: Andrey Drobyshev
In the previous commit e2f938265e0 ("tests/qemu-iotests/197: add
testcase for CoR with subclusters") we've introduced a new testcase for
copy-on-read with subclusters. Test 197 always forces qcow2 as the top
image, but allows backing image to be in any format. That last test
case didn't meet these requirements, so let's fix it by using more
generic "qemu-io -c map" command.
Signed-off-by: Andrey Drobyshev
Message-ID: <20230907220718.983430-1-andrey.drobys...@virtuozzo.com>
Tested-by: Eric Blake
Reviewed-by: Eric Blake
Signed-off-by: Eric Blake
---
tests/qemu-iotests/197 | 8
tests/qemu-iotests/197.out | 18 --
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index f07a9da136a..8ad2bdb035e 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -136,18 +136,18 @@ IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \
$QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io
# Allocate individual subclusters in the top image, and not the whole cluster
-$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" "$TEST_WRAP" \
+$QEMU_IO -f qcow2 -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K"
"$TEST_WRAP" \
| _filter_qemu_io
# Only 2 subclusters should be allocated in the top image at this point
-$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+$QEMU_IO -f qcow2 -c map "$TEST_WRAP"
# Actual copy-on-read operation
-$QEMU_IO -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
+$QEMU_IO -f qcow2 -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
# And here we should have 4 subclusters allocated right in the middle of the
# top image. Make sure the whole cluster remains unallocated
-$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+$QEMU_IO -f qcow2 -c map "$TEST_WRAP"
_check_test_img
diff --git a/tests/qemu-iotests/197.out b/tests/qemu-iotests/197.out
index 8f34a30afea..86c57b51d30 100644
--- a/tests/qemu-iotests/197.out
+++ b/tests/qemu-iotests/197.out
@@ -42,17 +42,15 @@ wrote 2048/2048 bytes at offset 28672
2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 2048/2048 bytes at offset 34816
2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0 0x7000 TEST_DIR/t.IMGFMT
-0x7000 0x800 TEST_DIR/t.wrap.IMGFMT
-0x7800 0x1000 TEST_DIR/t.IMGFMT
-0x8800 0x800 TEST_DIR/t.wrap.IMGFMT
-0x9000 0x7000 TEST_DIR/t.IMGFMT
+28 KiB (0x7000) bytes not allocated at offset 0 bytes (0x0)
+2 KiB (0x800) bytes allocated at offset 28 KiB (0x7000)
+4 KiB (0x1000) bytes not allocated at offset 30 KiB (0x7800)
+2 KiB (0x800) bytes allocated at offset 34 KiB (0x8800)
+28 KiB (0x7000) bytes not allocated at offset 36 KiB (0x9000)
read 4096/4096 bytes at offset 30720
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0 0x7000 TEST_DIR/t.IMGFMT
-0x7000 0x2000 TEST_DIR/t.wrap.IMGFMT
-0x9000 0x7000 TEST_DIR/t.IMGFMT
+28 KiB (0x7000) bytes not allocated at offset 0 bytes (0x0)
+8 KiB (0x2000) bytes allocated at offset 28 KiB (0x7000)
+28 KiB (0x7000) bytes not allocated at offset 36 KiB (0x9000)
No errors were found on the image.
*** done
--
2.41.0
[PULL 12/13] qemu-nbd: Restore "qemu-nbd -v --fork" output
From: "Denis V. Lunev"
Closing stderr earlier is good for daemonized qemu-nbd under ssh
earlier, but breaks the case where -v is being used to track what is
happening in the server, as in iotest 233.
When we know we are verbose, we should preserve original stderr and
restore it once the setup stage is done. This commit restores the
original behavior with -v option. In this case original output
inside the test is kept intact.
Reported-by: Kevin Wolf
Signed-off-by: Denis V. Lunev
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
CC: Hanna Reitz
CC: Mike Maslenkin
Fixes: 5c56dd27a2 ("qemu-nbd: fix regression with qemu-nbd --fork run over ssh")
Message-ID: <20230906093210.339585-7-...@openvz.org>
Reviewed-by: Eric Blake
Tested-by: Eric Blake
Signed-off-by: Eric Blake
---
qemu-nbd.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7c4e22def17..1cdc41ed292 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -255,18 +255,23 @@ struct NbdClientOpts {
char *device;
char *srcpath;
SocketAddress *saddr;
+int stderr;
bool fork_process;
bool verbose;
};
-static void nbd_client_release_pipe(void)
+static void nbd_client_release_pipe(int old_stderr)
{
/* Close stderr so that the qemu-nbd process exits. */
-if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
+if (dup2(old_stderr, STDERR_FILENO) < 0) {
error_report("Could not release pipe to parent: %s",
strerror(errno));
exit(EXIT_FAILURE);
}
+if (old_stderr != STDOUT_FILENO && close(old_stderr) < 0) {
+error_report("Could not release qemu-nbd: %s", strerror(errno));
+exit(EXIT_FAILURE);
+}
}
#if HAVE_NBD_DEVICE
@@ -332,7 +337,7 @@ static void *nbd_client_thread(void *arg)
fprintf(stderr, "NBD device %s is now connected to %s\n",
opts->device, opts->srcpath);
} else {
-nbd_client_release_pipe();
+nbd_client_release_pipe(opts->stderr);
}
if (nbd_client(fd) < 0) {
@@ -597,6 +602,7 @@ int main(int argc, char **argv)
.device = NULL,
.srcpath = NULL,
.saddr = NULL,
+.stderr = STDOUT_FILENO,
};
#ifdef CONFIG_POSIX
@@ -951,6 +957,16 @@ int main(int argc, char **argv)
close(stderr_fd[0]);
+/* Remember parent's stderr if we will be restoring it. */
+if (opts.verbose /* fork_process is set */) {
+opts.stderr = dup(STDERR_FILENO);
+if (opts.stderr < 0) {
+error_report("Could not dup original stderr: %s",
+ strerror(errno));
+exit(EXIT_FAILURE);
+}
+}
+
ret = qemu_daemon(1, 0);
saved_errno = errno;/* dup2 will overwrite error below */
@@ -1181,7 +1197,7 @@ int main(int argc, char **argv)
}
if (opts.fork_process) {
-nbd_client_release_pipe();
+nbd_client_release_pipe(opts.stderr);
}
state = RUNNING;
--
2.41.0
[PULL 03/13] nbd: drop unused nbd_start_negotiate() aio_context argument
From: Stefan Hajnoczi
aio_context is always NULL, so drop it.
Suggested-by: Fabiano Rosas
Signed-off-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
Reviewed-by: Philippe Mathieu-Daudé
Message-ID: <20230830224802.493686-3-stefa...@redhat.com>
Signed-off-by: Eric Blake
---
nbd/client.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 16ec10c8a91..bd7e2001366 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -877,8 +877,7 @@ static int nbd_list_meta_contexts(QIOChannel *ioc,
* Returns: negative errno: failure talking to server
* non-negative: enum NBDMode describing server abilities
*/
-static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
- QCryptoTLSCreds *tlscreds,
+static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
const char *hostname, QIOChannel **outioc,
bool structured_reply, bool *zeroes,
Error **errp)
@@ -946,10 +945,6 @@ static int nbd_start_negotiate(AioContext *aio_context,
QIOChannel *ioc,
return -EINVAL;
}
ioc = *outioc;
-if (aio_context) {
-qio_channel_set_blocking(ioc, false, NULL);
-qio_channel_attach_aio_context(ioc, aio_context);
-}
} else {
error_setg(errp, "Server does not support STARTTLS");
return -EINVAL;
@@ -1026,7 +1021,7 @@ int nbd_receive_negotiate(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
assert(info->name && strlen(info->name) <= NBD_MAX_STRING_SIZE);
trace_nbd_receive_negotiate_name(info->name);
-result = nbd_start_negotiate(NULL, ioc, tlscreds, hostname, outioc,
+result = nbd_start_negotiate(ioc, tlscreds, hostname, outioc,
info->structured_reply, , errp);
if (result < 0) {
return result;
@@ -1149,7 +1144,7 @@ int nbd_receive_export_list(QIOChannel *ioc,
QCryptoTLSCreds *tlscreds,
QIOChannel *sioc = NULL;
*info = NULL;
-result = nbd_start_negotiate(NULL, ioc, tlscreds, hostname, , true,
+result = nbd_start_negotiate(ioc, tlscreds, hostname, , true,
NULL, errp);
if (tlscreds && sioc) {
ioc = sioc;
--
2.41.0
[PULL 09/13] qemu-nbd: move srcpath into struct NbdClientOpts
From: "Denis V. Lunev"
We pass other parameters into nbd_client_thread() in this way. This patch
makes the code more consistent.
Signed-off-by: Denis V. Lunev
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
Message-ID: <20230906093210.339585-4-...@openvz.org>
Reviewed-by: Eric Blake
[eblake: Note that this also cleans up a -Wshadow issue, first
introduced in e5b815b0]
Signed-off-by: Eric Blake
---
qemu-nbd.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index acbdc0cd8fd..16c59424f13 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -73,7 +73,6 @@
#define MBR_SIZE 512
-static char *srcpath;
static SocketAddress *saddr;
static int persistent = 0;
static enum { RUNNING, TERMINATE, TERMINATED } state;
@@ -255,6 +254,7 @@ static int qemu_nbd_client_list(SocketAddress *saddr,
QCryptoTLSCreds *tls,
struct NbdClientOpts {
char *device;
+char *srcpath;
bool fork_process;
bool verbose;
};
@@ -320,7 +320,7 @@ static void *nbd_client_thread(void *arg)
if (opts->verbose && !opts->fork_process) {
fprintf(stderr, "NBD device %s is now connected to %s\n",
-opts->device, srcpath);
+opts->device, opts->srcpath);
} else {
/* Close stderr so that the qemu-nbd process exits. */
if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
@@ -590,6 +590,7 @@ int main(int argc, char **argv)
.fork_process = false,
.verbose = false,
.device = NULL,
+.srcpath = NULL,
};
#ifdef CONFIG_POSIX
@@ -1059,19 +1060,19 @@ int main(int argc, char **argv)
bdrv_init();
atexit(qemu_nbd_shutdown);
-srcpath = argv[optind];
+opts.srcpath = argv[optind];
if (imageOpts) {
-QemuOpts *opts;
+QemuOpts *o;
if (fmt) {
error_report("--image-opts and -f are mutually exclusive");
exit(EXIT_FAILURE);
}
-opts = qemu_opts_parse_noisily(_opts, srcpath, true);
-if (!opts) {
+o = qemu_opts_parse_noisily(_opts, opts.srcpath, true);
+if (!o) {
qemu_opts_reset(_opts);
exit(EXIT_FAILURE);
}
-options = qemu_opts_to_qdict(opts, NULL);
+options = qemu_opts_to_qdict(o, NULL);
qemu_opts_reset(_opts);
blk = blk_new_open(NULL, NULL, options, flags, _err);
} else {
@@ -1079,7 +1080,7 @@ int main(int argc, char **argv)
options = qdict_new();
qdict_put_str(options, "driver", fmt);
}
-blk = blk_new_open(srcpath, NULL, options, flags, _err);
+blk = blk_new_open(opts.srcpath, NULL, options, flags, _err);
}
if (!blk) {
--
2.41.0
[PULL 08/13] qemu-nbd: define struct NbdClientOpts when HAVE_NBD_DEVICE is not defined
From: "Denis V. Lunev"
This patch also drops definition of some locals in main() to avoid
useless data copy.
Signed-off-by: Denis V. Lunev
CC: Eric Blake
CC: Vladimir Sementsov-Ogievskiy
Message-ID: <20230906093210.339585-3-...@openvz.org>
Reviewed-by: Eric Blake
Signed-off-by: Eric Blake
---
qemu-nbd.c | 60 --
1 file changed, 27 insertions(+), 33 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e2480061a16..acbdc0cd8fd 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -253,6 +253,12 @@ static int qemu_nbd_client_list(SocketAddress *saddr,
QCryptoTLSCreds *tls,
}
+struct NbdClientOpts {
+char *device;
+bool fork_process;
+bool verbose;
+};
+
#if HAVE_NBD_DEVICE
static void *show_parts(void *arg)
{
@@ -271,12 +277,6 @@ static void *show_parts(void *arg)
return NULL;
}
-struct NbdClientOpts {
-char *device;
-bool fork_process;
-bool verbose;
-};
-
static void *nbd_client_thread(void *arg)
{
struct NbdClientOpts *opts = arg;
@@ -519,7 +519,6 @@ int main(int argc, char **argv)
const char *bindto = NULL;
const char *port = NULL;
char *sockpath = NULL;
-char *device = NULL;
QemuOpts *sn_opts = NULL;
const char *sn_id_or_name = NULL;
const char *sopt = "hVb:o:p:rsnc:dvk:e:f:tl:x:T:D:AB:L";
@@ -582,16 +581,16 @@ int main(int argc, char **argv)
const char *tlshostname = NULL;
bool imageOpts = false;
bool writethrough = false; /* Client will flush as needed. */
-bool verbose = false;
-bool fork_process = false;
bool list = false;
unsigned socket_activation;
const char *pid_file_name = NULL;
const char *selinux_label = NULL;
BlockExportOptions *export_opts;
-#if HAVE_NBD_DEVICE
-struct NbdClientOpts opts;
-#endif
+struct NbdClientOpts opts = {
+.fork_process = false,
+.verbose = false,
+.device = NULL,
+};
#ifdef CONFIG_POSIX
os_setup_early_signal_handling();
@@ -719,7 +718,7 @@ int main(int argc, char **argv)
disconnect = true;
break;
case 'c':
-device = optarg;
+opts.device = optarg;
break;
case 'e':
if (qemu_strtoi(optarg, NULL, 0, ) < 0 ||
@@ -750,7 +749,7 @@ int main(int argc, char **argv)
}
break;
case 'v':
-verbose = true;
+opts.verbose = true;
break;
case 'V':
version(argv[0]);
@@ -782,7 +781,7 @@ int main(int argc, char **argv)
tlsauthz = optarg;
break;
case QEMU_NBD_OPT_FORK:
-fork_process = true;
+opts.fork_process = true;
break;
case 'L':
list = true;
@@ -802,12 +801,12 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
if (export_name || export_description || dev_offset ||
-device || disconnect || fmt || sn_id_or_name || bitmaps ||
+opts.device || disconnect || fmt || sn_id_or_name || bitmaps ||
alloc_depth || seen_aio || seen_discard || seen_cache) {
error_report("List mode is incompatible with per-device settings");
exit(EXIT_FAILURE);
}
-if (fork_process) {
+if (opts.fork_process) {
error_report("List mode is incompatible with forking");
exit(EXIT_FAILURE);
}
@@ -832,7 +831,8 @@ int main(int argc, char **argv)
}
} else {
/* Using socket activation - check user didn't use -p etc. */
-const char *err_msg = socket_activation_validate_opts(device, sockpath,
+const char *err_msg = socket_activation_validate_opts(opts.device,
+ sockpath,
bindto, port,
selinux_label,
list);
@@ -850,7 +850,7 @@ int main(int argc, char **argv)
}
if (tlscredsid) {
-if (device) {
+if (opts.device) {
error_report("TLS is not supported with a host device");
exit(EXIT_FAILURE);
}
@@ -880,7 +880,7 @@ int main(int argc, char **argv)
if (selinux_label) {
#ifdef CONFIG_SELINUX
-if (sockpath == NULL && device == NULL) {
+if (sockpath == NULL && opts.device == NULL) {
error_report("--selinux-label is not permitted without --socket");
exit(EXIT_FAILURE);
}
@@ -897,7 +897,7 @@ int main(int argc, char **argv)
}
#if !HAVE_NBD_DEVICE
-if (disconnect || device) {
+if (disconnect || opts.device) {
error_report("Kernel /dev/nbdN support not available");
exit(EXIT_FAILURE);
}
@@ -919,7 +919,7 @@ int main(int argc, char **argv)
}
#endif
-
Re: [PATCH v2 4/3] qemu-iotests/197: use more generic commands for formats other than qcow2
On Fri, Sep 08, 2023 at 01:07:18AM +0300, Andrey Drobyshev via wrote:
> In the previous commit e2f938265e0 ("tests/qemu-iotests/197: add
> testcase for CoR with subclusters") we've introduced a new testcase for
> copy-on-read with subclusters. Test 197 always forces qcow2 as the top
> image, but allows backing image to be in any format. That last test
> case didn't meet these requirements, so let's fix it by using more
> generic "qemu-io -c map" command.
>
> Signed-off-by: Andrey Drobyshev
> ---
> tests/qemu-iotests/197 | 8
> tests/qemu-iotests/197.out | 18 --
> 2 files changed, 12 insertions(+), 14 deletions(-)
Tested-by: Eric Blake
>
> diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
> index f07a9da136..8ad2bdb035 100755
> --- a/tests/qemu-iotests/197
> +++ b/tests/qemu-iotests/197
> @@ -136,18 +136,18 @@ IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \
> $QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io
>
> # Allocate individual subclusters in the top image, and not the whole cluster
> -$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" "$TEST_WRAP" \
> +$QEMU_IO -f qcow2 -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K"
> "$TEST_WRAP" \
> | _filter_qemu_io
Adding the -f qcow2 makes sense (this is a test of subcluster
behavior); and the backing file remains whatever format was passed to
./check.
> +++ b/tests/qemu-iotests/197.out
> @@ -42,17 +42,15 @@ wrote 2048/2048 bytes at offset 28672
> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote 2048/2048 bytes at offset 34816
> 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -Offset Length File
> -0 0x7000 TEST_DIR/t.IMGFMT
> -0x7000 0x800 TEST_DIR/t.wrap.IMGFMT
> -0x7800 0x1000 TEST_DIR/t.IMGFMT
> -0x8800 0x800 TEST_DIR/t.wrap.IMGFMT
> -0x9000 0x7000 TEST_DIR/t.IMGFMT
> +28 KiB (0x7000) bytes not allocated at offset 0 bytes (0x0)
> +2 KiB (0x800) bytes allocated at offset 28 KiB (0x7000)
> +4 KiB (0x1000) bytes not allocated at offset 30 KiB (0x7800)
> +2 KiB (0x800) bytes allocated at offset 34 KiB (0x8800)
> +28 KiB (0x7000) bytes not allocated at offset 36 KiB (0x9000)
> read 4096/4096 bytes at offset 30720
Same information, but without the backing file details (which clears
up the problem with -nbd).
Reviewed-by: Eric Blake
Adding to my NBD queue, for a pull request soon.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
Re: [PATCH 8/8] qemu-nbd: fix formatting in main()
On Wed, Sep 06, 2023 at 11:32:10AM +0200, Denis V. Lunev wrote: > Just a formatting, no functional changes. > > Signed-off-by: Denis V. Lunev > CC: Eric Blake > CC: Vladimir Sementsov-Ogievskiy > --- > Do not really sure that this patch is mandatory, just stabs my eye. Feel free > to drop if this is too useless. > > qemu-nbd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index b9c74ce77c..8eb1d1f40b 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -581,7 +581,8 @@ int main(int argc, char **argv) > pthread_t client_thread; > const char *fmt = NULL; > Error *local_err = NULL; > -BlockdevDetectZeroesOptions detect_zeroes = > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; > +BlockdevDetectZeroesOptions detect_zeroes = > + > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; check-patch allows code up to 90 columngs although it does advise staying under 80. You fixed the long line by keeping the wrapped portion right-flushed to 80 columns; I think more typical tree-wide is to just indent by four spaces (at least, that's what emacs suggests I do). But me changing what you wrote would a complete rewrite, so I'm reluctant to include it in my upcoming pull request, although I'm not ruling out a later cleanup (perhaps if it touches more than one stylistic thing at once). I'm queuing 1-7 through my NBD tree, and running another round of iotests before sending the pull request this week. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH v2 3/3] tests/qemu-iotests/197: add testcase for CoR with subclusters
On 9/6/23 12:43, Denis V. Lunev wrote: > On 7/11/23 19:25, Andrey Drobyshev wrote: >> Add testcase which checks that allocations during copy-on-read are >> performed on the subcluster basis when subclusters are enabled in target >> image. >> >> This testcase also triggers the following assert with previous commit >> not being applied, so we check that as well: >> >> qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion >> `skip_bytes < pnum' failed. >> >> Reviewed-by: Eric Blake >> Reviewed-by: Denis V. Lunev >> Signed-off-by: Andrey Drobyshev >> --- >> tests/qemu-iotests/197 | 29 + >> tests/qemu-iotests/197.out | 24 >> 2 files changed, 53 insertions(+) >> >> diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197 >> index a2547bc280..f07a9da136 100755 >> --- a/tests/qemu-iotests/197 >> +++ b/tests/qemu-iotests/197 >> @@ -122,6 +122,35 @@ $QEMU_IO -f qcow2 -C -c 'read 0 1024' >> "$TEST_WRAP" | _filter_qemu_io >> $QEMU_IO -f qcow2 -c map "$TEST_WRAP" >> _check_test_img >> +echo >> +echo '=== Copy-on-read with subclusters ===' >> +echo >> + >> +# Create base and top images 64K (1 cluster) each. Make subclusters >> enabled >> +# for the top image >> +_make_test_img 64K >> +IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \ >> + _make_test_img --no-opts -o extended_l2=true -F "$IMGFMT" -b >> "$TEST_IMG" \ >> + 64K | _filter_img_create >> + >> +$QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io >> + >> +# Allocate individual subclusters in the top image, and not the whole >> cluster >> +$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" >> "$TEST_WRAP" \ >> + | _filter_qemu_io >> + >> +# Only 2 subclusters should be allocated in the top image at this point >> +$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map >> + >> +# Actual copy-on-read operation >> +$QEMU_IO -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io >> + >> +# And here we should have 4 subclusters allocated right in the middle >> of the >> +# top image. Make sure the whole cluster remains unallocated >> +$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map >> + >> +_check_test_img >> + >> # success, all done >> echo '*** done' >> status=0 >> diff --git a/tests/qemu-iotests/197.out b/tests/qemu-iotests/197.out >> index ad414c3b0e..8f34a30afe 100644 >> --- a/tests/qemu-iotests/197.out >> +++ b/tests/qemu-iotests/197.out >> @@ -31,4 +31,28 @@ read 1024/1024 bytes at offset 0 >> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) >> No errors were found on the image. >> + >> +=== Copy-on-read with subclusters === >> + >> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 >> +Formatting 'TEST_DIR/t.wrap.IMGFMT', fmt=IMGFMT size=65536 >> backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT >> +wrote 65536/65536 bytes at offset 0 >> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +wrote 2048/2048 bytes at offset 28672 >> +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +wrote 2048/2048 bytes at offset 34816 >> +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +Offset Length File >> +0 0x7000 TEST_DIR/t.IMGFMT >> +0x7000 0x800 TEST_DIR/t.wrap.IMGFMT >> +0x7800 0x1000 TEST_DIR/t.IMGFMT >> +0x8800 0x800 TEST_DIR/t.wrap.IMGFMT >> +0x9000 0x7000 TEST_DIR/t.IMGFMT >> +read 4096/4096 bytes at offset 30720 >> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> +Offset Length File >> +0 0x7000 TEST_DIR/t.IMGFMT >> +0x7000 0x2000 TEST_DIR/t.wrap.IMGFMT >> +0x9000 0x7000 TEST_DIR/t.IMGFMT >> +No errors were found on the image. >> *** done > It is revealed that this patch seems to break unit tests if run for NBD > format > > iris ~/src/qemu/build/tests/qemu-iotests $ ./check -nbd 197 > QEMU -- > "/home/den/src/qemu/build/tests/qemu-iotests/../../qemu-system-x86_64" > -nodefaults -display none -accel qtest > QEMU_IMG -- > "/home/den/src/qemu/build/tests/qemu-iotests/../../qemu-img" > QEMU_IO -- > "/home/den/src/qemu/build/tests/qemu-iotests/../../qemu-io" --cache > writeback --aio threads -f raw > QEMU_NBD -- > "/home/den/src/qemu/build/tests/qemu-iotests/../../qemu-nbd" > IMGFMT -- raw > IMGPROTO -- nbd > PLATFORM -- Linux/x86_64 iris 6.2.0-31-generic > TEST_DIR -- /home/den/src/qemu/build/tests/qemu-iotests/scratch > SOCK_DIR -- /tmp/tmpzw5ky8d3 > GDB_OPTIONS -- > VALGRIND_QEMU -- > PRINT_QEMU_OUTPUT -- > > 197 fail [11:41:26] [11:41:31] 5.7s (last: 3.8s) output > mismatch (see > /home/den/src/qemu/build/tests/qemu-iotests/scratch/raw-nbd-197/197.out.bad) > --- /home/den/src/qemu/tests/qemu-iotests/197.out > +++ >
Re: [PATCH v2 0/3] block: align CoR requests to subclusters
On 9/7/23 22:11, Michael Tokarev wrote: 11.07.2023 20:25, Andrey Drobyshev via wrote: v1 --> v2: * Fixed line indentation; * Fixed wording in a comment; * Added R-b. v1: https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00606.html Andrey Drobyshev (3): block: add subcluster_size field to BlockDriverInfo block/io: align requests to subcluster_size tests/qemu-iotests/197: add testcase for CoR with subclusters block.c | 7 + block/io.c | 50 ++-- block/mirror.c | 8 +++--- block/qcow2.c | 1 + include/block/block-common.h | 5 include/block/block-io.h | 8 +++--- tests/qemu-iotests/197 | 29 + tests/qemu-iotests/197.out | 24 + 8 files changed, 99 insertions(+), 33 deletions(-) So, given the size of patch series and amount of time the series were sitting there.. I'm hesitating to apply it to -stable. The whole issue, while real, smells like somewhat unusual case. Any comments on this? Thanks, /mjt The issue was observed by us in the reality on not yet released product, but the series requires correct, which has been sent as 4/3 patch today by Andrey. Unit tests are broken at the moment. Thanks, Den
[PATCH v2 4/3] qemu-iotests/197: use more generic commands for formats other than qcow2
In the previous commit e2f938265e0 ("tests/qemu-iotests/197: add
testcase for CoR with subclusters") we've introduced a new testcase for
copy-on-read with subclusters. Test 197 always forces qcow2 as the top
image, but allows backing image to be in any format. That last test
case didn't meet these requirements, so let's fix it by using more
generic "qemu-io -c map" command.
Signed-off-by: Andrey Drobyshev
---
tests/qemu-iotests/197 | 8
tests/qemu-iotests/197.out | 18 --
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index f07a9da136..8ad2bdb035 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -136,18 +136,18 @@ IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \
$QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io
# Allocate individual subclusters in the top image, and not the whole cluster
-$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" "$TEST_WRAP" \
+$QEMU_IO -f qcow2 -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K"
"$TEST_WRAP" \
| _filter_qemu_io
# Only 2 subclusters should be allocated in the top image at this point
-$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+$QEMU_IO -f qcow2 -c map "$TEST_WRAP"
# Actual copy-on-read operation
-$QEMU_IO -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
+$QEMU_IO -f qcow2 -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
# And here we should have 4 subclusters allocated right in the middle of the
# top image. Make sure the whole cluster remains unallocated
-$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+$QEMU_IO -f qcow2 -c map "$TEST_WRAP"
_check_test_img
diff --git a/tests/qemu-iotests/197.out b/tests/qemu-iotests/197.out
index 8f34a30afe..86c57b51d3 100644
--- a/tests/qemu-iotests/197.out
+++ b/tests/qemu-iotests/197.out
@@ -42,17 +42,15 @@ wrote 2048/2048 bytes at offset 28672
2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 2048/2048 bytes at offset 34816
2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0 0x7000 TEST_DIR/t.IMGFMT
-0x7000 0x800 TEST_DIR/t.wrap.IMGFMT
-0x7800 0x1000 TEST_DIR/t.IMGFMT
-0x8800 0x800 TEST_DIR/t.wrap.IMGFMT
-0x9000 0x7000 TEST_DIR/t.IMGFMT
+28 KiB (0x7000) bytes not allocated at offset 0 bytes (0x0)
+2 KiB (0x800) bytes allocated at offset 28 KiB (0x7000)
+4 KiB (0x1000) bytes not allocated at offset 30 KiB (0x7800)
+2 KiB (0x800) bytes allocated at offset 34 KiB (0x8800)
+28 KiB (0x7000) bytes not allocated at offset 36 KiB (0x9000)
read 4096/4096 bytes at offset 30720
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0 0x7000 TEST_DIR/t.IMGFMT
-0x7000 0x2000 TEST_DIR/t.wrap.IMGFMT
-0x9000 0x7000 TEST_DIR/t.IMGFMT
+28 KiB (0x7000) bytes not allocated at offset 0 bytes (0x0)
+8 KiB (0x2000) bytes allocated at offset 28 KiB (0x7000)
+28 KiB (0x7000) bytes not allocated at offset 36 KiB (0x9000)
No errors were found on the image.
*** done
--
2.39.3
Re: [PATCH 7/8] qemu-nbd: document -v behavior in respect to --fork in man
On 9/8/23 00:01, Eric Blake wrote: On Wed, Sep 06, 2023 at 11:32:09AM +0200, Denis V. Lunev wrote: Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy --- docs/tools/qemu-nbd.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst index faf6349ea5..5c48ee7345 100644 --- a/docs/tools/qemu-nbd.rst +++ b/docs/tools/qemu-nbd.rst @@ -197,7 +197,9 @@ driver options if :option:`--image-opts` is specified. .. option:: -v, --verbose - Display extra debugging information. + Display extra debugging information. This option also keeps opened original + *STDERR* stream if ``qemu-nbd`` process is daemonized due to other options + like :option:`--fork` or :option:`-c`. As a native speaker, I find the following a bit easier to parse: This option also keeps the original *STDERR* stream open if ... I can make that touchup as part of queuing the series. Reviewed-by: Eric Blake That would be great, thanks!
Re: [PATCH 7/8] qemu-nbd: document -v behavior in respect to --fork in man
On Wed, Sep 06, 2023 at 11:32:09AM +0200, Denis V. Lunev wrote: > Signed-off-by: Denis V. Lunev > CC: Eric Blake > CC: Vladimir Sementsov-Ogievskiy > --- > docs/tools/qemu-nbd.rst | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst > index faf6349ea5..5c48ee7345 100644 > --- a/docs/tools/qemu-nbd.rst > +++ b/docs/tools/qemu-nbd.rst > @@ -197,7 +197,9 @@ driver options if :option:`--image-opts` is specified. > > .. option:: -v, --verbose > > - Display extra debugging information. > + Display extra debugging information. This option also keeps opened original > + *STDERR* stream if ``qemu-nbd`` process is daemonized due to other options > + like :option:`--fork` or :option:`-c`. As a native speaker, I find the following a bit easier to parse: This option also keeps the original *STDERR* stream open if ... I can make that touchup as part of queuing the series. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH 6/8] qemu-nbd: Restore "qemu-nbd -v --fork" output
On Wed, Sep 06, 2023 at 11:32:08AM +0200, Denis V. Lunev wrote:
> Closing stderr earlier is good for daemonized qemu-nbd under ssh
> earlier, but breaks the case where -v is being used to track what is
> happening in the server, as in iotest 233.
>
> When we know we are verbose, we should preserve original stderr and
> restore it once the setup stage is done. This commit restores the
> original behavior with -v option. In this case original output
> inside the test is kept intact.
>
> Reported-by: Kevin Wolf
> Signed-off-by: Denis V. Lunev
> CC: Eric Blake
> CC: Vladimir Sementsov-Ogievskiy
> CC: Hanna Reitz
> CC: Mike Maslenkin
> Fixes: 5c56dd27a2 ("qemu-nbd: fix regression with qemu-nbd --fork run over
> ssh")
> ---
> qemu-nbd.c | 24
> 1 file changed, 20 insertions(+), 4 deletions(-)
Tested-by: Eric Blake
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
Re: [PATCH 5/8] qemu-nbd: invent nbd_client_release_pipe() helper
On Wed, Sep 06, 2023 at 11:32:07AM +0200, Denis V. Lunev wrote: > Move the code from main() and nbd_client_thread() into the specific > helper. This code is going to be grown. > > Signed-off-by: Denis V. Lunev > CC: Eric Blake > CC: Vladimir Sementsov-Ogievskiy > --- > qemu-nbd.c | 23 --- > 1 file changed, 12 insertions(+), 11 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
[PATCH 1/1] block: improve alignment detection and fix 271 test
Unfortunately 271 IO test is broken if started in non-cached mode.
Commits
commit a6b257a08e3d72219f03e461a52152672fec0612
Author: Nir Soffer
Date: Tue Aug 13 21:21:03 2019 +0300
file-posix: Handle undetectable alignment
and
commit 9c60a5d1978e6dcf85c0e01b50e6f7f54ca09104
Author: Kevin Wolf
Date: Thu Jul 16 16:26:00 2020 +0200
block: Require aligned image size to avoid assertion failure
have interesting side effect if used togather.
If the image size is not multiple of 4k and that image falls under
original constraints of Nil's patch, the image can not be opened
due to the check in the bdrv_check_perm().
The patch tries to satisfy the requirements of bdrv_check_perm()
inside raw_probe_alignment(). This is at my opinion better that just
disallowing to run that test in non-cached mode. The operation is legal
by itself.
Signed-off-by: Denis V. Lunev
CC: Nir Soffer
CC: Kevin Wolf
CC: Hanna Reitz
CC: Alberto Garcia
---
block/file-posix.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index b16e9c21a1..988cfdc76c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -447,8 +447,21 @@ static void raw_probe_alignment(BlockDriverState *bs, int
fd, Error **errp)
for (i = 0; i < ARRAY_SIZE(alignments); i++) {
align = alignments[i];
if (raw_is_io_aligned(fd, buf, align)) {
-/* Fallback to safe value. */
-bs->bl.request_alignment = (align != 1) ? align : max_align;
+if (align != 1) {
+bs->bl.request_alignment = align;
+break;
+}
+/*
+ * Fallback to safe value. max_align is perfect, but the size
of the device must be multiple of
+ * the virtual length of the device. In the other case we will
get a error in
+ * bdrv_node_refresh_perm().
+ */
+for (align = max_align; align > 1; align /= 2) {
+if ((bs->total_sectors * BDRV_SECTOR_SIZE) % align == 0) {
+break;
+}
+}
+bs->bl.request_alignment = align;
break;
}
}
--
2.34.1
Re: [PATCH 4/8] qemu-nbd: put saddr into into struct NbdClientOpts
On Wed, Sep 06, 2023 at 11:32:06AM +0200, Denis V. Lunev wrote: > We pass other parameters into nbd_client_thread() in this way. This patch > makes the code more consistent. > > Signed-off-by: Denis V. Lunev > CC: Eric Blake > CC: Vladimir Sementsov-Ogievskiy > --- > qemu-nbd.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH 3/8] qemu-nbd: move srcpath into struct NbdClientOpts
On Wed, Sep 06, 2023 at 11:32:05AM +0200, Denis V. Lunev wrote:
> We pass other parameters into nbd_client_thread() in this way. This patch
> makes the code more consistent.
>
> Signed-off-by: Denis V. Lunev
> CC: Eric Blake
> CC: Vladimir Sementsov-Ogievskiy
> ---
> qemu-nbd.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> @@ -1059,19 +1060,19 @@ int main(int argc, char **argv)
> bdrv_init();
> atexit(qemu_nbd_shutdown);
>
> -srcpath = argv[optind];
> +opts.srcpath = argv[optind];
> if (imageOpts) {
> -QemuOpts *opts;
> +QemuOpts *o;
> if (fmt) {
> error_report("--image-opts and -f are mutually exclusive");
> exit(EXIT_FAILURE);
> }
> -opts = qemu_opts_parse_noisily(_opts, srcpath, true);
> -if (!opts) {
> +o = qemu_opts_parse_noisily(_opts, opts.srcpath, true);
> +if (!o) {
Hmm - this would have been flagged by -Wshadow, and there are other
series working to clean up tree-wide issues that shadowing can cause.
Looking again, the shadowing was previously introduced before this
series, but only when HAVE_NBD_DEVICE was defined; then patch 2/8 made
the shadowing unconditional. Reworking the series to clean up the
shadowing earlier in 2/8 is just churn, so I don't mind that it took
us to this point to notice it; however, I'm inclined to add a note to
the commit message that it is a (happy) side-effect.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
Re: [PATCH 2/8] qemu-nbd: define struct NbdClientOpts when HAVE_NBD_DEVICE is not defined
On Wed, Sep 06, 2023 at 11:32:04AM +0200, Denis V. Lunev wrote:
> This patch also drops definition of some locals in main() to avoid
> useless data copy.
>
> Signed-off-by: Denis V. Lunev
> CC: Eric Blake
> CC: Vladimir Sementsov-Ogievskiy
> ---
> qemu-nbd.c | 60 --
> 1 file changed, 27 insertions(+), 33 deletions(-)
> @@ -519,7 +519,6 @@ int main(int argc, char **argv)
> const char *bindto = NULL;
> const char *port = NULL;
> char *sockpath = NULL;
> -char *device = NULL;
> QemuOpts *sn_opts = NULL;
> const char *sn_id_or_name = NULL;
> const char *sopt = "hVb:o:p:rsnc:dvk:e:f:tl:x:T:D:AB:L";
> @@ -582,16 +581,16 @@ int main(int argc, char **argv)
> const char *tlshostname = NULL;
> bool imageOpts = false;
> bool writethrough = false; /* Client will flush as needed. */
> -bool verbose = false;
> -bool fork_process = false;
> bool list = false;
> unsigned socket_activation;
> const char *pid_file_name = NULL;
> const char *selinux_label = NULL;
> BlockExportOptions *export_opts;
> -#if HAVE_NBD_DEVICE
> -struct NbdClientOpts opts;
> -#endif
> +struct NbdClientOpts opts = {
> +.fork_process = false,
> +.verbose = false,
> +.device = NULL,
> +};
Could also do 'struct NbdClietnOpts opts = {};' since you happen to be
zero-initializing, but this may not remain the case if more fields get
added to the struct, so I'm fine leaving it as written.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
[PATCH v3 0/2] qemu-img: map: implement support for compressed clusters
v2 --> v3: * Make "compressed" field mandatory, not optional; * Adjust field description in qapi/block-core.json; * Squash patch 3 into patch 2 so that failing tests don't break bisect; * Update even more tests' outputs now that the field is mandatory. v2: https://lists.nongnu.org/archive/html/qemu-block/2023-07/msg00106.html Andrey Drobyshev (2): block: add BDRV_BLOCK_COMPRESSED flag for bdrv_block_status() qemu-img: map: report compressed data blocks block/qcow.c | 5 +- block/qcow2.c | 3 + block/vmdk.c | 2 + include/block/block-common.h | 3 + qapi/block-core.json | 7 +- qemu-img.c| 8 +- tests/qemu-iotests/122.out| 84 +- tests/qemu-iotests/146.out| 780 +- tests/qemu-iotests/154.out| 194 ++--- tests/qemu-iotests/179.out| 178 ++-- tests/qemu-iotests/209.out| 4 +- tests/qemu-iotests/221.out| 16 +- tests/qemu-iotests/223.out| 60 +- tests/qemu-iotests/241.out| 10 +- tests/qemu-iotests/244.out| 24 +- tests/qemu-iotests/252.out| 10 +- tests/qemu-iotests/253.out| 20 +- tests/qemu-iotests/274.out| 48 +- .../tests/nbd-qemu-allocation.out | 16 +- tests/qemu-iotests/tests/qemu-img-bitmaps.out | 24 +- 20 files changed, 757 insertions(+), 739 deletions(-) -- 2.39.3
[PATCH v3 1/2] block: add BDRV_BLOCK_COMPRESSED flag for bdrv_block_status()
Functions qcow2_get_host_offset(), get_cluster_offset(),
vmdk_co_block_status() explicitly report compressed cluster types when data
is compressed. However, this information is never passed further. Let's
make use of it by adding new BDRV_BLOCK_COMPRESSED flag for
bdrv_block_status(), so that caller may know that the data range is
compressed. In particular, we're going to use this flag to tweak
"qemu-img map" output.
This new flag is only being utilized by qcow, qcow2 and vmdk formats, as only
those support compression.
Reviewed-by: Denis V. Lunev
Reviewed-by: Hanna Czenczek
Signed-off-by: Andrey Drobyshev
---
block/qcow.c | 5 -
block/qcow2.c| 3 +++
block/vmdk.c | 2 ++
include/block/block-common.h | 3 +++
4 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/block/qcow.c b/block/qcow.c
index 577bd70324..d56d24ab6d 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -549,7 +549,10 @@ qcow_co_block_status(BlockDriverState *bs, bool want_zero,
if (!cluster_offset) {
return 0;
}
-if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypto) {
+if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+return BDRV_BLOCK_DATA | BDRV_BLOCK_COMPRESSED;
+}
+if (s->crypto) {
return BDRV_BLOCK_DATA;
}
*map = cluster_offset | index_in_cluster;
diff --git a/block/qcow2.c b/block/qcow2.c
index b48cd9ce63..b81dc5066b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2162,6 +2162,9 @@ qcow2_co_block_status(BlockDriverState *bs, bool
want_zero, int64_t offset,
{
status |= BDRV_BLOCK_RECURSE;
}
+if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+status |= BDRV_BLOCK_COMPRESSED;
+}
return status;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index 70066c2b01..56b3d5151d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1770,6 +1770,8 @@ vmdk_co_block_status(BlockDriverState *bs, bool want_zero,
if (extent->flat) {
ret |= BDRV_BLOCK_RECURSE;
}
+} else {
+ret |= BDRV_BLOCK_COMPRESSED;
}
*file = extent->file->bs;
break;
diff --git a/include/block/block-common.h b/include/block/block-common.h
index df5ffc8d09..5b5ae07c93 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -287,6 +287,8 @@ typedef enum {
* layer rather than any backing, set by block layer
* BDRV_BLOCK_EOF: the returned pnum covers through end of file for this
* layer, set by block layer
+ * BDRV_BLOCK_COMPRESSED: the underlying data is compressed; only valid for
+ *the formats supporting compression: qcow, qcow2
*
* Internal flags:
* BDRV_BLOCK_RAW: for use by passthrough drivers, such as raw, to request
@@ -322,6 +324,7 @@ typedef enum {
#define BDRV_BLOCK_ALLOCATED0x10
#define BDRV_BLOCK_EOF 0x20
#define BDRV_BLOCK_RECURSE 0x40
+#define BDRV_BLOCK_COMPRESSED 0x80
typedef QTAILQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
--
2.39.3
Re: [PATCH 1/8] qemu-nbd: improve error message for dup2 error
On Wed, Sep 06, 2023 at 11:32:03AM +0200, Denis V. Lunev wrote:
> This error is happened when we are not able to close the pipe to the
s/is happened when/happens if/
> parent (to trace errors in the child process) and assign stderr to
> /dev/null as required by the daemonizing convention.
>
> Signed-off-by: Denis V. Lunev
> Suggested-by: Eric Blake
> CC: Eric Blake
> CC: Vladimir Sementsov-Ogievskiy
> ---
> qemu-nbd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index aaccaa3318..4575e4291e 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -324,7 +324,7 @@ static void *nbd_client_thread(void *arg)
> } else {
> /* Close stderr so that the qemu-nbd process exits. */
> if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
> -error_report("Could not set stderr to /dev/null: %s",
> +error_report("Could not release pipe to parent: %s",
> strerror(errno));
> exit(EXIT_FAILURE);
> }
> @@ -1181,7 +1181,7 @@ int main(int argc, char **argv)
>
> if (fork_process) {
> if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) {
> -error_report("Could not set stderr to /dev/null: %s",
> +error_report("Could not release pipe to parent: %s",
> strerror(errno));
> exit(EXIT_FAILURE);
> }
> --
> 2.34.1
>
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
Re: [PATCH v2 0/3] block: align CoR requests to subclusters
11.07.2023 20:25, Andrey Drobyshev via wrote: v1 --> v2: * Fixed line indentation; * Fixed wording in a comment; * Added R-b. v1: https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00606.html Andrey Drobyshev (3): block: add subcluster_size field to BlockDriverInfo block/io: align requests to subcluster_size tests/qemu-iotests/197: add testcase for CoR with subclusters block.c | 7 + block/io.c | 50 ++-- block/mirror.c | 8 +++--- block/qcow2.c| 1 + include/block/block-common.h | 5 include/block/block-io.h | 8 +++--- tests/qemu-iotests/197 | 29 + tests/qemu-iotests/197.out | 24 + 8 files changed, 99 insertions(+), 33 deletions(-) So, given the size of patch series and amount of time the series were sitting there.. I'm hesitating to apply it to -stable. The whole issue, while real, smells like somewhat unusual case. Any comments on this? Thanks, /mjt
Re: [PATCH 0/2] virtio: Drop out of coroutine context in virtio_load()
On Tue, Sep 05, 2023 at 04:50:00PM +0200, Kevin Wolf wrote: > This fixes a recently introduced assertion failure that was reported to > happen when migrating virtio-net with a failover. The latent bug that > we're executing code in coroutine context that was never supposed to run > there has existed for a long time. However, the new assertion that > callers of bdrv_graph_rdlock_main_loop() don't run in coroutine context > makes it very visible because it's now always a crash. > > Kevin Wolf (2): > vmstate: Mark VMStateInfo.get/put() coroutine_mixed_fn > virtio: Drop out of coroutine context in virtio_load() > > include/migration/vmstate.h | 8 --- > hw/virtio/virtio.c | 45 - > 2 files changed, 45 insertions(+), 8 deletions(-) This looks like a bandaid for a specific instance of this problem rather than a solution that takes care of the root cause. Is it possible to make VMStateInfo.get/put() consistenty coroutine_fn? Stefan signature.asc Description: PGP signature
Re: [PATCH 2/2] virtio: Drop out of coroutine context in virtio_load()
On Tue, Sep 05, 2023 at 04:50:02PM +0200, Kevin Wolf wrote: > virtio_load() as a whole should run in coroutine context because it > reads from the migration stream and we don't want this to block. Is that "should" a "must" or a "can"? If it's a "must" then virtio_load() needs assert(qemu_in_coroutine()). But the previous patch mentioned that loadvm for snapshots calls it outside coroutine context. So maybe it's a "can"? > > However, it calls virtio_set_features_nocheck() and devices don't > expect their .set_features callback to run in a coroutine and therefore > call functions that may not be called in coroutine context. To fix this, > drop out of coroutine context for calling virtio_set_features_nocheck(). > > Without this fix, the following crash was reported: > > #0 __pthread_kill_implementation (threadid=, > signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 > #1 0x7efc738c05d3 in __pthread_kill_internal (signo=6, > threadid=) at pthread_kill.c:78 > #2 0x7efc73873d26 in __GI_raise (sig=sig@entry=6) at > ../sysdeps/posix/raise.c:26 > #3 0x7efc738477f3 in __GI_abort () at abort.c:79 > #4 0x7efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", > assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()", > file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", > line=line@entry=275, function=function@entry=0x560aebfcd34d "void > bdrv_graph_rdlock_main_loop(void)") at assert.c:92 > #5 0x7efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf > "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275, > function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at > assert.c:101 > #6 0x560aebcd8dd6 in bdrv_register_buf () > #7 0x560aeb97ed97 in ram_block_added.llvm () > #8 0x560aebb8303f in ram_block_add.llvm () > #9 0x560aebb834fa in qemu_ram_alloc_internal.llvm () > #10 0x560aebb2ac98 in vfio_region_mmap () > #11 0x560aebb3ea0f in vfio_bars_register () > #12 0x560aebb3c628 in vfio_realize () > #13 0x560aeb90f0c2 in pci_qdev_realize () > #14 0x560aebc40305 in device_set_realized () > #15 0x560aebc48e07 in property_set_bool.llvm () > #16 0x560aebc46582 in object_property_set () > #17 0x560aebc4cd58 in object_property_set_qobject () > #18 0x560aebc46ba7 in object_property_set_bool () > #19 0x560aeb98b3ca in qdev_device_add_from_qdict () > #20 0x560aebb1fbaf in virtio_net_set_features () > #21 0x560aebb46b51 in virtio_set_features_nocheck () > #22 0x560aebb47107 in virtio_load () > #23 0x560aeb9ae7ce in vmstate_load_state () > #24 0x560aeb9d2ee9 in qemu_loadvm_state_main () > #25 0x560aeb9d45e1 in qemu_loadvm_state () > #26 0x560aeb9bc32c in process_incoming_migration_co.llvm () > #27 0x560aebeace56 in coroutine_trampoline.llvm () > > Cc: qemu-sta...@nongnu.org > Buglink: https://issues.redhat.com/browse/RHEL-832 > Signed-off-by: Kevin Wolf > --- > hw/virtio/virtio.c | 45 - > 1 file changed, 40 insertions(+), 5 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[PULL 4/5] hw/ufs: Support for UFS logical unit
From: Jeuk Kim
This commit adds support for ufs logical unit.
The LU handles processing for the SCSI command,
unit descriptor query request.
This commit enables the UFS device to process
IO requests.
Signed-off-by: Jeuk Kim
Reviewed-by: Stefan Hajnoczi
Message-id:
beacc504376ab6a14b1a3830bb3c69382cf6aebc.1693980783.git.jeuk20@gmail.com
Signed-off-by: Stefan Hajnoczi
---
hw/ufs/ufs.h | 43 ++
include/scsi/constants.h |1 +
hw/ufs/lu.c | 1445 ++
hw/ufs/ufs.c | 252 ++-
hw/ufs/meson.build |2 +-
hw/ufs/trace-events | 25 +
6 files changed, 1761 insertions(+), 7 deletions(-)
create mode 100644 hw/ufs/lu.c
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index 3d1b2cff4e..f244228617 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -18,6 +18,18 @@
#define UFS_MAX_LUS 32
#define UFS_BLOCK_SIZE 4096
+typedef struct UfsBusClass {
+BusClass parent_class;
+bool (*parent_check_address)(BusState *bus, DeviceState *dev, Error
**errp);
+} UfsBusClass;
+
+typedef struct UfsBus {
+SCSIBus parent_bus;
+} UfsBus;
+
+#define TYPE_UFS_BUS "ufs-bus"
+DECLARE_OBJ_CHECKERS(UfsBus, UfsBusClass, UFS_BUS, TYPE_UFS_BUS)
+
typedef enum UfsRequestState {
UFS_REQUEST_IDLE = 0,
UFS_REQUEST_READY = 1,
@@ -29,6 +41,7 @@ typedef enum UfsRequestState {
typedef enum UfsReqResult {
UFS_REQUEST_SUCCESS = 0,
UFS_REQUEST_FAIL = 1,
+UFS_REQUEST_NO_COMPLETE = 2,
} UfsReqResult;
typedef struct UfsRequest {
@@ -44,6 +57,17 @@ typedef struct UfsRequest {
QEMUSGList *sg;
} UfsRequest;
+typedef struct UfsLu {
+SCSIDevice qdev;
+uint8_t lun;
+UnitDescriptor unit_desc;
+} UfsLu;
+
+typedef struct UfsWLu {
+SCSIDevice qdev;
+uint8_t lun;
+} UfsWLu;
+
typedef struct UfsParams {
char *serial;
uint8_t nutrs; /* Number of UTP Transfer Request Slots */
@@ -52,12 +76,18 @@ typedef struct UfsParams {
typedef struct UfsHc {
PCIDevice parent_obj;
+UfsBus bus;
MemoryRegion iomem;
UfsReg reg;
UfsParams params;
uint32_t reg_size;
UfsRequest *req_list;
+UfsLu *lus[UFS_MAX_LUS];
+UfsWLu *report_wlu;
+UfsWLu *dev_wlu;
+UfsWLu *boot_wlu;
+UfsWLu *rpmb_wlu;
DeviceDescriptor device_desc;
GeometryDescriptor geometry_desc;
Attributes attributes;
@@ -71,6 +101,12 @@ typedef struct UfsHc {
#define TYPE_UFS "ufs"
#define UFS(obj) OBJECT_CHECK(UfsHc, (obj), TYPE_UFS)
+#define TYPE_UFS_LU "ufs-lu"
+#define UFSLU(obj) OBJECT_CHECK(UfsLu, (obj), TYPE_UFS_LU)
+
+#define TYPE_UFS_WLU "ufs-wlu"
+#define UFSWLU(obj) OBJECT_CHECK(UfsWLu, (obj), TYPE_UFS_WLU)
+
typedef enum UfsQueryFlagPerm {
UFS_QUERY_FLAG_NONE = 0x0,
UFS_QUERY_FLAG_READ = 0x1,
@@ -85,4 +121,11 @@ typedef enum UfsQueryAttrPerm {
UFS_QUERY_ATTR_WRITE = 0x2,
} UfsQueryAttrPerm;
+static inline bool is_wlun(uint8_t lun)
+{
+return (lun == UFS_UPIU_REPORT_LUNS_WLUN ||
+lun == UFS_UPIU_UFS_DEVICE_WLUN || lun == UFS_UPIU_BOOT_WLUN ||
+lun == UFS_UPIU_RPMB_WLUN);
+}
+
#endif /* HW_UFS_UFS_H */
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 6a8bad556a..9b98451912 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -231,6 +231,7 @@
#define MODE_PAGE_FLEXIBLE_DISK_GEOMETRY 0x05
#define MODE_PAGE_CACHING 0x08
#define MODE_PAGE_AUDIO_CTL 0x0e
+#define MODE_PAGE_CONTROL 0x0a
#define MODE_PAGE_POWER 0x1a
#define MODE_PAGE_FAULT_FAIL 0x1c
#define MODE_PAGE_TO_PROTECT 0x1d
diff --git a/hw/ufs/lu.c b/hw/ufs/lu.c
new file mode 100644
index 00..e1c46bddb1
--- /dev/null
+++ b/hw/ufs/lu.c
@@ -0,0 +1,1445 @@
+/*
+ * QEMU UFS Logical Unit
+ *
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd. All rights reserved.
+ *
+ * Written by Jeuk Kim
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "qemu/memalign.h"
+#include "hw/scsi/scsi.h"
+#include "scsi/constants.h"
+#include "sysemu/block-backend.h"
+#include "qemu/cutils.h"
+#include "trace.h"
+#include "ufs.h"
+
+/*
+ * The code below handling SCSI commands is copied from hw/scsi/scsi-disk.c,
+ * with minor adjustments to make it work for UFS.
+ */
+
+#define SCSI_DMA_BUF_SIZE (128 * KiB)
+#define SCSI_MAX_INQUIRY_LEN 256
+#define SCSI_INQUIRY_DATA_SIZE 36
+#define SCSI_MAX_MODE_LEN 256
+
+typedef struct UfsSCSIReq {
+SCSIRequest req;
+/* Both sector and sector_count are in terms of BDRV_SECTOR_SIZE bytes. */
+uint64_t sector;
+uint32_t sector_count;
+uint32_t buflen;
+bool started;
+bool need_fua_emulation;
+struct iovec iov;
+QEMUIOVector qiov;
+BlockAcctCookie acct;
+} UfsSCSIReq;
+
+static void ufs_scsi_free_request(SCSIRequest *req)
+{
+UfsSCSIReq *r =
Re: [PULL for-6.2 0/7] Ide patches
07.09.2023 19:54, John Snow wrote: .. > > > Niklas Cassel (7): > hw/ide/core: set ERR_STAT in unsupported command completion > hw/ide/ahci: write D2H FIS when processing NCQ command > hw/ide/ahci: simplify and document PxCI handling > hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared > hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set > hw/ide/ahci: fix ahci_write_fis_sdb() > hw/ide/ahci: fix broken SError handling Is anything here stable-worthy? Yes, assuming it doesn't break anything. Hmm. I was thinking maybe one or two of the above. Are you suggesting the *whole* lot? I can't give IDE the testing it deserves anymore, but I trust Niklas. I don't have good test suites for *inside* linux/windows guests so I am admittedly relying on qtests and for people to bark if something regressed. I'd say to tentatively add them to your list and if we find regressions during this window, we can exclude them from a stable release. Yeah, sure, that's okay. Thank you! /mjt
[PULL 2/5] hw/ufs: Initial commit for emulated Universal-Flash-Storage
From: Jeuk Kim
Universal Flash Storage (UFS) is a high-performance mass storage device
with a serial interface. It is primarily used as a high-performance
data storage device for embedded applications.
This commit contains code for UFS device to be recognized
as a UFS PCI device.
Patches to handle UFS logical unit and Transfer Request will follow.
Signed-off-by: Jeuk Kim
Reviewed-by: Stefan Hajnoczi
Message-id:
10232660d462ee5cd10cf673f1a9a1205fc8276c.1693980783.git.jeuk20@gmail.com
Signed-off-by: Stefan Hajnoczi
---
MAINTAINERS |6 +
docs/specs/pci-ids.rst |2 +
meson.build |1 +
hw/ufs/trace.h |1 +
hw/ufs/ufs.h | 42 ++
include/block/ufs.h | 1090 ++
include/hw/pci/pci.h |1 +
include/hw/pci/pci_ids.h |1 +
hw/ufs/ufs.c | 278 ++
hw/Kconfig |1 +
hw/meson.build |1 +
hw/ufs/Kconfig |4 +
hw/ufs/meson.build |1 +
hw/ufs/trace-events | 32 ++
14 files changed, 1461 insertions(+)
create mode 100644 hw/ufs/trace.h
create mode 100644 hw/ufs/ufs.h
create mode 100644 include/block/ufs.h
create mode 100644 hw/ufs/ufs.c
create mode 100644 hw/ufs/Kconfig
create mode 100644 hw/ufs/meson.build
create mode 100644 hw/ufs/trace-events
diff --git a/MAINTAINERS b/MAINTAINERS
index b471973e1e..3ac4ac6219 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2248,6 +2248,12 @@ F: tests/qtest/nvme-test.c
F: docs/system/devices/nvme.rst
T: git git://git.infradead.org/qemu-nvme.git nvme-next
+ufs
+M: Jeuk Kim
+S: Supported
+F: hw/ufs/*
+F: include/block/ufs.h
+
megasas
M: Hannes Reinecke
L: qemu-block@nongnu.org
diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
index e302bea484..d6707fa069 100644
--- a/docs/specs/pci-ids.rst
+++ b/docs/specs/pci-ids.rst
@@ -92,6 +92,8 @@ PCI devices (other than virtio):
PCI PVPanic device (``-device pvpanic-pci``)
1b36:0012
PCI ACPI ERST device (``-device acpi-erst``)
+1b36:0013
+ PCI UFS device (``-device ufs``)
All these devices are documented in :doc:`index`.
diff --git a/meson.build b/meson.build
index bf9831c715..0e31bdfabf 100644
--- a/meson.build
+++ b/meson.build
@@ -3287,6 +3287,7 @@ if have_system
'hw/ssi',
'hw/timer',
'hw/tpm',
+'hw/ufs',
'hw/usb',
'hw/vfio',
'hw/virtio',
diff --git a/hw/ufs/trace.h b/hw/ufs/trace.h
new file mode 100644
index 00..2dbd6397c3
--- /dev/null
+++ b/hw/ufs/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-hw_ufs.h"
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
new file mode 100644
index 00..d9d195caec
--- /dev/null
+++ b/hw/ufs/ufs.h
@@ -0,0 +1,42 @@
+/*
+ * QEMU UFS
+ *
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd. All rights reserved.
+ *
+ * Written by Jeuk Kim
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_UFS_UFS_H
+#define HW_UFS_UFS_H
+
+#include "hw/pci/pci_device.h"
+#include "hw/scsi/scsi.h"
+#include "block/ufs.h"
+
+#define UFS_MAX_LUS 32
+#define UFS_BLOCK_SIZE 4096
+
+typedef struct UfsParams {
+char *serial;
+uint8_t nutrs; /* Number of UTP Transfer Request Slots */
+uint8_t nutmrs; /* Number of UTP Task Management Request Slots */
+} UfsParams;
+
+typedef struct UfsHc {
+PCIDevice parent_obj;
+MemoryRegion iomem;
+UfsReg reg;
+UfsParams params;
+uint32_t reg_size;
+
+qemu_irq irq;
+QEMUBH *doorbell_bh;
+QEMUBH *complete_bh;
+} UfsHc;
+
+#define TYPE_UFS "ufs"
+#define UFS(obj) OBJECT_CHECK(UfsHc, (obj), TYPE_UFS)
+
+#endif /* HW_UFS_UFS_H */
diff --git a/include/block/ufs.h b/include/block/ufs.h
new file mode 100644
index 00..fd884eb8ce
--- /dev/null
+++ b/include/block/ufs.h
@@ -0,0 +1,1090 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef BLOCK_UFS_H
+#define BLOCK_UFS_H
+
+#include "hw/registerfields.h"
+
+typedef struct QEMU_PACKED UfsReg {
+uint32_t cap;
+uint32_t rsvd0;
+uint32_t ver;
+uint32_t rsvd1;
+uint32_t hcpid;
+uint32_t hcmid;
+uint32_t ahit;
+uint32_t rsvd2;
+uint32_t is;
+uint32_t ie;
+uint32_t rsvd3[2];
+uint32_t hcs;
+uint32_t hce;
+uint32_t uecpa;
+uint32_t uecdl;
+uint32_t uecn;
+uint32_t uect;
+uint32_t uecdme;
+uint32_t utriacr;
+uint32_t utrlba;
+uint32_t utrlbau;
+uint32_t utrldbr;
+uint32_t utrlclr;
+uint32_t utrlrsr;
+uint32_t utrlcnr;
+uint32_t rsvd4[2];
+uint32_t utmrlba;
+uint32_t utmrlbau;
+uint32_t utmrldbr;
+uint32_t utmrlclr;
+uint32_t utmrlrsr;
+uint32_t rsvd5[3];
+uint32_t uiccmd;
+uint32_t ucmdarg1;
+uint32_t ucmdarg2;
+uint32_t ucmdarg3;
+uint32_t rsvd6[4];
+uint32_t rsvd7[4];
+uint32_t rsvd8[16];
+uint32_t ccap;
+} UfsReg;
+
+REG32(CAP, offsetof(UfsReg, cap))
+FIELD(CAP, NUTRS, 0, 5)
+FIELD(CAP, RTT, 8, 8)
+FIELD(CAP, NUTMRS, 16, 3)
+FIELD(CAP,
[PULL 1/5] iothread: Set the GSource "name" field
From: Fabiano Rosas
Having a name in the source helps with debugging core dumps when one
might not have access to TLS data to cross-reference AioContexts with
their addresses.
Signed-off-by: Fabiano Rosas
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20230905180359.14083-1-faro...@suse.de
Signed-off-by: Stefan Hajnoczi
---
iothread.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/iothread.c b/iothread.c
index b41c305bd9..b753286414 100644
--- a/iothread.c
+++ b/iothread.c
@@ -138,12 +138,14 @@ static void iothread_instance_finalize(Object *obj)
qemu_sem_destroy(>init_done_sem);
}
-static void iothread_init_gcontext(IOThread *iothread)
+static void iothread_init_gcontext(IOThread *iothread, const char *thread_name)
{
GSource *source;
+g_autofree char *name = g_strdup_printf("%s aio-context", thread_name);
iothread->worker_context = g_main_context_new();
source = aio_get_g_source(iothread_get_aio_context(iothread));
+g_source_set_name(source, name);
g_source_attach(source, iothread->worker_context);
g_source_unref(source);
iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
@@ -180,7 +182,7 @@ static void iothread_init(EventLoopBase *base, Error **errp)
{
Error *local_error = NULL;
IOThread *iothread = IOTHREAD(base);
-char *thread_name;
+g_autofree char *thread_name = NULL;
iothread->stopping = false;
iothread->running = true;
@@ -189,11 +191,14 @@ static void iothread_init(EventLoopBase *base, Error
**errp)
return;
}
+thread_name = g_strdup_printf("IO %s",
+object_get_canonical_path_component(OBJECT(base)));
+
/*
* Init one GMainContext for the iothread unconditionally, even if
* it's not used
*/
-iothread_init_gcontext(iothread);
+iothread_init_gcontext(iothread, thread_name);
iothread_set_aio_context_params(base, _error);
if (local_error) {
@@ -206,11 +211,8 @@ static void iothread_init(EventLoopBase *base, Error
**errp)
/* This assumes we are called from a thread with useful CPU affinity for us
* to inherit.
*/
-thread_name = g_strdup_printf("IO %s",
-object_get_canonical_path_component(OBJECT(base)));
qemu_thread_create(>thread, thread_name, iothread_run,
iothread, QEMU_THREAD_JOINABLE);
-g_free(thread_name);
/* Wait for initialization to complete */
while (iothread->thread_id == -1) {
--
2.41.0
[PULL 5/5] tests/qtest: Introduce tests for UFS
From: Jeuk Kim
This patch includes the following tests
Test mmio read
Test ufs device initialization and ufs-lu recognition
Test I/O (Performs a write followed by a read to verify)
Signed-off-by: Jeuk Kim
Acked-by: Thomas Huth
Reviewed-by: Stefan Hajnoczi
Message-id:
9e9207f54505e9ba30931849f949ff6f474ac333.1693980783.git.jeuk20@gmail.com
Signed-off-by: Stefan Hajnoczi
---
MAINTAINERS | 1 +
tests/qtest/ufs-test.c | 587
tests/qtest/meson.build | 1 +
3 files changed, 589 insertions(+)
create mode 100644 tests/qtest/ufs-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3ac4ac6219..bf2366815b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2253,6 +2253,7 @@ M: Jeuk Kim
S: Supported
F: hw/ufs/*
F: include/block/ufs.h
+F: tests/qtest/ufs-test.c
megasas
M: Hannes Reinecke
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
new file mode 100644
index 00..ed3dbca154
--- /dev/null
+++ b/tests/qtest/ufs-test.c
@@ -0,0 +1,587 @@
+/*
+ * QTest testcase for UFS
+ *
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd. All rights reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+#include "libqtest.h"
+#include "libqos/qgraph.h"
+#include "libqos/pci.h"
+#include "scsi/constants.h"
+#include "include/block/ufs.h"
+
+/* Test images sizes in Bytes */
+#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
+/* Timeout for various operations, in seconds. */
+#define TIMEOUT_SECONDS 10
+/* Maximum PRD entry count */
+#define MAX_PRD_ENTRY_COUNT 10
+#define PRD_ENTRY_DATA_SIZE 4096
+/* Constants to build upiu */
+#define UTP_COMMAND_DESCRIPTOR_SIZE 4096
+#define UTP_RESPONSE_UPIU_OFFSET 1024
+#define UTP_PRDT_UPIU_OFFSET 2048
+
+typedef struct QUfs QUfs;
+
+struct QUfs {
+QOSGraphObject obj;
+QPCIDevice dev;
+QPCIBar bar;
+
+uint64_t utrlba;
+uint64_t utmrlba;
+uint64_t cmd_desc_addr;
+uint64_t data_buffer_addr;
+
+bool enabled;
+};
+
+static inline uint32_t ufs_rreg(QUfs *ufs, size_t offset)
+{
+return qpci_io_readl(>dev, ufs->bar, offset);
+}
+
+static inline void ufs_wreg(QUfs *ufs, size_t offset, uint32_t value)
+{
+qpci_io_writel(>dev, ufs->bar, offset, value);
+}
+
+static void ufs_wait_for_irq(QUfs *ufs)
+{
+uint64_t end_time;
+uint32_t is;
+/* Wait for device to reset as the linux driver does. */
+end_time = g_get_monotonic_time() + TIMEOUT_SECONDS * G_TIME_SPAN_SECOND;
+do {
+qtest_clock_step(ufs->dev.bus->qts, 100);
+is = ufs_rreg(ufs, A_IS);
+} while (is == 0 && g_get_monotonic_time() < end_time);
+}
+
+static UtpTransferReqDesc ufs_build_req_utrd(uint64_t cmd_desc_addr,
+ uint8_t slot,
+ uint32_t data_direction,
+ uint16_t prd_table_length)
+{
+UtpTransferReqDesc req = { 0 };
+uint64_t command_desc_base_addr =
+cmd_desc_addr + slot * UTP_COMMAND_DESCRIPTOR_SIZE;
+
+req.header.dword_0 =
+cpu_to_le32(1 << 28 | data_direction | UFS_UTP_REQ_DESC_INT_CMD);
+req.header.dword_2 = cpu_to_le32(UFS_OCS_INVALID_COMMAND_STATUS);
+
+req.command_desc_base_addr_hi = cpu_to_le32(command_desc_base_addr >> 32);
+req.command_desc_base_addr_lo =
+cpu_to_le32(command_desc_base_addr & 0x);
+req.response_upiu_offset =
+cpu_to_le16(UTP_RESPONSE_UPIU_OFFSET / sizeof(uint32_t));
+req.response_upiu_length = cpu_to_le16(sizeof(UtpUpiuRsp));
+req.prd_table_offset = cpu_to_le16(UTP_PRDT_UPIU_OFFSET /
sizeof(uint32_t));
+req.prd_table_length = cpu_to_le16(prd_table_length);
+return req;
+}
+
+static void ufs_send_nop_out(QUfs *ufs, uint8_t slot,
+ UtpTransferReqDesc *utrd_out, UtpUpiuRsp *rsp_out)
+{
+/* Build up utp transfer request descriptor */
+UtpTransferReqDesc utrd = ufs_build_req_utrd(ufs->cmd_desc_addr, slot,
+ UFS_UTP_NO_DATA_TRANSFER, 0);
+uint64_t utrd_addr = ufs->utrlba + slot * sizeof(UtpTransferReqDesc);
+uint64_t req_upiu_addr =
+ufs->cmd_desc_addr + slot * UTP_COMMAND_DESCRIPTOR_SIZE;
+uint64_t rsp_upiu_addr = req_upiu_addr + UTP_RESPONSE_UPIU_OFFSET;
+qtest_memwrite(ufs->dev.bus->qts, utrd_addr, , sizeof(utrd));
+
+/* Build up request upiu */
+UtpUpiuReq req_upiu = { 0 };
+req_upiu.header.trans_type = UFS_UPIU_TRANSACTION_NOP_OUT;
+req_upiu.header.task_tag = slot;
+qtest_memwrite(ufs->dev.bus->qts, req_upiu_addr, _upiu,
+ sizeof(req_upiu));
+
+/* Ring Doorbell */
+ufs_wreg(ufs, A_UTRLDBR, 1);
+ufs_wait_for_irq(ufs);
+g_assert_true(FIELD_EX32(ufs_rreg(ufs, A_IS), IS, UTRCS));
+ufs_wreg(ufs, A_IS, FIELD_DP32(0, IS, UTRCS, 1));
+
+qtest_memread(ufs->dev.bus->qts, utrd_addr, utrd_out,
[PULL 3/5] hw/ufs: Support for Query Transfer Requests
From: Jeuk Kim
This commit makes the UFS device support query
and nop out transfer requests.
The next patch would be support for UFS logical
unit and scsi command transfer request.
Signed-off-by: Jeuk Kim
Reviewed-by: Stefan Hajnoczi
Message-id:
ff7a5f0fd26761936a553ffb89d3df0ba62844e9.1693980783.git.jeuk20@gmail.com
Signed-off-by: Stefan Hajnoczi
---
hw/ufs/ufs.h| 46 +++
hw/ufs/ufs.c| 988 +++-
hw/ufs/trace-events | 1 +
3 files changed, 1033 insertions(+), 2 deletions(-)
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index d9d195caec..3d1b2cff4e 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -18,6 +18,32 @@
#define UFS_MAX_LUS 32
#define UFS_BLOCK_SIZE 4096
+typedef enum UfsRequestState {
+UFS_REQUEST_IDLE = 0,
+UFS_REQUEST_READY = 1,
+UFS_REQUEST_RUNNING = 2,
+UFS_REQUEST_COMPLETE = 3,
+UFS_REQUEST_ERROR = 4,
+} UfsRequestState;
+
+typedef enum UfsReqResult {
+UFS_REQUEST_SUCCESS = 0,
+UFS_REQUEST_FAIL = 1,
+} UfsReqResult;
+
+typedef struct UfsRequest {
+struct UfsHc *hc;
+UfsRequestState state;
+int slot;
+
+UtpTransferReqDesc utrd;
+UtpUpiuReq req_upiu;
+UtpUpiuRsp rsp_upiu;
+
+/* for scsi command */
+QEMUSGList *sg;
+} UfsRequest;
+
typedef struct UfsParams {
char *serial;
uint8_t nutrs; /* Number of UTP Transfer Request Slots */
@@ -30,6 +56,12 @@ typedef struct UfsHc {
UfsReg reg;
UfsParams params;
uint32_t reg_size;
+UfsRequest *req_list;
+
+DeviceDescriptor device_desc;
+GeometryDescriptor geometry_desc;
+Attributes attributes;
+Flags flags;
qemu_irq irq;
QEMUBH *doorbell_bh;
@@ -39,4 +71,18 @@ typedef struct UfsHc {
#define TYPE_UFS "ufs"
#define UFS(obj) OBJECT_CHECK(UfsHc, (obj), TYPE_UFS)
+typedef enum UfsQueryFlagPerm {
+UFS_QUERY_FLAG_NONE = 0x0,
+UFS_QUERY_FLAG_READ = 0x1,
+UFS_QUERY_FLAG_SET = 0x2,
+UFS_QUERY_FLAG_CLEAR = 0x4,
+UFS_QUERY_FLAG_TOGGLE = 0x8,
+} UfsQueryFlagPerm;
+
+typedef enum UfsQueryAttrPerm {
+UFS_QUERY_ATTR_NONE = 0x0,
+UFS_QUERY_ATTR_READ = 0x1,
+UFS_QUERY_ATTR_WRITE = 0x2,
+} UfsQueryAttrPerm;
+
#endif /* HW_UFS_UFS_H */
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index df87f2a6d5..56a8ec286b 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -15,10 +15,221 @@
#include "ufs.h"
/* The QEMU-UFS device follows spec version 3.1 */
-#define UFS_SPEC_VER 0x0310
+#define UFS_SPEC_VER 0x0310
#define UFS_MAX_NUTRS 32
#define UFS_MAX_NUTMRS 8
+static MemTxResult ufs_addr_read(UfsHc *u, hwaddr addr, void *buf, int size)
+{
+hwaddr hi = addr + size - 1;
+
+if (hi < addr) {
+return MEMTX_DECODE_ERROR;
+}
+
+if (!FIELD_EX32(u->reg.cap, CAP, 64AS) && (hi >> 32)) {
+return MEMTX_DECODE_ERROR;
+}
+
+return pci_dma_read(PCI_DEVICE(u), addr, buf, size);
+}
+
+static MemTxResult ufs_addr_write(UfsHc *u, hwaddr addr, const void *buf,
+ int size)
+{
+hwaddr hi = addr + size - 1;
+if (hi < addr) {
+return MEMTX_DECODE_ERROR;
+}
+
+if (!FIELD_EX32(u->reg.cap, CAP, 64AS) && (hi >> 32)) {
+return MEMTX_DECODE_ERROR;
+}
+
+return pci_dma_write(PCI_DEVICE(u), addr, buf, size);
+}
+
+static void ufs_complete_req(UfsRequest *req, UfsReqResult req_result);
+
+static inline hwaddr ufs_get_utrd_addr(UfsHc *u, uint32_t slot)
+{
+hwaddr utrl_base_addr = (((hwaddr)u->reg.utrlbau) << 32) + u->reg.utrlba;
+hwaddr utrd_addr = utrl_base_addr + slot * sizeof(UtpTransferReqDesc);
+
+return utrd_addr;
+}
+
+static inline hwaddr ufs_get_req_upiu_base_addr(const UtpTransferReqDesc *utrd)
+{
+uint32_t cmd_desc_base_addr_lo =
+le32_to_cpu(utrd->command_desc_base_addr_lo);
+uint32_t cmd_desc_base_addr_hi =
+le32_to_cpu(utrd->command_desc_base_addr_hi);
+
+return (((hwaddr)cmd_desc_base_addr_hi) << 32) + cmd_desc_base_addr_lo;
+}
+
+static inline hwaddr ufs_get_rsp_upiu_base_addr(const UtpTransferReqDesc *utrd)
+{
+hwaddr req_upiu_base_addr = ufs_get_req_upiu_base_addr(utrd);
+uint32_t rsp_upiu_byte_off =
+le16_to_cpu(utrd->response_upiu_offset) * sizeof(uint32_t);
+return req_upiu_base_addr + rsp_upiu_byte_off;
+}
+
+static MemTxResult ufs_dma_read_utrd(UfsRequest *req)
+{
+UfsHc *u = req->hc;
+hwaddr utrd_addr = ufs_get_utrd_addr(u, req->slot);
+MemTxResult ret;
+
+ret = ufs_addr_read(u, utrd_addr, >utrd, sizeof(req->utrd));
+if (ret) {
+trace_ufs_err_dma_read_utrd(req->slot, utrd_addr);
+}
+return ret;
+}
+
+static MemTxResult ufs_dma_read_req_upiu(UfsRequest *req)
+{
+UfsHc *u = req->hc;
+hwaddr req_upiu_base_addr = ufs_get_req_upiu_base_addr(>utrd);
+UtpUpiuReq *req_upiu = >req_upiu;
+uint32_t copy_size;
+uint16_t data_segment_length;
+MemTxResult ret;
+
+/*
+ * To know the size of the req_upiu, we need to read the
+ * data_segment_length
[PULL 0/5] Block patches
The following changes since commit 03a3a62fbd0aa5227e978eef3c67d3978aec9e5f: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-09-07 10:29:06 -0400) are available in the Git repository at: https://gitlab.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 631c872614aca91eaf947c1748f0f27f99635d92: tests/qtest: Introduce tests for UFS (2023-09-07 14:01:29 -0400) Pull request - Jeuk Kim's emulated UFS device - Fabiano Rosas' IOThread GSource "name" debugging aid Fabiano Rosas (1): iothread: Set the GSource "name" field Jeuk Kim (4): hw/ufs: Initial commit for emulated Universal-Flash-Storage hw/ufs: Support for Query Transfer Requests hw/ufs: Support for UFS logical unit tests/qtest: Introduce tests for UFS MAINTAINERS |7 + docs/specs/pci-ids.rst |2 + meson.build |1 + hw/ufs/trace.h |1 + hw/ufs/ufs.h | 131 include/block/ufs.h | 1090 +++ include/hw/pci/pci.h |1 + include/hw/pci/pci_ids.h |1 + include/scsi/constants.h |1 + hw/ufs/lu.c | 1445 hw/ufs/ufs.c | 1502 ++ iothread.c | 14 +- tests/qtest/ufs-test.c | 587 +++ hw/Kconfig |1 + hw/meson.build |1 + hw/ufs/Kconfig |4 + hw/ufs/meson.build |1 + hw/ufs/trace-events | 58 ++ tests/qtest/meson.build |1 + 19 files changed, 4843 insertions(+), 6 deletions(-) create mode 100644 hw/ufs/trace.h create mode 100644 hw/ufs/ufs.h create mode 100644 include/block/ufs.h create mode 100644 hw/ufs/lu.c create mode 100644 hw/ufs/ufs.c create mode 100644 tests/qtest/ufs-test.c create mode 100644 hw/ufs/Kconfig create mode 100644 hw/ufs/meson.build create mode 100644 hw/ufs/trace-events -- 2.41.0
Re: [PATCH v10 0/4] hw/ufs: Add Universal Flash Storage (UFS) support
On Wed, 6 Sept 2023 at 03:45, Jeuk Kim wrote: > > Since v9: > - Added the "UFS_" prefix to all define and enum defined in block/ufs.h. > This fixes > https://gitlab.com/qemu-project/qemu/-/jobs/4977255992 > which is a win32 build error. > > - Fixed not to use pointer type casting (uint32_t * -> unsigned long *). > It causes the bug in the find_first_bit() function on big endian host pc. > This fixes > https://gitlab.com/qemu-project/qemu/-/jobs/4977256030 > which is qos-test failure on s390x hosts. > > Please let me know if there are any problems. > Thank you very much! Applied, thanks! https://gitlab.com/stefanha/qemu/-/commits/block Stefan > > Jeuk > > Jeuk Kim (4): > hw/ufs: Initial commit for emulated Universal-Flash-Storage > hw/ufs: Support for Query Transfer Requests > hw/ufs: Support for UFS logical unit > tests/qtest: Introduce tests for UFS > > MAINTAINERS |7 + > docs/specs/pci-ids.rst |2 + > hw/Kconfig |1 + > hw/meson.build |1 + > hw/ufs/Kconfig |4 + > hw/ufs/lu.c | 1445 > hw/ufs/meson.build |1 + > hw/ufs/trace-events | 58 ++ > hw/ufs/trace.h |1 + > hw/ufs/ufs.c | 1502 ++ > hw/ufs/ufs.h | 131 > include/block/ufs.h | 1090 +++ > include/hw/pci/pci.h |1 + > include/hw/pci/pci_ids.h |1 + > include/scsi/constants.h |1 + > meson.build |1 + > tests/qtest/meson.build |1 + > tests/qtest/ufs-test.c | 587 +++ > 18 files changed, 4835 insertions(+) > create mode 100644 hw/ufs/Kconfig > create mode 100644 hw/ufs/lu.c > create mode 100644 hw/ufs/meson.build > create mode 100644 hw/ufs/trace-events > create mode 100644 hw/ufs/trace.h > create mode 100644 hw/ufs/ufs.c > create mode 100644 hw/ufs/ufs.h > create mode 100644 include/block/ufs.h > create mode 100644 tests/qtest/ufs-test.c > > -- > 2.34.1 > >
Re: [PULL for-6.2 0/7] Ide patches
On Thu, Sep 7, 2023, 12:49 PM Michael Tokarev wrote: > 07.09.2023 06:42, John Snow wrote: > > > > > IDE Pull request > > > > > > > > Niklas Cassel (7): > >hw/ide/core: set ERR_STAT in unsupported command completion > >hw/ide/ahci: write D2H FIS when processing NCQ command > >hw/ide/ahci: simplify and document PxCI handling > >hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared > >hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set > >hw/ide/ahci: fix ahci_write_fis_sdb() > >hw/ide/ahci: fix broken SError handling > > Is anything here stable-worthy? > > /mjt > Yes, assuming it doesn't break anything. I can't give IDE the testing it deserves anymore, but I trust Niklas. I don't have good test suites for *inside* linux/windows guests so I am admittedly relying on qtests and for people to bark if something regressed. I'd say to tentatively add them to your list and if we find regressions during this window, we can exclude them from a stable release. >
Re: [PULL for-6.2 0/7] Ide patches
07.09.2023 06:42, John Snow wrote: IDE Pull request Niklas Cassel (7): hw/ide/core: set ERR_STAT in unsupported command completion hw/ide/ahci: write D2H FIS when processing NCQ command hw/ide/ahci: simplify and document PxCI handling hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set hw/ide/ahci: fix ahci_write_fis_sdb() hw/ide/ahci: fix broken SError handling Is anything here stable-worthy? /mjt
Re: [PULL 00/18] Parallels format driver
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PULL for-6.2 0/7] Ide patches
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PULL 00/14] Block patches
On 06.09.23 15:18, Stefan Hajnoczi wrote: On Fri, 1 Sept 2023 at 04:18, Hanna Czenczek wrote: The following changes since commit f5fe7c17ac4e309e47e78f0f9761aebc8d2f2c81: Merge tag 'pull-tcg-20230823-2' of https://gitlab.com/rth7680/qemu into staging (2023-08-28 16:07:04 -0400) are available in the Git repository at: https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-09-01 Hi Hanna, Please push a signed tag (git tag -s). Thanks! Is it not signed? I don’t think gitlab has support to show that, but github shows it as verified: https://github.com/XanClic/qemu/releases/tag/pull-block-2023-09-01 And when I clone it: ``` $ git clone https://gitlab.com/hreitz/qemu -b pull-block-2023-09-01 --depth=1 [...] $ cd qemu $ git tag -v pull-block-2023-09-01 LANG=C git tag -v pull-block-2023-09-01 object 380448464dd89291cf7fd7434be6c225482a334d type commit tag pull-block-2023-09-01 tagger Hanna Reitz 1693555853 +0200 Block patches - Fix for file-posix's zoning code crashing on I/O errors - Throttling refactoring gpg: Signature made Fri Sep 1 10:11:46 2023 CEST gpg: using RSA key CB62D7A0EE3829E45F004D34A1FA40D098019CDF gpg: issuer "hre...@redhat.com" gpg: Good signature from "Hanna Reitz " [ultimate] Primary key fingerprint: CB62 D7A0 EE38 29E4 5F00 4D34 A1FA 40D0 9801 9CDF ``` Hanna
Re: [PULL 00/14] Block layer patches
Am 06.09.2023 um 17:13 hat Stefan Hajnoczi geschrieben: > test-bdrv-drain is failing. I think my coroutine wrapper patch might > be necessary: > https://gitlab.com/qemu-project/qemu/-/jobs/5029372308#L4907 > > I have dropped this patch series for the time being. Feel free to > remove my patches and send a new revision. > > I will debug the test-bdrv-drain issue. Hm, I can't reproduce it. And even in CI only seems to have happened in this one job (ubuntu-20.04-s390x-all). If you think that it's caused by your patches, I can drop them for v2. Kevin
Re: [PULL for-6.2 0/7] Ide patches
On 7/9/23 05:43, John Snow wrote: I guess the last time I sent IDE patches was for 6.2 and that tag got stuck in my git-publish invocation, oops. I am not suggesting we break the laws of causality to merge these patches. lol On Wed, Sep 6, 2023 at 11:42 PM John Snow wrote: The following changes since commit c152379422a204109f34ca2b43ecc538c7d738ae: Merge tag 'ui-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-09-06 11:16:01 -0400) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/ide-pull-request for you to fetch changes up to 9f89423537653de07ca40c18b5ff5b70b104cc93: hw/ide/ahci: fix broken SError handling (2023-09-06 22:48:04 -0400) IDE Pull request \o/ Niklas Cassel (7): hw/ide/core: set ERR_STAT in unsupported command completion hw/ide/ahci: write D2H FIS when processing NCQ command hw/ide/ahci: simplify and document PxCI handling hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set hw/ide/ahci: fix ahci_write_fis_sdb() hw/ide/ahci: fix broken SError handling
