On Wed, Mar 27, 2019 at 05:10:52 -0500, Eric Blake wrote:
Time to actually issue the QMP transactions that start and stop backup commands (for now, just pull mode, not push). Starting a job has to kick off several pre-req steps, then a transaction, and additionally spawn an NBD server for pull mode; ending a job as well as failing partway through beginning a job has to unwind the earlier steps. Implementing push mode, as well as incremental pull and checkpoint creation, is deferred to later patches.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_domain.c | 18 ++- src/qemu/qemu_driver.c | 288 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_process.c | 7 + 3 files changed, 299 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6648240dc4..9840be546e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2687,16 +2687,24 @@ qemuDomainObjPrivateXMLParseBlockjobs(virQEMUDriverPtr driver, char *active; int tmp; int ret = -1; + size_t i;
if ((active = virXPathString("string(./blockjobs/@active)", ctxt)) && (tmp = virTristateBoolTypeFromString(active)) > 0) priv->reconnectBlockjobs = tmp;
- if ((node = virXPathNode("./domainbackup", ctxt)) && - !(priv->backup = virDomainBackupDefParseNode(ctxt->doc, node, - driver->xmlopt, - VIR_DOMAIN_BACKUP_PARSE_INTERNAL))) - goto cleanup; + if ((node = virXPathNode("./domainbackup", ctxt))) { + if (!(priv->backup = virDomainBackupDefParseNode(ctxt->doc, node, + driver->xmlopt, + VIR_DOMAIN_BACKUP_PARSE_INTERNAL))) + goto cleanup; + /* The backup job is only stored in XML if backupBegin + * succeeded at exporting the disk, so no need to store disk + * state when we can just force-reset it to a known-good + * value. */ + for (i = 0; i < priv->backup->ndisks; i++) + priv->backup->disks[i].state = VIR_DOMAIN_BACKUP_DISK_STATE_EXPORT; + }
ret = 0; cleanup: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78c9f963ca..e6e7ba7330 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17722,8 +17722,80 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, return ret; }
-static int qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml, - const char *checkpointXml, unsigned int flags) +static int +qemuDomainBackupPrepare(virQEMUDriverPtr driver, virDomainObjPtr vm, + virDomainBackupDefPtr def) +{ + int ret = -1; + size_t i; + + if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + goto cleanup; + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDef *disk = &def->disks[i]; + virStorageSourcePtr src = vm->def->disks[disk->idx]->src; + + if (!disk->store) + continue; + if (virAsprintf(&disk->store->nodeformat, "tmp-%s", disk->name) < 0) + goto cleanup;
Libvirt controlled nodenames need to be unique. It's okay to do a per-disk unique name, but you should encode the operation that created the name into it. See qemuMigrationSrcNBDStorageCopyBlockdev for example.
+ if (!disk->store->format) + disk->store->format = VIR_STORAGE_FILE_QCOW2; + if (def->incremental) { + if (src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("incremental backup of %s requires qcow2"), + disk->name); + goto cleanup; + } + } + } + ret = 0; + cleanup: + return ret; +}
[...]
+static int +qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml, + const char *checkpointXml, unsigned int flags) { virQEMUDriverPtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL;
[...]
@@ -17776,25 +17855,122 @@ static int qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml, if (!(def = virDomainBackupDefParseString(diskXml, driver->xmlopt, 0))) goto cleanup;
+ if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_BITMAP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu binary lacks pull-mode backup support")); + goto cleanup; + } + if (!def->server || + def->server->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("<domainbackup> must specify TCP server for now")); + goto cleanup; + } + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("push mode backups not supported yet")); + goto cleanup; + } + if (def->incremental) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BITMAP_MERGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu binary lacks persistent bitmaps support")); + goto cleanup; + } + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cannot create incremental backups yet")); + goto cleanup; + } + + if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) + goto cleanup; + /* We are going to modify the domain below. */ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup;
- priv = vm->privateData; if (priv->backup) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("another backup job is already running")); goto endjob; }
- if (virDomainBackupAlignDisks(def, vm->def, suffix) < 0) + if (virDomainBackupAlignDisks(def, vm->def, suffix) < 0 || + qemuDomainBackupPrepare(driver, vm, def) < 0) goto endjob;
/* actually start the checkpoint. 2x2 array of push/pull, full/incr, plus additional tweak if checkpoint requested */ - /* TODO: issue QMP commands: - - pull: nbd-server-start with <server> from user (or autogenerate server) - - push/pull: blockdev-add per <disk> + qemuDomainObjEnterMonitor(driver, vm); + /* - push/pull: blockdev-add per <disk> */ + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDef *disk = &def->disks[i]; + virJSONValuePtr file; + virStorageSourcePtr src = vm->def->disks[disk->idx]->src; + const char *node = src->nodeformat; + + if (!disk->store) + continue; + if (qemuDomainStorageFileInit(driver, vm, disk->store, src) < 0) + goto endmon; + if (disk->store->detected) { + if (virStorageFileCreate(disk->store) < 0) { + virReportSystemError(errno, + _("failed to create image file '%s'"), + NULLSTR(disk->store->path)); + goto endmon; + } + disk->state = VIR_DOMAIN_BACKUP_DISK_STATE_CREATED; + } + if (qemuDomainDiskChainElementPrepare(driver, vm, disk->store, false, + true) < 0) + goto endmon; + disk->state = VIR_DOMAIN_BACKUP_DISK_STATE_LABEL; + if (disk->store->detected) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + + /* Force initialization of scratch/target file to new qcow2 */ + if (!(cmd = virCommandNewArgList(qemuImgPath, + "create", + "-f", + virStorageFileFormatTypeToString(disk->store->format), + "-o", + NULL)))
ewww. We've got blockdev-create now. I'll be posting patches soon.
+ goto endmon; + virBufferAsprintf(&buf, "backing_fmt=%s,backing_file=", + virStorageFileFormatTypeToString(src->format)); + virQEMUBuildBufferEscapeComma(&buf, src->path); + virCommandAddArgBuffer(cmd, &buf); + + virQEMUBuildBufferEscapeComma(&buf, disk->store->path); + virCommandAddArgBuffer(cmd, &buf); + if (virCommandRun(cmd, NULL) < 0) + goto endmon; + virCommandFree(cmd); + cmd = NULL; + } + + if (virJSONValueObjectCreate(&file, + "s:driver", "file", + "s:filename", disk->store->path, + NULL) < 0) + goto endmon; + if (virJSONValueObjectCreate(&json, + "s:driver", virStorageFileFormatTypeToString(disk->store->format), + "s:node-name", disk->store->nodeformat, + "a:file", &file, + "s:backing", node, NULL) < 0) { + virJSONValueFree(file); + goto endmon; + } + if (qemuMonitorBlockdevAdd(priv->mon, json) < 0) + goto endmon; + json = NULL; + disk->state = VIR_DOMAIN_BACKUP_DISK_STATE_READY; + } + + /* TODO: - incr: bitmap-add of tmp, bitmap-merge per <disk> - transaction, containing: - push+full: blockdev-backup sync:full @@ -17802,8 +17978,77 @@ static int qemuDomainBackupBegin(virDomainPtr domain, const char *diskXml, - pull+full: blockdev-backup sync:none - pull+incr: blockdev-backup sync:none, bitmap-disable of tmp - if checkpoint: bitmap-disable of old, bitmap-add of new + */ + if (!(json = virJSONValueNewArray())) + goto endmon; + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDef *disk = &def->disks[i]; + virStorageSourcePtr src = vm->def->disks[disk->idx]->src; + + if (!disk->store) + continue; + if (qemuMonitorJSONTransactionAdd(json, + "blockdev-backup", + "s:device", src->nodeformat, + "s:target", disk->store->nodeformat, + "s:sync", "none", + "s:job-id", disk->name,
This really also needs to be a unique name. Otherwise it will cause more than necessary headache when attempting to combine blockjobs and block backup. Prefix it with the job type e.g. "backup-vda" or "backup-nodename" as it looks like this is a per-node operation.
+ NULL) < 0) + goto endmon; + }