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 <[email protected]> > --- > 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;
> + }
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
