* [Qemu-devel] [PULL 01/15] block: add missed aio_context_acquire into release_drive
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 02/15] nbd sockets vnc: Mark problematic address family tests TODO Max Reitz
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: "Denis V. Lunev" <den@openvz.org>
Recently we expirience hang with iothreads enabled with the following
call trace:
Thread 1 (Thread 0x7fa95efebc80 (LWP 177117)):
0 ppoll () from /lib64/libc.so.6
2 qemu_poll_ns () at qemu-timer.c:313
3 aio_poll () at aio-posix.c:457
4 bdrv_flush () at block/io.c:2641
5 bdrv_close () at block.c:2143
6 bdrv_delete () at block.c:2352
7 bdrv_unref () at block.c:3429
8 blk_remove_bs () at block/block-backend.c:427
9 blk_delete () at block/block-backend.c:178
10 blk_unref () at block/block-backend.c:226
11 object_property_del_all () at qom/object.c:399
12 object_finalize () at qom/object.c:461
13 object_unref () at qom/object.c:898
14 object_property_del_child () at qom/object.c:422
15 qmp_marshal_device_del () at qmp-marshal.c:1145
16 handle_qmp_command () at /usr/src/debug/qemu-2.6.0/monitor.c:3929
Technically bdrv_flush() stucks in
while (rwco.ret == NOT_DONE) {
aio_poll(aio_context, true);
}
but rwco.ret is equal to 0 thus we have missed wakeup. Code investigation
reveals that we do not have performed aio_context_acquire() on this call
stack.
This patch adds missed lock.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
Message-id: 1490717566-25516-1-git-send-email-den@openvz.org
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/core/qdev-properties-system.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index c34be1c1ba..e885e650fb 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -124,8 +124,12 @@ static void release_drive(Object *obj, const char *name, void *opaque)
BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
if (*ptr) {
+ AioContext *ctx = blk_get_aio_context(*ptr);
+
+ aio_context_acquire(ctx);
blockdev_auto_del(*ptr);
blk_detach_dev(*ptr, dev);
+ aio_context_release(ctx);
}
}
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 02/15] nbd sockets vnc: Mark problematic address family tests TODO
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 01/15] block: add missed aio_context_acquire into release_drive Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 03/15] char: Fix socket with "type": "vsock" address Max Reitz
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
Certain features make sense only with certain address families. For
instance, passing file descriptors requires AF_UNIX. Testing
SocketAddress's saddr->type == SOCKET_ADDRESS_KIND_UNIX is obvious,
but problematic: it can't recognize AF_UNIX when type ==
SOCKET_ADDRESS_KIND_FD.
Mark such tests of saddr->type TODO. We may want to check the address
family with getsockname() there.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1490895797-29094-2-git-send-email-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 1 +
blockdev-nbd.c | 1 +
chardev/char-socket.c | 5 ++---
ui/vnc.c | 1 +
util/qemu-sockets.c | 4 ++++
5 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 1b832c2132..36ea617989 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -421,6 +421,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
goto error;
}
+ /* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
if (s->saddr->type != SOCKET_ADDRESS_KIND_INET) {
error_setg(errp, "TLS only supported over IP sockets");
goto error;
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 7ea836b46e..8a11807df3 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -124,6 +124,7 @@ void qmp_nbd_server_start(SocketAddress *addr,
goto error;
}
+ /* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
if (addr->type != SOCKET_ADDRESS_KIND_INET) {
error_setg(errp, "TLS is only supported with IPv4/IPv6");
goto error;
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index d7e92e1bd3..6344b07822 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -47,7 +47,6 @@ typedef struct {
int max_size;
int do_telnetopt;
int do_nodelay;
- int is_unix;
int *read_msgfds;
size_t read_msgfds_num;
int *write_msgfds;
@@ -825,7 +824,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
QIOChannelSocket *sioc = NULL;
- s->is_unix = addr->type == SOCKET_ADDRESS_KIND_UNIX;
s->is_listen = is_listen;
s->is_telnet = is_telnet;
s->do_nodelay = do_nodelay;
@@ -865,7 +863,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
s->addr = QAPI_CLONE(SocketAddress, sock->addr);
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
- if (s->is_unix) {
+ /* TODO SOCKET_ADDRESS_FD where fd has AF_UNIX */
+ if (addr->type == SOCKET_ADDRESS_KIND_UNIX) {
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 243e99bb95..1095f9de68 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3642,6 +3642,7 @@ static int vnc_display_connect(VncDisplay *vd,
error_setg(errp, "Expected a single address in reverse mode");
return -1;
}
+ /* TODO SOCKET_ADDRESS_KIND_FD when fd has AF_UNIX */
vd->is_unix = saddr[0]->type == SOCKET_ADDRESS_KIND_UNIX;
sioc = qio_channel_socket_new();
qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 40164bf681..9b7368113c 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1154,6 +1154,10 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
{
int fd;
+ /*
+ * TODO SOCKET_ADDRESS_KIND_FD when fd is AF_INET or AF_INET6
+ * (although other address families can do SOCK_DGRAM, too)
+ */
switch (remote->type) {
case SOCKET_ADDRESS_KIND_INET:
fd = inet_dgram_saddr(remote->u.inet.data,
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 03/15] char: Fix socket with "type": "vsock" address
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 01/15] block: add missed aio_context_acquire into release_drive Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 02/15] nbd sockets vnc: Mark problematic address family tests TODO Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 04/15] io vnc sockets: Clean up SocketAddressKind switches Max Reitz
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
Watch this:
$ qemu-system-x86_64 -nodefaults -S -display none -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 91, "minor": 8, "major": 2}, "package": " (v2.8.0-1195-gf84141e-dirty)"}, "capabilities": []}}
{ "execute": "qmp_capabilities" }
{"return": {}}
{ "execute": "chardev-add", "arguments": { "id": "chr0", "backend": { "type": "socket", "data": { "addr": { "type": "vsock", "data": { "cid": "CID", "port": "P" }}}}}}
Aborted (core dumped)
Crashes because SocketAddress_to_str() is blissfully unaware of
SOCKET_ADDRESS_KIND_VSOCK. Fix that. Pick the output format to match
socket_parse(), just like the existing formats.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1490895797-29094-3-git-send-email-armbru@redhat.com
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
chardev/char-socket.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 6344b07822..36ab0d633a 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -357,6 +357,10 @@ static char *SocketAddress_to_str(const char *prefix, SocketAddress *addr,
return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.data->str,
is_listen ? ",server" : "");
break;
+ case SOCKET_ADDRESS_KIND_VSOCK:
+ return g_strdup_printf("%svsock:%s:%s", prefix,
+ addr->u.vsock.data->cid,
+ addr->u.vsock.data->port);
default:
abort();
}
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 04/15] io vnc sockets: Clean up SocketAddressKind switches
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (2 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 03/15] char: Fix socket with "type": "vsock" address Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 05/15] block: Document -drive problematic code and bugs Max Reitz
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
We have quite a few switches over SocketAddressKind. Some have case
labels for all enumeration values, others rely on a default label.
Some abort when the value isn't a valid SocketAddressKind, others
report an error then.
Unify as follows. Always provide case labels for all enumeration
values, to clarify intent. Abort when the value isn't a valid
SocketAddressKind, because the program state is messed up then.
Improve a few error messages while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1490895797-29094-4-git-send-email-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
io/dns-resolver.c | 7 +++++--
ui/vnc.c | 18 ++++++++++++------
util/qemu-sockets.c | 4 +---
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/io/dns-resolver.c b/io/dns-resolver.c
index 0ac6b23c02..a407075934 100644
--- a/io/dns-resolver.c
+++ b/io/dns-resolver.c
@@ -164,9 +164,12 @@ int qio_dns_resolver_lookup_sync(QIODNSResolver *resolver,
addrs,
errp);
- default:
- error_setg(errp, "Unknown socket address kind");
+ case SOCKET_ADDRESS_KIND_FD:
+ error_setg(errp, "Unsupported socket address type 'fd'");
return -1;
+
+ default:
+ abort();
}
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 1095f9de68..349cfc9d86 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -129,10 +129,13 @@ static void vnc_init_basic_info(SocketAddress *addr,
info->family = NETWORK_ADDRESS_FAMILY_UNIX;
break;
- default:
- error_setg(errp, "Unsupported socket kind %d",
- addr->type);
+ case SOCKET_ADDRESS_KIND_VSOCK:
+ case SOCKET_ADDRESS_KIND_FD:
+ error_setg(errp, "Unsupported socket address type %s",
+ SocketAddressKind_lookup[addr->type]);
break;
+ default:
+ abort();
}
return;
@@ -411,10 +414,13 @@ VncInfo *qmp_query_vnc(Error **errp)
info->family = NETWORK_ADDRESS_FAMILY_UNIX;
break;
- default:
- error_setg(errp, "Unsupported socket kind %d",
- addr->type);
+ case SOCKET_ADDRESS_KIND_VSOCK:
+ case SOCKET_ADDRESS_KIND_FD:
+ error_setg(errp, "Unsupported socket address type %s",
+ SocketAddressKind_lookup[addr->type]);
goto out_error;
+ default:
+ abort();
}
info->has_host = true;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9b7368113c..4ae37bd2a2 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1337,9 +1337,7 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
break;
default:
- error_setg(errp, "socket family %d unsupported",
- addr->type);
- return NULL;
+ abort();
}
return buf;
}
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 05/15] block: Document -drive problematic code and bugs
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (3 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 04/15] io vnc sockets: Clean up SocketAddressKind switches Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 06/15] gluster: Prepare for SocketAddressFlat extension Max Reitz
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
-blockdev and blockdev_add convert their arguments via QObject to
BlockdevOptions for qmp_blockdev_add(), which converts them back to
QObject, then to a flattened QDict. The QDict's members are typed
according to the QAPI schema.
-drive converts its argument via QemuOpts to a (flat) QDict. This
QDict's members are all QString.
Thus, the QType of a flat QDict member depends on whether it comes
from -drive or -blockdev/blockdev_add, except when the QAPI type maps
to QString, which is the case for 'str' and enumeration types.
The block layer core extracts generic configuration from the flat
QDict, and the block driver extracts driver-specific configuration.
Both commonly do so by converting (parts of) the flat QDict to
QemuOpts, which turns all values into strings. Not exactly elegant,
but correct.
However, A few places access the flat QDict directly:
* Most of them access members that are always QString. Correct.
* bdrv_open_inherit() accesses a boolean, carefully. Correct.
* nfs_config() uses a QObject input visitor. Correct only because the
visited type contains nothing but QStrings.
* nbd_config() and ssh_config() use a QObject input visitor, and the
visited types contain non-QStrings: InetSocketAddress members
@numeric, @to, @ipv4, @ipv6. -drive works as long as you don't try
to use them (they're all optional). @to is ignored anyway.
Reproducer:
-drive driver=ssh,server.host=h,server.port=22,server.ipv4,path=p
-drive driver=nbd,server.type=inet,server.data.host=h,server.data.port=22,server.data.ipv4
both fail with "Invalid parameter type for 'data.ipv4', expected: boolean"
Add suitable comments to all these places. Mark the buggy ones FIXME.
"Fortunately", -drive's driver-specific options are entirely
undocumented.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1490895797-29094-5-git-send-email-armbru@redhat.com
[mreitz: Fixed two typos]
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
block/file-posix.c | 6 ++++++
block/nbd.c | 8 ++++++++
block/nfs.c | 7 +++++++
block/rbd.c | 6 ++++++
block/ssh.c | 8 ++++++++
6 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 6e906ec53c..927ba89eb7 100644
--- a/block.c
+++ b/block.c
@@ -1157,6 +1157,13 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,
if (file != NULL) {
filename = blk_bs(file)->filename;
} else {
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting
+ * non-string types would require more care. When @options
+ * come from -blockdev or blockdev_add, its members are typed
+ * according to the QAPI schema, but when they come from
+ * -drive, they're all QString.
+ */
filename = qdict_get_try_str(options, "filename");
}
@@ -1324,6 +1331,13 @@ static int bdrv_fill_options(QDict **options, const char *filename,
BlockDriver *drv = NULL;
Error *local_err = NULL;
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting non-string
+ * types would require more care. When @options come from
+ * -blockdev or blockdev_add, its members are typed according to
+ * the QAPI schema, but when they come from -drive, they're all
+ * QString.
+ */
drvname = qdict_get_try_str(*options, "driver");
if (drvname) {
drv = bdrv_find_format(drvname);
@@ -1358,6 +1372,7 @@ static int bdrv_fill_options(QDict **options, const char *filename,
}
/* Find the right block driver */
+ /* See cautionary note on accessing @options above */
filename = qdict_get_try_str(*options, "filename");
if (!drvname && protocol) {
@@ -1987,6 +2002,13 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
qdict_extract_subqdict(parent_options, &options, bdref_key_dot);
g_free(bdref_key_dot);
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting non-string
+ * types would require more care. When @parent_options come from
+ * -blockdev or blockdev_add, its members are typed according to
+ * the QAPI schema, but when they come from -drive, they're all
+ * QString.
+ */
reference = qdict_get_try_str(parent_options, bdref_key);
if (reference || qdict_haskey(options, "file.filename")) {
backing_filename[0] = '\0';
@@ -2059,6 +2081,13 @@ bdrv_open_child_bs(const char *filename, QDict *options, const char *bdref_key,
qdict_extract_subqdict(options, &image_options, bdref_key_dot);
g_free(bdref_key_dot);
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting non-string
+ * types would require more care. When @options come from
+ * -blockdev or blockdev_add, its members are typed according to
+ * the QAPI schema, but when they come from -drive, they're all
+ * QString.
+ */
reference = qdict_get_try_str(options, bdref_key);
if (!filename && !reference && !qdict_size(image_options)) {
if (!allow_none) {
@@ -2274,9 +2303,13 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
goto fail;
}
- /* Set the BDRV_O_RDWR and BDRV_O_ALLOW_RDWR flags.
- * FIXME: we're parsing the QDict to avoid having to create a
- * QemuOpts just for this, but neither option is optimal. */
+ /*
+ * Set the BDRV_O_RDWR and BDRV_O_ALLOW_RDWR flags.
+ * Caution: getting a boolean member of @options requires care.
+ * When @options come from -blockdev or blockdev_add, members are
+ * typed according to the QAPI schema, but when they come from
+ * -drive, they're all QString.
+ */
if (g_strcmp0(qdict_get_try_str(options, BDRV_OPT_READ_ONLY), "on") &&
!qdict_get_try_bool(options, BDRV_OPT_READ_ONLY, false)) {
flags |= (BDRV_O_RDWR | BDRV_O_ALLOW_RDWR);
@@ -2298,6 +2331,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
options = qdict_clone_shallow(options);
/* Find the right image format driver */
+ /* See cautionary note on accessing @options above */
drvname = qdict_get_try_str(options, "driver");
if (drvname) {
drv = bdrv_find_format(drvname);
@@ -2309,6 +2343,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
assert(drvname || !(flags & BDRV_O_PROTOCOL));
+ /* See cautionary note on accessing @options above */
backing = qdict_get_try_str(options, "backing");
if (backing && *backing == '\0') {
flags |= BDRV_O_NO_BACKING;
@@ -2787,6 +2822,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
do {
QString *new_obj = qobject_to_qstring(entry->value);
const char *new = qstring_get_str(new_obj);
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting
+ * non-string types would require more care. When
+ * bs->options come from -blockdev or blockdev_add, its
+ * members are typed according to the QAPI schema, but
+ * when they come from -drive, they're all QString.
+ */
const char *old = qdict_get_try_str(reopen_state->bs->options,
entry->key);
diff --git a/block/file-posix.c b/block/file-posix.c
index 0841a08785..0c4896876e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2193,6 +2193,12 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
#if defined(__APPLE__) && defined(__MACH__)
+ /*
+ * Caution: while qdict_get_str() is fine, getting non-string types
+ * would require more care. When @options come from -blockdev or
+ * blockdev_add, its members are typed according to the QAPI
+ * schema, but when they come from -drive, they're all QString.
+ */
const char *filename = qdict_get_str(options, "filename");
char bsd_path[MAXPATHLEN] = "";
bool error_occurred = false;
diff --git a/block/nbd.c b/block/nbd.c
index 36ea617989..11e3ba75fa 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -278,6 +278,14 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp)
goto done;
}
+ /*
+ * FIXME .numeric, .to, .ipv4 or .ipv6 don't work with -drive
+ * server.type=inet. .to doesn't matter, it's ignored anyway.
+ * That's because when @options come from -blockdev or
+ * blockdev_add, members are typed according to the QAPI schema,
+ * but when they come from -drive, they're all QString. The
+ * visitor expects the former.
+ */
iv = qobject_input_visitor_new(crumpled_addr);
visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
if (local_err) {
diff --git a/block/nfs.c b/block/nfs.c
index 3f43f6e26a..0816678307 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -474,6 +474,13 @@ static NFSServer *nfs_config(QDict *options, Error **errp)
goto out;
}
+ /*
+ * Caution: this works only because all scalar members of
+ * NFSServer are QString in @crumpled_addr. The visitor expects
+ * @crumpled_addr to be typed according to the QAPI schema. It
+ * is when @options come from -blockdev or blockdev_add. But when
+ * they come from -drive, they're all QString.
+ */
iv = qobject_input_visitor_new(crumpled_addr);
visit_type_NFSServer(iv, NULL, &server, &local_error);
if (local_error) {
diff --git a/block/rbd.c b/block/rbd.c
index fbdb131a68..1ceeeb5a60 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -385,6 +385,12 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
goto exit;
}
+ /*
+ * Caution: while qdict_get_try_str() is fine, getting non-string
+ * types would require more care. When @options come from -blockdev
+ * or blockdev_add, its members are typed according to the QAPI
+ * schema, but when they come from -drive, they're all QString.
+ */
pool = qdict_get_try_str(options, "pool");
conf = qdict_get_try_str(options, "conf");
clientname = qdict_get_try_str(options, "user");
diff --git a/block/ssh.c b/block/ssh.c
index 278e66faa6..471ba8a260 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -601,6 +601,14 @@ static InetSocketAddress *ssh_config(QDict *options, Error **errp)
goto out;
}
+ /*
+ * FIXME .numeric, .to, .ipv4 or .ipv6 don't work with -drive.
+ * .to doesn't matter, it's ignored anyway.
+ * That's because when @options come from -blockdev or
+ * blockdev_add, members are typed according to the QAPI schema,
+ * but when they come from -drive, they're all QString. The
+ * visitor expects the former.
+ */
iv = qobject_input_visitor_new(crumpled_addr);
visit_type_InetSocketAddress(iv, NULL, &inet, &local_error);
if (local_error) {
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 06/15] gluster: Prepare for SocketAddressFlat extension
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (4 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 05/15] block: Document -drive problematic code and bugs Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 07/15] qapi-schema: SocketAddressFlat variants 'vsock' and 'fd' Max Reitz
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
qemu_gluster_glfs_init() and qemu_gluster_parse_json() rely on the
fact that SocketAddressFlatType has only two members
SOCKET_ADDRESS_FLAT_TYPE_INET and SOCKET_ADDRESS_FLAT_TYPE_UNIX.
Correct, but won't stay correct. Make them more robust.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 1490895797-29094-6-git-send-email-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/gluster.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index a577daef10..fb0aafeaa3 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -412,10 +412,12 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
glfs_set_preopened(gconf->volume, glfs);
for (server = gconf->server; server; server = server->next) {
- if (server->value->type == SOCKET_ADDRESS_FLAT_TYPE_UNIX) {
+ switch (server->value->type) {
+ case SOCKET_ADDRESS_FLAT_TYPE_UNIX:
ret = glfs_set_volfile_server(glfs, "unix",
server->value->u.q_unix.path, 0);
- } else {
+ break;
+ case SOCKET_ADDRESS_FLAT_TYPE_INET:
if (parse_uint_full(server->value->u.inet.port, &port, 10) < 0 ||
port > 65535) {
error_setg(errp, "'%s' is not a valid port number",
@@ -426,6 +428,9 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
ret = glfs_set_volfile_server(glfs, "tcp",
server->value->u.inet.host,
(int)port);
+ break;
+ default:
+ abort();
}
if (ret < 0) {
@@ -487,7 +492,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
char *str = NULL;
const char *ptr;
size_t num_servers;
- int i;
+ int i, type;
/* create opts info from runtime_json_opts list */
opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
@@ -539,16 +544,17 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
if (!strcmp(ptr, "tcp")) {
ptr = "inet"; /* accept legacy "tcp" */
}
- gsconf->type = qapi_enum_parse(SocketAddressFlatType_lookup, ptr,
- SOCKET_ADDRESS_FLAT_TYPE__MAX, -1,
- &local_err);
- if (local_err) {
- error_append_hint(&local_err,
- "Parameter '%s' may be 'inet' or 'unix'\n",
- GLUSTER_OPT_TYPE);
+ type = qapi_enum_parse(SocketAddressFlatType_lookup, ptr,
+ SOCKET_ADDRESS_FLAT_TYPE__MAX, -1, NULL);
+ if (type != SOCKET_ADDRESS_FLAT_TYPE_INET
+ && type != SOCKET_ADDRESS_FLAT_TYPE_UNIX) {
+ error_setg(&local_err,
+ "Parameter '%s' may be 'inet' or 'unix'",
+ GLUSTER_OPT_TYPE);
error_append_hint(&local_err, GERR_INDEX_HINT, i);
goto out;
}
+ gsconf->type = type;
qemu_opts_del(opts);
if (gsconf->type == SOCKET_ADDRESS_FLAT_TYPE_INET) {
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 07/15] qapi-schema: SocketAddressFlat variants 'vsock' and 'fd'
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (5 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 06/15] gluster: Prepare for SocketAddressFlat extension Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 08/15] sockets: New helper socket_address_crumple() Max Reitz
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
Note that the new variants are impossible in qemu_gluster_glfs_init(),
because the gconf->server can only come from qemu_gluster_parse_uri()
or qemu_gluster_parse_json(), and neither can create anything but
'inet' or 'unix'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1490895797-29094-7-git-send-email-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi-schema.json | 19 ++++++++-----------
block/gluster.c | 2 ++
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index b921994ae3..250e4dc49b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4144,7 +4144,7 @@
# Since: 2.9
##
{ 'enum': 'SocketAddressFlatType',
- 'data': [ 'unix', 'inet' ] }
+ 'data': [ 'inet', 'unix', 'vsock', 'fd' ] }
##
# @SocketAddressFlat:
@@ -4153,22 +4153,19 @@
#
# @type: Transport type
#
-# This is similar to SocketAddress, only distinction:
-#
-# 1. SocketAddressFlat is a flat union, SocketAddress is a simple union.
-# A flat union is nicer than simple because it avoids nesting
-# (i.e. more {}) on the wire.
-#
-# 2. SocketAddressFlat supports only types 'unix' and 'inet', because
-# that's what its current users need.
+# This is just like SocketAddress, except it's a flat union rather
+# than a simple union. Nicer because it avoids nesting on the wire,
+# i.e. this form has fewer {}.
#
# Since: 2.9
##
{ 'union': 'SocketAddressFlat',
'base': { 'type': 'SocketAddressFlatType' },
'discriminator': 'type',
- 'data': { 'unix': 'UnixSocketAddress',
- 'inet': 'InetSocketAddress' } }
+ 'data': { 'inet': 'InetSocketAddress',
+ 'unix': 'UnixSocketAddress',
+ 'vsock': 'VsockSocketAddress',
+ 'fd': 'String' } }
##
# @getfd:
diff --git a/block/gluster.c b/block/gluster.c
index fb0aafeaa3..cf29b5f9a4 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -429,6 +429,8 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
server->value->u.inet.host,
(int)port);
break;
+ case SOCKET_ADDRESS_FLAT_TYPE_VSOCK:
+ case SOCKET_ADDRESS_FLAT_TYPE_FD:
default:
abort();
}
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 08/15] sockets: New helper socket_address_crumple()
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (6 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 07/15] qapi-schema: SocketAddressFlat variants 'vsock' and 'fd' Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 09/15] nbd: Tidy up blockdev-add interface Max Reitz
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
SocketAddress is a simple union, and simple unions are awkward: they
have their variant members wrapped in a "data" object on the wire, and
require additional indirections in C. I intend to limit its use to
existing external interfaces. New ones should use SocketAddressFlat.
I further intend to convert all internal interfaces to
SocketAddressFlat. This helper should go away then.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1490895797-29094-8-git-send-email-armbru@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/qemu/sockets.h | 11 +++++++++++
util/qemu-sockets.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 5f1bab9b3e..7842f6d150 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -119,4 +119,15 @@ SocketAddress *socket_remote_address(int fd, Error **errp);
*/
char *socket_address_to_string(struct SocketAddress *addr, Error **errp);
+/**
+ * socket_address_crumple:
+ * @addr_flat: the socket address to crumple
+ *
+ * Convert SocketAddressFlat to SocketAddress. Caller is responsible
+ * for freeing with qapi_free_SocketAddress().
+ *
+ * Returns: the argument converted to SocketAddress.
+ */
+SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat);
+
#endif /* QEMU_SOCKETS_H */
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 4ae37bd2a2..21442c30dc 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -25,6 +25,7 @@
#include "qapi/error.h"
#include "qemu/sockets.h"
#include "qemu/main-loop.h"
+#include "qapi/clone-visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qobject-output-visitor.h"
#include "qapi-visit.h"
@@ -1341,3 +1342,34 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
}
return buf;
}
+
+SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat)
+{
+ SocketAddress *addr = g_new(SocketAddress, 1);
+
+ switch (addr_flat->type) {
+ case SOCKET_ADDRESS_FLAT_TYPE_INET:
+ addr->type = SOCKET_ADDRESS_KIND_INET;
+ addr->u.inet.data = QAPI_CLONE(InetSocketAddress,
+ &addr_flat->u.inet);
+ break;
+ case SOCKET_ADDRESS_FLAT_TYPE_UNIX:
+ addr->type = SOCKET_ADDRESS_KIND_UNIX;
+ addr->u.q_unix.data = QAPI_CLONE(UnixSocketAddress,
+ &addr_flat->u.q_unix);
+ break;
+ case SOCKET_ADDRESS_FLAT_TYPE_VSOCK:
+ addr->type = SOCKET_ADDRESS_KIND_VSOCK;
+ addr->u.vsock.data = QAPI_CLONE(VsockSocketAddress,
+ &addr_flat->u.vsock);
+ break;
+ case SOCKET_ADDRESS_FLAT_TYPE_FD:
+ addr->type = SOCKET_ADDRESS_KIND_FD;
+ addr->u.fd.data = QAPI_CLONE(String, &addr_flat->u.fd);
+ break;
+ default:
+ abort();
+ }
+
+ return addr;
+}
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 09/15] nbd: Tidy up blockdev-add interface
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (7 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 08/15] sockets: New helper socket_address_crumple() Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 10/15] sheepdog: Fix blockdev-add Max Reitz
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
SocketAddress is a simple union, and simple unions are awkward: they
have their variant members wrapped in a "data" object on the wire, and
require additional indirections in C. I intend to limit its use to
existing external interfaces, and convert all internal interfaces to
SocketAddressFlat.
BlockdevOptionsNbd is an external interface using SocketAddress. We
already use SocketAddressFlat elsewhere in blockdev-add. Replace it
by SocketAddressFlat while we can (it's new in 2.9) for simplicity and
consistency. For example,
{ "execute": "blockdev-add",
"arguments": { "node-name": "foo", "driver": "nbd",
"server": { "type": "inet",
"data": { "host": "localhost",
"port": "12345" } } } }
becomes
{ "execute": "blockdev-add",
"arguments": { "node-name": "foo", "driver": "nbd",
"server": { "type": "inet",
"host": "localhost", "port": "12345" } } }
Since the internal interfaces still take SocketAddress, this requires
conversion function socket_address_crumple(). It'll go away when I
update the interfaces.
Unfortunately, SocketAddress is also visible in -drive since 2.8:
-drive if=none,driver=nbd,server.type=inet,server.data.host=127.0.0.1,server.data.port=12345
Nobody should be using it, as it's fairly new and has never been
documented, so adding still more compatibility gunk to keep it working
isn't worth the trouble. You now have to use
-drive if=none,driver=nbd,server.type=inet,server.host=127.0.0.1,server.port=12345
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1490895797-29094-9-git-send-email-armbru@redhat.com
[mreitz: Change iotest 147 accordingly]
Because of this interface change, iotest 147 has to be adapted.
Unfortunately, we cannot just flatten all of the addresses because
nbd-server-start still takes a plain SocketAddress. Therefore, we need
both and this is most easily achieved by writing the SocketAddress into
the code and flattening it where necessary.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170330221243.17333-1-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 2 +-
block/nbd.c | 53 ++++++++++++++++++++++++++------------------------
tests/qemu-iotests/147 | 25 ++++++++++++++++--------
3 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8de39d143a..e4d8a63575 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2847,7 +2847,7 @@
# Since: 2.9
##
{ 'struct': 'BlockdevOptionsNbd',
- 'data': { 'server': 'SocketAddress',
+ 'data': { 'server': 'SocketAddressFlat',
'*export': 'str',
'*tls-creds': 'str' } }
diff --git a/block/nbd.c b/block/nbd.c
index 11e3ba75fa..8bb29a90bb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -47,7 +47,7 @@ typedef struct BDRVNBDState {
NBDClientSession client;
/* For nbd_refresh_filename() */
- SocketAddress *saddr;
+ SocketAddressFlat *saddr;
char *export, *tlscredsid;
} BDRVNBDState;
@@ -95,7 +95,7 @@ static int nbd_parse_uri(const char *filename, QDict *options)
goto out;
}
qdict_put(options, "server.type", qstring_from_str("unix"));
- qdict_put(options, "server.data.path",
+ qdict_put(options, "server.path",
qstring_from_str(qp->p[0].value));
} else {
QString *host;
@@ -116,10 +116,10 @@ static int nbd_parse_uri(const char *filename, QDict *options)
}
qdict_put(options, "server.type", qstring_from_str("inet"));
- qdict_put(options, "server.data.host", host);
+ qdict_put(options, "server.host", host);
port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
- qdict_put(options, "server.data.port", qstring_from_str(port_str));
+ qdict_put(options, "server.port", qstring_from_str(port_str));
g_free(port_str);
}
@@ -197,7 +197,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
/* are we a UNIX or TCP socket? */
if (strstart(host_spec, "unix:", &unixpath)) {
qdict_put(options, "server.type", qstring_from_str("unix"));
- qdict_put(options, "server.data.path", qstring_from_str(unixpath));
+ qdict_put(options, "server.path", qstring_from_str(unixpath));
} else {
InetSocketAddress *addr = NULL;
@@ -207,8 +207,8 @@ static void nbd_parse_filename(const char *filename, QDict *options,
}
qdict_put(options, "server.type", qstring_from_str("inet"));
- qdict_put(options, "server.data.host", qstring_from_str(addr->host));
- qdict_put(options, "server.data.port", qstring_from_str(addr->port));
+ qdict_put(options, "server.host", qstring_from_str(addr->host));
+ qdict_put(options, "server.port", qstring_from_str(addr->port));
qapi_free_InetSocketAddress(addr);
}
@@ -248,20 +248,21 @@ static bool nbd_process_legacy_socket_options(QDict *output_options,
}
qdict_put(output_options, "server.type", qstring_from_str("unix"));
- qdict_put(output_options, "server.data.path", qstring_from_str(path));
+ qdict_put(output_options, "server.path", qstring_from_str(path));
} else if (host) {
qdict_put(output_options, "server.type", qstring_from_str("inet"));
- qdict_put(output_options, "server.data.host", qstring_from_str(host));
- qdict_put(output_options, "server.data.port",
+ qdict_put(output_options, "server.host", qstring_from_str(host));
+ qdict_put(output_options, "server.port",
qstring_from_str(port ?: stringify(NBD_DEFAULT_PORT)));
}
return true;
}
-static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp)
+static SocketAddressFlat *nbd_config(BDRVNBDState *s, QDict *options,
+ Error **errp)
{
- SocketAddress *saddr = NULL;
+ SocketAddressFlat *saddr = NULL;
QDict *addr = NULL;
QObject *crumpled_addr = NULL;
Visitor *iv = NULL;
@@ -287,7 +288,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp)
* visitor expects the former.
*/
iv = qobject_input_visitor_new(crumpled_addr);
- visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
+ visit_type_SocketAddressFlat(iv, NULL, &saddr, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto done;
@@ -306,9 +307,10 @@ NBDClientSession *nbd_get_client_session(BlockDriverState *bs)
return &s->client;
}
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
+static QIOChannelSocket *nbd_establish_connection(SocketAddressFlat *saddr_flat,
Error **errp)
{
+ SocketAddress *saddr = socket_address_crumple(saddr_flat);
QIOChannelSocket *sioc;
Error *local_err = NULL;
@@ -318,6 +320,7 @@ static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
qio_channel_socket_connect_sync(sioc,
saddr,
&local_err);
+ qapi_free_SocketAddress(saddr);
if (local_err) {
error_propagate(errp, local_err);
return NULL;
@@ -409,7 +412,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
goto error;
}
- /* Translate @host, @port, and @path to a SocketAddress */
+ /* Translate @host, @port, and @path to a SocketAddressFlat */
if (!nbd_process_legacy_socket_options(options, opts, errp)) {
goto error;
}
@@ -430,11 +433,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
}
/* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
- if (s->saddr->type != SOCKET_ADDRESS_KIND_INET) {
+ if (s->saddr->type != SOCKET_ADDRESS_FLAT_TYPE_INET) {
error_setg(errp, "TLS only supported over IP sockets");
goto error;
}
- hostname = s->saddr->u.inet.data->host;
+ hostname = s->saddr->u.inet.host;
}
/* establish TCP connection, return error if it fails
@@ -457,7 +460,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
object_unref(OBJECT(tlscreds));
}
if (ret < 0) {
- qapi_free_SocketAddress(s->saddr);
+ qapi_free_SocketAddressFlat(s->saddr);
g_free(s->export);
g_free(s->tlscredsid);
}
@@ -483,7 +486,7 @@ static void nbd_close(BlockDriverState *bs)
nbd_client_close(bs);
- qapi_free_SocketAddress(s->saddr);
+ qapi_free_SocketAddressFlat(s->saddr);
g_free(s->export);
g_free(s->tlscredsid);
}
@@ -514,15 +517,15 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
Visitor *ov;
const char *host = NULL, *port = NULL, *path = NULL;
- if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) {
- const InetSocketAddress *inet = s->saddr->u.inet.data;
+ if (s->saddr->type == SOCKET_ADDRESS_FLAT_TYPE_INET) {
+ const InetSocketAddress *inet = &s->saddr->u.inet;
if (!inet->has_ipv4 && !inet->has_ipv6 && !inet->has_to) {
host = inet->host;
port = inet->port;
}
- } else if (s->saddr->type == SOCKET_ADDRESS_KIND_UNIX) {
- path = s->saddr->u.q_unix.data->path;
- }
+ } else if (s->saddr->type == SOCKET_ADDRESS_FLAT_TYPE_UNIX) {
+ path = s->saddr->u.q_unix.path;
+ } /* else can't represent as pseudo-filename */
qdict_put(opts, "driver", qstring_from_str("nbd"));
@@ -541,7 +544,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
}
ov = qobject_output_visitor_new(&saddr_qdict);
- visit_type_SocketAddress(ov, NULL, &s->saddr, &error_abort);
+ visit_type_SocketAddressFlat(ov, NULL, &s->saddr, &error_abort);
visit_complete(ov, &saddr_qdict);
visit_free(ov);
qdict_put_obj(opts, "server", saddr_qdict);
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index cca75c562c..32afea63d4 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -30,6 +30,13 @@ NBD_PORT = 10811
test_img = os.path.join(iotests.test_dir, 'test.img')
unix_socket = os.path.join(iotests.test_dir, 'nbd.socket')
+
+def flatten_sock_addr(crumpled_address):
+ result = { 'type': crumpled_address['type'] }
+ result.update(crumpled_address['data'])
+ return result
+
+
class NBDBlockdevAddBase(iotests.QMPTestCase):
def blockdev_add_options(self, address, export=None):
options = { 'node-name': 'nbd-blockdev',
@@ -85,13 +92,15 @@ class QemuNBD(NBDBlockdevAddBase):
'host': 'localhost',
'port': str(NBD_PORT)
} }
- self.client_test('nbd://localhost:%i' % NBD_PORT, address)
+ self.client_test('nbd://localhost:%i' % NBD_PORT,
+ flatten_sock_addr(address))
def test_unix(self):
self._server_up('-k', unix_socket)
address = { 'type': 'unix',
'data': { 'path': unix_socket } }
- self.client_test('nbd+unix://?socket=' + unix_socket, address)
+ self.client_test('nbd+unix://?socket=' + unix_socket,
+ flatten_sock_addr(address))
class BuiltinNBD(NBDBlockdevAddBase):
@@ -134,7 +143,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
} }
self._server_up(address)
self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT,
- address, 'nbd-export')
+ flatten_sock_addr(address), 'nbd-export')
self._server_down()
def test_inet6(self):
@@ -149,10 +158,10 @@ class BuiltinNBD(NBDBlockdevAddBase):
'file': {
'driver': 'nbd',
'export': 'nbd-export',
- 'server': address
+ 'server': flatten_sock_addr(address)
} }
self._server_up(address)
- self.client_test(filename, address, 'nbd-export')
+ self.client_test(filename, flatten_sock_addr(address), 'nbd-export')
self._server_down()
def test_unix(self):
@@ -160,7 +169,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
'data': { 'path': unix_socket } }
self._server_up(address)
self.client_test('nbd+unix:///nbd-export?socket=' + unix_socket,
- address, 'nbd-export')
+ flatten_sock_addr(address), 'nbd-export')
self._server_down()
def test_fd(self):
@@ -182,9 +191,9 @@ class BuiltinNBD(NBDBlockdevAddBase):
'file': {
'driver': 'nbd',
'export': 'nbd-export',
- 'server': address
+ 'server': flatten_sock_addr(address)
} }
- self.client_test(filename, address, 'nbd-export')
+ self.client_test(filename, flatten_sock_addr(address), 'nbd-export')
self._server_down()
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 10/15] sheepdog: Fix blockdev-add
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (8 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 09/15] nbd: Tidy up blockdev-add interface Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 11/15] qemu-io-cmds: Assert that global and nofile commands don't use ct->perms Max Reitz
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Markus Armbruster <armbru@redhat.com>
Commit 831acdc "sheepdog: Implement bdrv_parse_filename()" and commit
d282f34 "sheepdog: Support blockdev-add" have different ideas on how
the QemuOpts parameters for the server address are named. Fix that.
While there, rename BlockdevOptionsSheepdog member addr to server, for
consistency with BlockdevOptionsSsh, BlockdevOptionsGluster,
BlockdevOptionsNbd.
Commit 831acdc's example becomes
--drive driver=sheepdog,server.type=inet,server.host=fido,server.port=7000,vdi=dolly
instead of
--drive driver=sheepdog,host=fido,vdi=dolly
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Message-id: 1490895797-29094-10-git-send-email-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 4 +--
block/sheepdog.c | 90 +++++++++++++++++++++++++++++++++-------------------
2 files changed, 60 insertions(+), 34 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e4d8a63575..033457ce86 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2623,7 +2623,7 @@
# Driver specific block device options for sheepdog
#
# @vdi: Virtual disk image name
-# @addr: The Sheepdog server to connect to
+# @server: The Sheepdog server to connect to
# @snap-id: Snapshot ID
# @tag: Snapshot tag name
#
@@ -2632,7 +2632,7 @@
# Since: 2.9
##
{ 'struct': 'BlockdevOptionsSheepdog',
- 'data': { 'addr': 'SocketAddressFlat',
+ 'data': { 'server': 'SocketAddressFlat',
'vdi': 'str',
'*snap-id': 'uint32',
'*tag': 'str' } }
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 89e98edab6..1b71fc81ec 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -13,9 +13,11 @@
*/
#include "qemu/osdep.h"
+#include "qapi-visit.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qint.h"
+#include "qapi/qobject-input-visitor.h"
#include "qemu/uri.h"
#include "qemu/error-report.h"
#include "qemu/sockets.h"
@@ -547,6 +549,47 @@ static SocketAddress *sd_socket_address(const char *path,
return addr;
}
+static SocketAddress *sd_server_config(QDict *options, Error **errp)
+{
+ QDict *server = NULL;
+ QObject *crumpled_server = NULL;
+ Visitor *iv = NULL;
+ SocketAddressFlat *saddr_flat = NULL;
+ SocketAddress *saddr = NULL;
+ Error *local_err = NULL;
+
+ qdict_extract_subqdict(options, &server, "server.");
+
+ crumpled_server = qdict_crumple(server, errp);
+ if (!crumpled_server) {
+ goto done;
+ }
+
+ /*
+ * FIXME .numeric, .to, .ipv4 or .ipv6 don't work with -drive
+ * server.type=inet. .to doesn't matter, it's ignored anyway.
+ * That's because when @options come from -blockdev or
+ * blockdev_add, members are typed according to the QAPI schema,
+ * but when they come from -drive, they're all QString. The
+ * visitor expects the former.
+ */
+ iv = qobject_input_visitor_new(crumpled_server);
+ visit_type_SocketAddressFlat(iv, NULL, &saddr_flat, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto done;
+ }
+
+ saddr = socket_address_crumple(saddr_flat);
+
+done:
+ qapi_free_SocketAddressFlat(saddr_flat);
+ visit_free(iv);
+ qobject_decref(crumpled_server);
+ QDECREF(server);
+ return saddr;
+}
+
/* Return -EIO in case of error, file descriptor on success */
static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)
{
@@ -1174,15 +1217,15 @@ static void sd_parse_filename(const char *filename, QDict *options,
return;
}
- if (cfg.host) {
- qdict_set_default_str(options, "host", cfg.host);
- }
- if (cfg.port) {
- snprintf(buf, sizeof(buf), "%d", cfg.port);
- qdict_set_default_str(options, "port", buf);
- }
if (cfg.path) {
- qdict_set_default_str(options, "path", cfg.path);
+ qdict_set_default_str(options, "server.path", cfg.path);
+ qdict_set_default_str(options, "server.type", "unix");
+ } else {
+ qdict_set_default_str(options, "server.type", "inet");
+ qdict_set_default_str(options, "server.host",
+ cfg.host ?: SD_DEFAULT_ADDR);
+ snprintf(buf, sizeof(buf), "%d", cfg.port ?: SD_DEFAULT_PORT);
+ qdict_set_default_str(options, "server.port", buf);
}
qdict_set_default_str(options, "vdi", cfg.vdi);
qdict_set_default_str(options, "tag", cfg.tag);
@@ -1510,18 +1553,6 @@ static QemuOptsList runtime_opts = {
.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
.desc = {
{
- .name = "host",
- .type = QEMU_OPT_STRING,
- },
- {
- .name = "port",
- .type = QEMU_OPT_STRING,
- },
- {
- .name = "path",
- .type = QEMU_OPT_STRING,
- },
- {
.name = "vdi",
.type = QEMU_OPT_STRING,
},
@@ -1543,7 +1574,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
int ret, fd;
uint32_t vid = 0;
BDRVSheepdogState *s = bs->opaque;
- const char *host, *port, *path, *vdi, *snap_id_str, *tag;
+ const char *vdi, *snap_id_str, *tag;
uint64_t snap_id;
char *buf = NULL;
QemuOpts *opts;
@@ -1560,20 +1591,17 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
goto err_no_fd;
}
- host = qemu_opt_get(opts, "host");
- port = qemu_opt_get(opts, "port");
- path = qemu_opt_get(opts, "path");
+ s->addr = sd_server_config(options, errp);
+ if (!s->addr) {
+ ret = -EINVAL;
+ goto err_no_fd;
+ }
+
vdi = qemu_opt_get(opts, "vdi");
snap_id_str = qemu_opt_get(opts, "snap-id");
snap_id = qemu_opt_get_number(opts, "snap-id", CURRENT_VDI_ID);
tag = qemu_opt_get(opts, "tag");
- if ((host || port) && path) {
- error_setg(errp, "can't use 'path' together with 'host' or 'port'");
- ret = -EINVAL;
- goto err_no_fd;
- }
-
if (!vdi) {
error_setg(errp, "parameter 'vdi' is missing");
ret = -EINVAL;
@@ -1604,8 +1632,6 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
goto err_no_fd;
}
- s->addr = sd_socket_address(path, host, port);
-
QLIST_INIT(&s->inflight_aio_head);
QLIST_INIT(&s->failed_aio_head);
QLIST_INIT(&s->inflight_aiocb_head);
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 11/15] qemu-io-cmds: Assert that global and nofile commands don't use ct->perms
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (9 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 10/15] sheepdog: Fix blockdev-add Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 12/15] iotests: fix 097 when run with qcow Max Reitz
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Peter Maydell <peter.maydell@linaro.org>
It would be a bug for a command with the CMD_NOFILE_OK or
CMD_FLAG_GLOBAL flags set to also set the ct->perms field,
because the former says "OK for a file not to be open"
but the latter is a check on a file.
Add an assertion in qemuio_add_command() so we can catch that
sort of buggy command definition immediately rather than it
being a bug that only manifests when a particular set of
command line options is used.
(Coverity gets confused about this (CID 1371723) and reports
that we might dereference a NULL blk pointer in this case,
because it can't tell that that code path never happens with
the cmdinfo_t that we have. This commit won't help unconfuse
it, but it does fix the underlying issue.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1490967529-4767-1-git-send-email-peter.maydell@linaro.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io-cmds.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 2c48f9ce1a..883f53b64d 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -35,6 +35,13 @@ static int compare_cmdname(const void *a, const void *b)
void qemuio_add_command(const cmdinfo_t *ci)
{
+ /* ci->perm assumes a file is open, but the GLOBAL and NOFILE_OK
+ * flags allow it not to be, so that combination is invalid.
+ * Catch it now rather than letting it manifest as a crash if a
+ * particular set of command line options are used.
+ */
+ assert(ci->perm == 0 ||
+ (ci->flags & (CMD_FLAG_GLOBAL | CMD_NOFILE_OK)) == 0);
cmdtab = g_renew(cmdinfo_t, cmdtab, ++ncmds);
cmdtab[ncmds - 1] = *ci;
qsort(cmdtab, ncmds, sizeof(*cmdtab), compare_cmdname);
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 12/15] iotests: fix 097 when run with qcow
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (10 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 11/15] qemu-io-cmds: Assert that global and nofile commands don't use ct->perms Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 13/15] qcow2: Discard unaligned tail when wiping image Max Reitz
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: "Daniel P. Berrange" <berrange@redhat.com>
The previous commit:
commit a3e1505daec31ef56f0489f8c8fff1b8e4ca92bd
Author: Eric Blake <eblake@redhat.com>
Date: Mon Dec 5 09:49:34 2016 -0600
qcow2: Don't strand clusters near 2G intervals during commit
extended the 097 test case so that it did two passes, once
with an internal snapshot, once without.
qcow (v1) does not support internal snapshots, so this change
broke test 097 when run against qcow.
This splits 097 in two, creating a new 176 that tests the
internal snapshot codepath, effectively putting 097 back
to its content before the above commit.
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20170221115512.21918-8-berrange@redhat.com>
[eblake: test collisions: s/173/176/g]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170331185356.2479-2-eblake@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/097 | 10 +---
tests/qemu-iotests/097.out | 125 ++------------------------------------------
tests/qemu-iotests/176 | 126 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/176.out | 119 ++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
5 files changed, 251 insertions(+), 130 deletions(-)
create mode 100755 tests/qemu-iotests/176
create mode 100644 tests/qemu-iotests/176.out
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 4c33e8038a..1d28aff99d 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -56,26 +56,19 @@ _supported_os Linux
# 3: Two-layer backing chain, commit to lower backing file
# (in this case, the top image will implicitly stay unchanged)
#
-# Each pass is run twice, since qcow2 has different code paths for cleaning
-# an image depending on whether it has a snapshot.
-#
# 020 already tests committing, so this only tests whether image chains are
# working properly and that all images above the base are emptied; therefore,
# no complicated patterns are necessary. Check near the 2G mark, as qcow2
# has been buggy at that boundary in the past.
for i in 0 1 2 3; do
-for j in 0 1; do
echo
-echo "=== Test pass $i.$j ==="
+echo "=== Test pass $i ==="
echo
TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
_make_test_img -b "$TEST_IMG.itmd" 2100M
-if [ $j -eq 0 ]; then
- $QEMU_IMG snapshot -c snap "$TEST_IMG"
-fi
$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
@@ -121,7 +114,6 @@ $QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
done
-done
# success, all done
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
index 8106cc9275..81fc2252a0 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -1,6 +1,6 @@
QA output created by 097
-=== Test pass 0.0 ===
+=== Test pass 0 ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
@@ -29,66 +29,7 @@ Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-=== Test pass 0.1 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Image committed.
-read 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147287040
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147352576
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-
-=== Test pass 1.0 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Image committed.
-read 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147287040
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147352576
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-
-=== Test pass 1.1 ===
+=== Test pass 1 ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
@@ -118,7 +59,7 @@ Offset Length File
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-=== Test pass 2.0 ===
+=== Test pass 2 ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
@@ -148,65 +89,7 @@ Offset Length File
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-=== Test pass 2.1 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Image committed.
-read 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147287040
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147352576
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-
-=== Test pass 3.0 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Image committed.
-read 65536/65536 bytes at offset 2147287040
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147352576
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset Length File
-0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-Offset Length File
-0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
-0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-
-=== Test pass 3.1 ===
+=== Test pass 3 ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
new file mode 100755
index 0000000000..9a70a8707c
--- /dev/null
+++ b/tests/qemu-iotests/176
@@ -0,0 +1,126 @@
+#!/bin/bash
+#
+# Commit changes into backing chains and empty the top image if the
+# backing image is not explicitly specified.
+#
+# Variant of 097, which includes snapshots to test different codepath
+# in qcow2
+#
+# Copyright (C) 2014 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/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ _rm_test_img "$TEST_IMG.itmd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Any format supporting backing files and bdrv_make_empty
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+
+# Four passes:
+# 0: Two-layer backing chain, commit to upper backing file (implicitly)
+# (in this case, the top image will be emptied)
+# 1: Two-layer backing chain, commit to upper backing file (explicitly)
+# (in this case, the top image will implicitly stay unchanged)
+# 2: Two-layer backing chain, commit to upper backing file (implicitly with -d)
+# (in this case, the top image will explicitly stay unchanged)
+# 3: Two-layer backing chain, commit to lower backing file
+# (in this case, the top image will implicitly stay unchanged)
+#
+# 020 already tests committing, so this only tests whether image chains are
+# working properly and that all images above the base are emptied; therefore,
+# no complicated patterns are necessary. Check near the 2G mark, as qcow2
+# has been buggy at that boundary in the past.
+for i in 0 1 2 3; do
+
+echo
+echo "=== Test pass $i ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
+_make_test_img -b "$TEST_IMG.itmd" 2100M
+$QEMU_IMG snapshot -c snap "$TEST_IMG"
+
+$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'write -P 3 0x7fff0000 64k' "$TEST_IMG" | _filter_qemu_io
+
+if [ $i -lt 3 ]; then
+ if [ $i == 0 ]; then
+ # -b "$TEST_IMG.itmd" should be the default (that is, committing to the
+ # first backing file in the chain)
+ $QEMU_IMG commit "$TEST_IMG"
+ elif [ $i == 1 ]; then
+ # explicitly specify the commit target (this should imply -d)
+ $QEMU_IMG commit -b "$TEST_IMG.itmd" "$TEST_IMG"
+ else
+ # do not explicitly specify the commit target, but use -d to leave the
+ # top image unchanged
+ $QEMU_IMG commit -d "$TEST_IMG"
+ fi
+
+ # Bottom should be unchanged
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+
+ # Intermediate should contain changes from top
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+
+ # And in pass 0, the top image should be empty, whereas in both other passes
+ # it should be unchanged (which is both checked by qemu-img map)
+else
+ $QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"
+
+ # Bottom should contain all changes
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+
+ # Both top and intermediate should be unchanged
+fi
+
+$QEMU_IMG map "$TEST_IMG.base" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
+
+done
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
new file mode 100644
index 0000000000..fc6365309e
--- /dev/null
+++ b/tests/qemu-iotests/176.out
@@ -0,0 +1,119 @@
+QA output created by 176
+
+=== Test pass 0 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+
+=== Test pass 1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 2 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 3 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1f4bf03185..43142ddfcf 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -168,3 +168,4 @@
173 rw auto
174 auto
175 auto quick
+176 rw auto backing
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 13/15] qcow2: Discard unaligned tail when wiping image
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (11 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 12/15] iotests: fix 097 when run with qcow Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 14/15] iotests: Improve image-clear tests on non-aligned image Max Reitz
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Eric Blake <eblake@redhat.com>
There is a subtle difference between the fast (qcow2v3 with no
extra data) and slow path (qcow2v2 format [aka 0.10], or when a
snapshot is present) of qcow2_make_empty(). The slow path fails
to discard the final (partial) cluster of an unaligned image.
The problem stems from the fact that qcow2_discard_clusters() was
silently ignoring sub-cluster head and tail on unaligned requests.
A quick audit of all callers shows that qcow2_snapshot_create() has
always passed a cluster-aligned request since the call was added
in commit 1ebf561; qcow2_co_pdiscard() has passed a cluster-aligned
request since commit ecdbead taught the block layer about preferred
discard alignment; and qcow2_make_empty() was fixed to pass an
aligned start (but not necessarily end) in commit a3e1505.
Asserting that the start is always aligned also points out that we
now have a dead check: rounding the end offset down can never result
in a value less than the aligned start offset (the check was rendered
dead with commit ecdbead). Meanwhile, we do not want to round the
end cluster down in the one case of the end offset matching the
(unaligned) file size - that final partial cluster should still be
discarded.
With those fixes in place, the fast and slow paths are back in sync
at discarding an entire image; the next patch will update
qemu-iotests to ensure we don't regress.
Note that bdrv_co_pdiscard ignores ALL partial cluster requests,
including the partial cluster at the end of an image; it can be
argued that the partial cluster at the end should be special-cased
so that a guest issuing discard requests at proper alignments
everywhere else can likewise empty the entire image. But that
optimization is left for another day.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170331185356.2479-3-eblake@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-cluster.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 78c11d4948..100398c565 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1519,12 +1519,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
- /* Round start up and end down */
- offset = align_offset(offset, s->cluster_size);
- end_offset = start_of_cluster(s, end_offset);
-
- if (offset > end_offset) {
- return 0;
+ /* The caller must cluster-align start; round end down except at EOF */
+ assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+ if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
+ end_offset = start_of_cluster(s, end_offset);
}
nb_clusters = size_to_clusters(s, end_offset - offset);
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 14/15] iotests: Improve image-clear tests on non-aligned image
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (12 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 13/15] qcow2: Discard unaligned tail when wiping image Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 15:33 ` [Qemu-devel] [PULL 15/15] block/parallels: Avoid overflows Max Reitz
2017-04-03 16:48 ` [Qemu-devel] [PULL 00/15] Block patches for rc3 Peter Maydell
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
From: Eric Blake <eblake@redhat.com>
Tweak 097 and 176 to operate on an image that is not cluster-aligned,
to give further coverage of clearing out an entire image, including
the recent fix to eliminate the difference between fast path (97) and
slow (176) for qcow2. Also tested on qcow (97 only, since qcow lacks
snapshots).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170331185356.2479-4-eblake@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/097 | 17 +++++++++-----
tests/qemu-iotests/097.out | 55 ++++++++++++++++++++++++++++++++++++----------
tests/qemu-iotests/176 | 17 +++++++++-----
tests/qemu-iotests/176.out | 55 ++++++++++++++++++++++++++++++++++++----------
4 files changed, 108 insertions(+), 36 deletions(-)
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 1d28aff99d..e22670c8d0 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -66,13 +66,15 @@ echo
echo "=== Test pass $i ==="
echo
-TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
-TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
-_make_test_img -b "$TEST_IMG.itmd" 2100M
+len=$((2100 * 1024 * 1024 + 512)) # larger than 2G, and not cluster aligned
+TEST_IMG="$TEST_IMG.base" _make_test_img $len
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" $len
+_make_test_img -b "$TEST_IMG.itmd" $len
-$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'write -P 3 0x7fff0000 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -P 1 0x7ffd0000 192k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -P 2 0x7ffe0000 128k" "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c "write -P 3 0x7fff0000 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -P 4 $(($len - 512)) 512" "$TEST_IMG" | _filter_qemu_io
if [ $i -lt 3 ]; then
if [ $i == 0 ]; then
@@ -90,11 +92,13 @@ if [ $i -lt 3 ]; then
# Bottom should be unchanged
$QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 0 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Intermediate should contain changes from top
$QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
$QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
$QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.itmd" | _filter_qemu_io
# And in pass 0, the top image should be empty, whereas in both other passes
# it should be unchanged (which is both checked by qemu-img map)
@@ -105,6 +109,7 @@ else
$QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Both top and intermediate should be unchanged
fi
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
index 81fc2252a0..f6705a1cc7 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -2,104 +2,130 @@ QA output created by 097
=== Test pass 0 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
=== Test pass 1 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 2 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 3 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -107,13 +133,18 @@ read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
*** done
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
index 9a70a8707c..950b28720e 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -69,14 +69,16 @@ echo
echo "=== Test pass $i ==="
echo
-TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
-TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
-_make_test_img -b "$TEST_IMG.itmd" 2100M
+len=$((2100 * 1024 * 1024 + 512)) # larger than 2G, and not cluster aligned
+TEST_IMG="$TEST_IMG.base" _make_test_img $len
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" $len
+_make_test_img -b "$TEST_IMG.itmd" $len
$QEMU_IMG snapshot -c snap "$TEST_IMG"
-$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'write -P 3 0x7fff0000 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -P 1 0x7ffd0000 192k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -P 2 0x7ffe0000 128k" "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c "write -P 3 0x7fff0000 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -P 4 $(($len - 512)) 512" "$TEST_IMG" | _filter_qemu_io
if [ $i -lt 3 ]; then
if [ $i == 0 ]; then
@@ -94,11 +96,13 @@ if [ $i -lt 3 ]; then
# Bottom should be unchanged
$QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 0 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Intermediate should contain changes from top
$QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
$QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
$QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.itmd" | _filter_qemu_io
# And in pass 0, the top image should be empty, whereas in both other passes
# it should be unchanged (which is both checked by qemu-img map)
@@ -109,6 +113,7 @@ else
$QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Both top and intermediate should be unchanged
fi
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index fc6365309e..6271fa7d6f 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -2,104 +2,130 @@ QA output created by 176
=== Test pass 0 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
=== Test pass 1 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 2 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 3 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.itmd
wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -107,13 +133,18 @@ read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
*** done
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PULL 15/15] block/parallels: Avoid overflows
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (13 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 14/15] iotests: Improve image-clear tests on non-aligned image Max Reitz
@ 2017-04-03 15:33 ` Max Reitz
2017-04-03 16:48 ` [Qemu-devel] [PULL 00/15] Block patches for rc3 Peter Maydell
15 siblings, 0 replies; 17+ messages in thread
From: Max Reitz @ 2017-04-03 15:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Peter Maydell
Change the types of variables in allocate_clusters() to int64_t so we do
not have to worry about potential overflows.
Add an assertion that our accesses to s->bat[] do not result in a buffer
overflow and that the implicit conversion performed when invoking
bat_entry_off() does not result in an integer overflow.
Coverity-id: 1307776
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170331170512.10381-1-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/parallels.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 4173b3fb9d..90acf79687 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -192,8 +192,7 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, int *pnum)
{
BDRVParallelsState *s = bs->opaque;
- uint32_t idx, to_allocate, i;
- int64_t pos, space;
+ int64_t pos, space, idx, to_allocate, i;
pos = block_status(s, sector_num, nb_sectors, pnum);
if (pos > 0) {
@@ -201,11 +200,19 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
}
idx = sector_num / s->tracks;
- if (idx >= s->bat_size) {
- return -EINVAL;
- }
-
to_allocate = DIV_ROUND_UP(sector_num + *pnum, s->tracks) - idx;
+
+ /* This function is called only by parallels_co_writev(), which will never
+ * pass a sector_num at or beyond the end of the image (because the block
+ * layer never passes such a sector_num to that function). Therefore, idx
+ * is always below s->bat_size.
+ * block_status() will limit *pnum so that sector_num + *pnum will not
+ * exceed the image end. Therefore, idx + to_allocate cannot exceed
+ * s->bat_size.
+ * Note that s->bat_size is an unsigned int, therefore idx + to_allocate
+ * will always fit into a uint32_t. */
+ assert(idx < s->bat_size && idx + to_allocate <= s->bat_size);
+
space = to_allocate * s->tracks;
if (s->data_end + space > bdrv_getlength(bs->file->bs) >> BDRV_SECTOR_BITS) {
int ret;
--
2.12.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PULL 00/15] Block patches for rc3
2017-04-03 15:33 [Qemu-devel] [PULL 00/15] Block patches for rc3 Max Reitz
` (14 preceding siblings ...)
2017-04-03 15:33 ` [Qemu-devel] [PULL 15/15] block/parallels: Avoid overflows Max Reitz
@ 2017-04-03 16:48 ` Peter Maydell
15 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2017-04-03 16:48 UTC (permalink / raw)
To: Max Reitz; +Cc: Qemu-block, QEMU Developers
On 3 April 2017 at 16:33, Max Reitz <mreitz@redhat.com> wrote:
> The following changes since commit 102a3d847891f48c0b13cace19d189217b05a725:
>
> usb-host: switch to LIBUSB_API_VERSION (2017-04-03 14:41:23 +0100)
>
> are available in the git repository at:
>
> git://github.com/XanClic/qemu.git tags/pull-block-2017-04-03
>
> for you to fetch changes up to 86d1bd70987cd11d85d01f52aa5824c63d910471:
>
> block/parallels: Avoid overflows (2017-04-03 17:11:40 +0200)
>
> ----------------------------------------------------------------
> Block patches for 2.9-rc3
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 17+ messages in thread