[adding some qemu visibility]
On 12/5/19 7:34 AM, Peter Krempa wrote:
+ if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps)))
+ return -1;
+
+ if (qemuMonitorTransactionBitmapAdd(actions,
+ dd->domdisk->src->nodeformat,
+ dd->incrementalBitmap,
+ false,
+ true) < 0)
+ return -1;
+
+ if (qemuMonitorTransactionBitmapMerge(actions,
+ dd->domdisk->src->nodeformat,
+ dd->incrementalBitmap,
+ &mergebitmaps) < 0)
+ return -1;
+
+ if (qemuMonitorTransactionBitmapAdd(actions,
+ dd->store->nodeformat,
+ dd->incrementalBitmap,
+ false,
+ true) < 0)
+ return -1;
Why do we need two of these calls?
/me reads on
+
+ if (qemuMonitorTransactionBitmapMerge(actions,
+ dd->store->nodeformat,
+ dd->incrementalBitmap,
+ &mergebitmapsstore) < 0)
+ return -1;
okay, it looks like you are trying to merge the same bitmap into two
different merge commands, which will all be part of the same transaction. I
guess it would be helpful to see a trace of the transaction in action to see
if we can simplify it (using 2 instead of 4 qemuMonitor* commands).
This is required because the backup blockjob requires the bitmap to be
present on the source ('device') image of the backup. The same also
applies for the image exported by NBD. The catch is that we expose the
scratch file via NBD which is actually the target image of the backup.
This means that in case of a pull backup we need two instances of the
bitmap so both the block job and the NBD server can use them. Arguably
there's a possible simplification here for push-mode backups where the
target bitmap doesn't make sense.
The backup job requires the bitmap to be on the source, but the qemu NBD
export code only requires the bitmap to be locatable somewhere on the
qemu NBD server requires the bitmap to be discoverable from anywhere on
the chain, and since the temporary target of the block job has the
source image as its backing file, that should be the case. That is:
base <- active <- temp
|
bitmap0
looking up [active, bitmap0] or [temp, bitmap0] should both succeed; we
need the former for the blockjob, and the latter for the NBD export.
If the NBD export _can't_ find bitmap0 through the backing chain, that
may be a symptom of the problem that Max has been trying to fix (his
upcoming v7 "deal with filters" is hinted at here, but will not be in 4.2):
https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg04520.html
In my original implementation, I did not need a duplicated bitmap in the
temp file. But that was pre-blockdev. Are we really hitting filter
limitations in qemu where the use of blockdev is preventing [temp,
bitmap0] from finding the bitmap across the backing chain? If so, we
should fix that in qemu - but we're so late for 4.2, that I guess
libvirt will have to work around it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
