[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
qemu implementation details leaks to NBD client
From: |
Nir Soffer |
Subject: |
qemu implementation details leaks to NBD client |
Date: |
Thu, 19 Dec 2019 13:36:30 +0200 |
When connecting to qemu NBD server during incremental backup, client needs to
register this meta context:
"qemu:dirty-bitmap:backup-sda"
To get dirty bitmap chunks in NBD_CMD_BLOCK_STATUS for export "sda".
This comes from libvirt code creating a backup node named "backup-sda"
for drive "sda",
and creating a temporary dirty bitmap with the same name, which is reasonable.
All clients using this API need to have code like:
# Libvirt uses this name for the dirty bitmap.
dirty_bitmap = "backup-" + export_name
Duplicating the magic libvirt code:
if (incremental) {
dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst);
We have only one client now[1] so this is not a huge issue, but it is
also easy to fix.
I think what we would like to have instead is:
"qemu:dirty-bitmap:sda"
So clients connecting to NBD server with exportname=sda would find the
dirty bitmap
for this export at the expected name, without the need to depend on
the internal node
name.
It looks like a but in qemu, since:
# @name: Export name. If unspecified, the @device parameter is used as the
# export name. (Since 2.12)
If export name is "sda"...
#
# @writable: Whether clients should be able to write to the device via the
# NBD connection (default false).
# @bitmap: Also export the dirty bitmap reachable from @device, so the
# NBD client can use NBD_OPT_SET_META_CONTEXT with
# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
This API expect that the user can access it at:
"qemu:dirty-bitmap:sda"
#
# Returns: error if the server is not running, or export with the same name
# already exists.
#
# Since: 1.3.0
##
{ 'command': 'nbd-server-add',
'data': {'device': 'str', '*name': 'str', '*writable': 'bool',
'*bitmap': 'str' } }
But when using this API like this:
{"device": "backup-sda", name="sda", bitmap="backup-sda"}
The bitmap is accessible at "backup-sda" instead of "sda".
Client can deal with this by quering the available bitmaps at
"qemu:dirty-bitmap:*"
but what should a client do if the server reports multiple bitmaps?
It seems that this untested change will fix the issue:
diff --git a/nbd/server.c b/nbd/server.c
index d8d1e62455..27aff0c28e 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1560,7 +1560,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
uint64_t dev_offset,
bdrv_dirty_bitmap_set_busy(bm, true);
exp->export_bitmap = bm;
exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
- bitmap);
+ name);
}
exp->close = close;
What do you think?
[1] https://gerrit.ovirt.org/#/c/105838/1/common/ovirt_imageio_common/nbd.py
Nir
- qemu implementation details leaks to NBD client,
Nir Soffer <=