On Wed, Apr 17, 2019 at 09:09:15 -0500, Eric Blake wrote:
> Add some monitor commands to be used during backup/checkpoint
> operations:
> - another facet to query-block for learning bitmap size
> - new export and bitmap parameters to nbd-server-add
> - new block-dirty-bitmap-{add,enable,disable,merge} functions
>
> Also add two capabilities for testing that they are supported;
> using block-dirty-bitmap-merge as the generic witness of checkpoint
> support (since all of the functionalities were added in the same
> qemu 4.0 release), and the bitmap parameter to nbd-server-add for
> pull-mode backup support. Even though both capabilities are
> likely to be present or absent together (that is, it is unlikely
> to encounter a qemu that backports only one of the two), it still
> makes sense to keep two capabilities as the two uses are
> orthogonal (full backups don't require checkpoints, push mode
> backups don't require NBD bitmap support, and checkpoints can
> be used for more than just incremental backups).
I'd suggest you split the capabilities, new QMP APIs, changes to
existing QMP apis and block job handling changes each into a separate
commit.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/qemu/qemu_capabilities.h | 2 +
> src/qemu/qemu_monitor.h | 21 +-
> src/qemu/qemu_monitor_json.h | 19 +-
> src/qemu/qemu_capabilities.c | 4 +
> src/qemu/qemu_migration.c | 2 +-
> src/qemu/qemu_monitor.c | 65 +++++-
> src/qemu/qemu_monitor_json.c | 199 +++++++++++++++++-
> .../caps_4.0.0.riscv32.xml | 2 +
> .../caps_4.0.0.riscv64.xml | 2 +
> .../caps_4.0.0.x86_64.xml | 2 +
> tests/qemumonitorjsontest.c | 2 +-
> 11 files changed, 312 insertions(+), 8 deletions(-)
[...]
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 1237c14a84..aaf8f1df26 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
[...]
> @@ -646,6 +650,19 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon,
> unsigned long long newmem);
> int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, bool online);
>
> +int qemuMonitorAddBitmap(qemuMonitorPtr mon, const char *node,
> + const char *bitmap, bool persistent)
Usually it's one argument per line.
> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +int qemuMonitorEnableBitmap(qemuMonitorPtr mon, const char *node,
> + const char *bitmap)
> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +int qemuMonitorMergeBitmaps(qemuMonitorPtr mon, const char *node,
> + const char *dst, virJSONValuePtr *src)
[...]
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 694ed90622..9b4a5c01ab 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
[...]
> @@ -4471,3 +4486,47 @@ qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon,
> virHashFree(info);
> return ret;
> }
> +
> +int
> +qemuMonitorAddBitmap(qemuMonitorPtr mon, const char *node,
> + const char *bitmap, bool persistent)
One argument per line.
> +{
> + VIR_DEBUG("node=%s bitmap=%s persistent=%d", node, bitmap, persistent);
Should 'node' be declared as NONNULL? it's used without NULLSTR here.
> +
> + QEMU_CHECK_MONITOR(mon);
> +
> + return qemuMonitorJSONAddBitmap(mon, node, bitmap, persistent);
> +}
> +
> +int
> +qemuMonitorEnableBitmap(qemuMonitorPtr mon, const char *node,
> + const char *bitmap)
> +{
> + VIR_DEBUG("node=%s bitmap=%s", node, bitmap);
> +
> + QEMU_CHECK_MONITOR(mon);
> +
> + return qemuMonitorJSONEnableBitmap(mon, node, bitmap);
> +}
> +
> +int
> +qemuMonitorMergeBitmaps(qemuMonitorPtr mon, const char *node,
> + const char *dst, virJSONValuePtr *src)
> +{
> + VIR_DEBUG("node=%s dst=%s src=%p", node, dst, *src);
Logging 'src' as pointer does not have much value. I'd just drop it as
it does not have a simple way to access it.
> +
> + QEMU_CHECK_MONITOR(mon);
> +
> + return qemuMonitorJSONMergeBitmaps(mon, node, dst, src);
> +}
> +
> +int
> +qemuMonitorDeleteBitmap(qemuMonitorPtr mon, const char *node,
> + const char *bitmap)
> +{
> + VIR_DEBUG("node=%s bitmap=%s", node, bitmap);
> +
> + QEMU_CHECK_MONITOR(mon);
> +
> + return qemuMonitorJSONDeleteBitmap(mon, node, bitmap);
> +}
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index fc2d193ae2..801433826f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1011,6 +1011,8 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
> type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
> else if (STREQ(type_str, "mirror"))
> type = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
> + else if (STREQ(type_str, "backup"))
> + type = VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP;
As I've pointed out in previous responses, we need to clarify whether
the backup job is a blockjob, general job or we need a new job API in
libvirt.
>
> switch ((virConnectDomainEventBlockJobStatus) event) {
> case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
> @@ -2755,6 +2757,82 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
> return ret;
> }
>
> +int qemuMonitorJSONUpdateCheckpointSize(qemuMonitorPtr mon,
> + virDomainCheckpointDefPtr chk)
> +{
> + int ret = -1;
> + size_t i, j;
> + virJSONValuePtr devices;
> +
> + if (!(devices = qemuMonitorJSONQueryBlock(mon)))
> + return -1;
> +
> + for (i = 0; i < virJSONValueArraySize(devices); i++) {
> + virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
> + virJSONValuePtr inserted;
> + virJSONValuePtr bitmaps = NULL;
> + const char *node;
> + virDomainCheckpointDiskDefPtr disk;
> +
> + if (!(dev = qemuMonitorJSONGetBlockDev(devices, i)))
> + goto cleanup;
> +
> + if (!(inserted = virJSONValueObjectGetObject(dev, "inserted")))
> + continue;
> + if (!(node = virJSONValueObjectGetString(inserted, "node-name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-block device entry was not in expected format"));
> + goto cleanup;
> + }
> +
> + for (j = 0; j < chk->ndisks; j++) {
> + disk = &chk->disks[j];
> + if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> + continue;
> + if (STREQ(chk->common.dom->disks[disk->idx]->src->nodeformat, node))
This will access stale data after you unplug a disk. Caching disk
indexes is a bad idea.
> + break;
> + }
> + if (j == chk->ndisks) {
> + VIR_DEBUG("query-block did not find node %s", node);
> + continue;
> + }
> + if (!(bitmaps = virJSONValueObjectGetArray(dev, "dirty-bitmaps"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("disk %s dirty bitmaps missing"), disk->name);
> + goto cleanup;
> + }
> + for (j = 0; j < virJSONValueArraySize(bitmaps); j++) {
> + virJSONValuePtr map = virJSONValueArrayGet(bitmaps, j);
> + const char *name;
> +
> + if (!(name = virJSONValueObjectGetString(map, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("dirty bitmaps entry was not in expected format"));
> + goto cleanup;
> + }
> + if (STRNEQ(name, disk->bitmap))
> + continue;
> + if (virJSONValueObjectGetNumberUlong(map, "count", &disk->size) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("invalid bitmap count"));
> + goto cleanup;
> + }
> + break;
> + }
> + if (j == virJSONValueArraySize(bitmaps)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("disk %s dirty bitmap info missing"), disk->name);
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + virJSONValueFree(devices);
> + return ret;
> +}
> +
>
> int qemuMonitorJSONSetPassword(qemuMonitorPtr mon,
> const char *protocol,
> @@ -4706,6 +4784,8 @@ qemuMonitorJSONParseBlockJobInfo(virHashTablePtr blockJobs,
> info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
> else if (STREQ(type, "mirror"))
> info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
> + else if (STREQ(type, "backup"))
> + info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP;
> else
> info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>
[...]
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
