On 5/14/20 1:21 AM, Vladimir Sementsov-Ogievskiy wrote:
13.05.2020 04:16, Eric Blake wrote:
Upcoming patches want to add some basic bitmap manipulation abilities
to qemu-img. But blockdev.o is too heavyweight to link into qemu-img
(among other things, it would drag in block jobs and transaction
support - qemu-img does offline manipulation, where atomicity is less
important because there are no concurrent modifications to compete
with), so it's time to split off the bare bones of what we will need
into a new file block/monitor/bitmap-qmp-cmds.o.
This is sufficient to expose 6 QMP commands for use by qemu-img (add,
remove, clear, enable, disable, merge), as well as move the three
helper functions touched in the previous patch. Regarding
MAINTAINERS, the new file is automatically part of block core, but
also makes sense as related to other dirty bitmap files.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Max Reitz <mre...@redhat.com>
---
+++ b/block/monitor/bitmap-qmp-cmds.c
Hmm, shouldn't transaction bitmap actions be moved here too? May be, not
in these series..
No, the very reason that we split this file off of blockdev.c is that
transactions are NOT needed in qemu-img. You have to have separate .o
files for what qemu-img needs, vs. what the rest of qemu proper needs,
and transactions are only needed in qemu proper at the moment. If we
ever need transactions in Kevin's qemu-storage-daemon but not all of
blockdev.c, then we'd need yet another .c file independent from either
blockdev.c or this patch's new bitmap-qmp-cmds.c.
@@ -0,0 +1,323 @@
+/*
+ * QEMU host block device bitmaps
A bit conflicts with tha fact that they are not of block-device level
but of node-level.
May be just "Block dirty bitmap qmp commands" ?
Copy-and-paste from blockdev.c, tweaked by adding one word. Your
wording is also fine.
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
Does it really apply here? block-dirty-bitmap-add was added in 2015..
May be Red Hat and Virtuozzo copyrights would be more appropriate.
When splitting a file, the safest course of action is to preserve ALL
copyright from the original file into the new file.
Adding additional copyright lines is okay as part of submitting new
functionality, but in this case, I don't feel comfortable adding Red Hat
copyright (my split isn't adding new functionality), and I don't have
authorization to add Virtuozzo copyright (as that is not my employer).
+#include "qemu/osdep.h"
+
+#include "sysemu/blockdev.h"
+#include "block/block.h"
+#include "block/block_int.h"
+#include "qapi/qapi-commands-block.h"
+#include "qapi/error.h"
compiles for with only four:
#include "qemu/osdep.h"
#include "block/block_int.h"
#include "qapi/qapi-commands-block.h"
#include "qapi/error.h"
Thanks. I blame rebase; an earlier version used a different header in
patch 4/9; when I moved things to block_int.h in that patch, I didn't
realize that this patch could be improved.
with at least extra includes dropped:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org