qemu-block
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]