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(a)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;
+ }