Add a simpler constructor for hash tables which specifically does not require specifying the initial hash size and uses simpler freeing function. The initial hash table size usually is not important as the hash table is growing when it reaches certain number of entries in one bucket. Additionally many callers pass in a random small number for ad-hoc table use so using a central one will simplify things. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virhash.c | 21 +++++++++++++++++++++ src/util/virhash.h | 1 + 3 files changed, 23 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0da02bb8bd..c155f51174 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2066,6 +2066,7 @@ virHashForEach; virHashFree; virHashGetItems; virHashLookup; +virHashNew; virHashRemoveAll; virHashRemoveEntry; virHashRemoveSet; diff --git a/src/util/virhash.c b/src/util/virhash.c index 4d90fa5333..de03032bf1 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -169,6 +169,27 @@ virHashTablePtr virHashCreateFull(ssize_t size, } +/** + * virHashNew: + * @dataFree: callback to free data + * + * Create a new virHashTablePtr. + * + * Returns the newly created object, or NULL if an error occurred. + */ +virHashTablePtr +virHashNew(virHashDataFreeSimple dataFree) +{ + return virHashCreateFull(32, + NULL, + dataFree, + virHashStrCode, + virHashStrEqual, + virHashStrCopy, + virHashStrFree); +} + + /** * virHashCreate: * @size: the size of the hash table diff --git a/src/util/virhash.h b/src/util/virhash.h index 94fe8e23e4..d7de0618cb 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -107,6 +107,7 @@ typedef void (*virHashKeyFree)(void *name); /* * Constructor and destructor. */ +virHashTablePtr virHashNew(virHashDataFreeSimple dataFree); virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree); virHashAtomicPtr virHashAtomicNew(ssize_t size, -- 2.21.0

Add a helper that checks whether an entry with given name exists but does not touch the userdata. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virhash.c | 49 ++++++++++++++++++++++++++++++++-------- src/util/virhash.h | 1 + 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c155f51174..588f0a4356 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2065,6 +2065,7 @@ virHashEqual; virHashForEach; virHashFree; virHashGetItems; +virHashHasEntry; virHashLookup; virHashNew; virHashRemoveAll; diff --git a/src/util/virhash.c b/src/util/virhash.c index de03032bf1..df07e37e42 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -445,6 +445,26 @@ virHashAtomicUpdate(virHashAtomicPtr table, } +static virHashEntryPtr +virHashGetEntry(const virHashTable *table, + const void *name) +{ + size_t key; + virHashEntryPtr entry; + + if (!table || !name) + return NULL; + + key = virHashComputeKey(table, name); + for (entry = table->table[key]; entry; entry = entry->next) { + if (table->keyEqual(entry->name, name)) + return entry; + } + + return NULL; +} + + /** * virHashLookup: * @table: the hash table @@ -457,18 +477,29 @@ virHashAtomicUpdate(virHashAtomicPtr table, void * virHashLookup(const virHashTable *table, const void *name) { - size_t key; - virHashEntryPtr entry; + virHashEntryPtr entry = virHashGetEntry(table, name); - if (!table || !name) + if (!entry) return NULL; - key = virHashComputeKey(table, name); - for (entry = table->table[key]; entry; entry = entry->next) { - if (table->keyEqual(entry->name, name)) - return entry->payload; - } - return NULL; + return entry->payload; +} + + +/** + * virHashHasEntry: + * @table: the hash table + * @name: the name of the userdata + * + * Find whether entry specified by @name exists. + * + * Returns true if the entry exists and false otherwise + */ +bool +virHashHasEntry(const virHashTable *table, + const void *name) +{ + return !!virHashGetEntry(table, name); } diff --git a/src/util/virhash.h b/src/util/virhash.h index d7de0618cb..8087965ee9 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -150,6 +150,7 @@ ssize_t virHashRemoveAll(virHashTablePtr table); * Retrieve the userdata. */ void *virHashLookup(const virHashTable *table, const void *name); +bool virHashHasEntry(const virHashTable *table, const void *name); /* * Retrieve & remove the userdata. -- 2.21.0

qemuDomainPrepareDiskSourceData historically prepared everything but we'we split out the majority of the functionality so that it sets up predominantely only according to the configuration of the disk. There was one leftover bit of setting the gluster debug level from the config. Split this out into a separate function so that qemuDomainPrepareDiskSourceData only perpares based on the disk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 51 +++++++++++++++++++++++++++++------------- src/qemu/qemu_domain.h | 4 +--- tests/qemublocktest.c | 2 +- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a97bf65e7f..6a93dd5293 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10251,6 +10251,32 @@ qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src, } +/** + * qemuDomainPrepareStorageSourceConfig: + * @src: storage source to configure + * @cfg: qemu driver config object + * @qemuCaps: capabilities of qemu + * + * Set properties of @src based on the qemu driver config @cfg. + * + */ +static void +qemuDomainPrepareStorageSourceConfig(virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) +{ + if (!cfg) + return; + + if (src->type == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { + src->debug = true; + src->debugLevel = cfg->glusterDebugLevel; + } +} + + /** * qemuDomainDetermineDiskChain: * @driver: qemu driver object @@ -10356,7 +10382,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (qemuDomainValidateStorageSource(n, priv->qemuCaps) < 0) return -1; - if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) < 0) + qemuDomainPrepareStorageSourceConfig(n, cfg, priv->qemuCaps); + + if (qemuDomainPrepareDiskSourceData(disk, n) < 0) return -1; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && @@ -15060,7 +15088,6 @@ qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def, * * @disk: Disk config object * @src: source to start from - * @cfg: qemu driver config object * * Prepares various aspects of a storage source belonging to a disk backing * chain. This function should be also called for detected backing chain @@ -15068,22 +15095,12 @@ qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def, */ int qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg, - virQEMUCapsPtr qemuCaps) + virStorageSourcePtr src) { /* transfer properties valid only for the top level image */ if (src == disk->src) src->detect_zeroes = disk->detect_zeroes; - if (cfg && - src->type == VIR_STORAGE_TYPE_NETWORK && - src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { - src->debug = true; - src->debugLevel = cfg->glusterDebugLevel; - } - /* transfer properties valid for the full chain */ src->iomode = disk->iomode; src->cachemode = disk->cachemode; @@ -15144,7 +15161,9 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, if (qemuDomainValidateStorageSource(disk->src, priv->qemuCaps) < 0) return -1; - if (qemuDomainPrepareDiskSourceData(disk, disk->src, cfg, priv->qemuCaps) < 0) + qemuDomainPrepareStorageSourceConfig(disk->src, cfg, priv->qemuCaps); + + if (qemuDomainPrepareDiskSourceData(disk, disk->src) < 0) return -1; if (qemuDomainSecretStorageSourcePrepare(priv, disk->src, @@ -15178,7 +15197,9 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, if (qemuDomainValidateStorageSource(src, priv->qemuCaps) < 0) return -1; - if (qemuDomainPrepareDiskSourceData(disk, src, cfg, priv->qemuCaps) < 0) + qemuDomainPrepareStorageSourceConfig(src, cfg, priv->qemuCaps); + + if (qemuDomainPrepareDiskSourceData(disk, src) < 0) return -1; if (qemuDomainSecretStorageSourcePrepare(priv, src, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d703417862..14367f7320 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1169,9 +1169,7 @@ qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def, int qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg, - virQEMUCapsPtr qemuCaps) + virStorageSourcePtr src) G_GNUC_WARN_UNUSED_RESULT; diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 0f073682ca..9b7abceb40 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -223,7 +223,7 @@ testQemuDiskXMLToProps(const void *opaque) if (qemuDomainValidateStorageSource(n, data->qemuCaps) < 0) goto cleanup; - if (qemuDomainPrepareDiskSourceData(disk, n, NULL, data->qemuCaps) < 0) + if (qemuDomainPrepareDiskSourceData(disk, n) < 0) goto cleanup; if (!(formatProps = qemuBlockStorageSourceGetBlockdevProps(n, n->backingStore)) || -- 2.21.0

Last use was removed in 29682196d8f. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 11 ----------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 3 files changed, 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88e93f6fb8..8b4e80fa8d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16922,17 +16922,6 @@ virDomainDiskIndexByName(virDomainDefPtr def, const char *name, return candidate; } -/* Return the path to a disk image if a string identifies at least one - * disk belonging to the domain (both device strings 'vda' and paths - * '/path/to/file' are converted into '/path/to/file'). */ -const char * -virDomainDiskPathByName(virDomainDefPtr def, const char *name) -{ - int idx = virDomainDiskIndexByName(def, name, true); - - return idx < 0 ? NULL : virDomainDiskGetSource(def->disks[idx]); -} - virDomainDiskDefPtr virDomainDiskByName(virDomainDefPtr def, const char *name, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b33e5334f4..ad8e88c2ee 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3162,7 +3162,6 @@ int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, virDomainDiskDefPtr virDomainDiskByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); -const char *virDomainDiskPathByName(virDomainDefPtr, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 588f0a4356..e6d40d1cbe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -362,7 +362,6 @@ virDomainDiskMirrorStateTypeFromString; virDomainDiskMirrorStateTypeToString; virDomainDiskModelTypeFromString; virDomainDiskModelTypeToString; -virDomainDiskPathByName; virDomainDiskRemove; virDomainDiskRemoveByName; virDomainDiskSetBlockIOTune; -- 2.21.0

Introduce a simpler replacement for virDomainDiskByName when looking up by disk target. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 21 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8b4e80fa8d..1fffd56a22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16931,6 +16931,22 @@ virDomainDiskByName(virDomainDefPtr def, return idx < 0 ? NULL : def->disks[idx]; } + +virDomainDiskDefPtr +virDomainDiskByTarget(virDomainDefPtr def, + const char *dst) +{ + size_t i; + + for (i = 0; i < def->ndisks; i++) { + if (STREQ(def->disks[i]->dst, dst)) + return def->disks[i]; + } + + return NULL; +} + + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ad8e88c2ee..690aa3c84b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3162,6 +3162,10 @@ int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, virDomainDiskDefPtr virDomainDiskByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); +virDomainDiskDefPtr +virDomainDiskByTarget(virDomainDefPtr def, + const char *dst); + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e6d40d1cbe..2379c1ed8b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -330,6 +330,7 @@ virDomainDiskBackingStoreParse; virDomainDiskBusTypeToString; virDomainDiskByAddress; virDomainDiskByName; +virDomainDiskByTarget; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; virDomainDiskDefAssignAddress; -- 2.21.0

In both replaced cases we have other code that verifies that the bus can't be changed or that the target is unique, so limiting the search to disks with same bus makes no sense. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 7 ++----- src/qemu/qemu_hotplug.c | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bf436f7dc3..ac5f108537 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7981,12 +7981,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, virDomainDeviceDef oldDev = { .type = dev->type }; int ret = -1; - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, - disk->bus, disk->dst))) { + if (!(orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("No device with bus '%s' and target '%s'"), - virDomainDiskBusTypeToString(disk->bus), - disk->dst); + _("disk '%s' not found"), disk->dst); goto cleanup; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bf301919cc..3cb7f4cddd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1092,7 +1092,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, * for devices supporting media changes */ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && - (orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { + (orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) { if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, disk->src, false) < 0) return -1; -- 2.21.0

Previous commit removed last use of this function so we can get rid of it.t Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 21 --------------------- src/conf/domain_conf.h | 3 --- src/libvirt_private.syms | 1 - 3 files changed, 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1fffd56a22..e12d676a8c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8566,27 +8566,6 @@ virDomainDeviceFindSCSIController(const virDomainDef *def, return NULL; } -virDomainDiskDefPtr -virDomainDiskFindByBusAndDst(virDomainDefPtr def, - int bus, - char *dst) -{ - size_t i; - - if (!dst) - return NULL; - - for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->bus == bus && - STREQ(def->disks[i]->dst, dst)) { - return def->disks[i]; - } - } - - return NULL; -} - - int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 690aa3c84b..071de9e71e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2859,9 +2859,6 @@ void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format); virDomainControllerDefPtr virDomainDeviceFindSCSIController(const virDomainDef *def, virDomainDeviceInfoPtr info); -virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def, - int bus, - char *dst); virDomainControllerDefPtr virDomainControllerDefNew(virDomainControllerType type); void virDomainControllerDefFree(virDomainControllerDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2379c1ed8b..fdfe1e7295 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -345,7 +345,6 @@ virDomainDiskDiscardTypeToString; virDomainDiskEmptySource; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; -virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; virDomainDiskGetDetectZeroesMode; -- 2.21.0

In many cases we used virDomainDiskByName to solely look up disk by target. We have a new helper now so we can replace it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_blockjob.c | 6 +++--- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 3 +-- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 64b21f2073..a08bafe5ce 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4072,7 +4072,7 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; - if (!(orig = virDomainDiskByName(vmdef, disk->dst, false))) { + if (!(orig = virDomainDiskByTarget(vmdef, disk->dst))) { virReportError(VIR_ERR_INVALID_ARG, _("target %s doesn't exist."), disk->dst); goto cleanup; diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 3506fa165b..f9b3bdaff4 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -601,7 +601,7 @@ qemuBlockJobRewriteConfigDiskSource(virDomainObjPtr vm, if (!vm->newDef) return; - if (!(persistDisk = virDomainDiskByName(vm->newDef, disk->dst, false))) + if (!(persistDisk = virDomainDiskByTarget(vm->newDef, disk->dst))) return; if (!virStorageSourceIsSameLocation(disk->src, persistDisk->src)) @@ -839,7 +839,7 @@ qemuBlockJobGetConfigDisk(virDomainObjPtr vm, disksrc = disk->src; - if (!(ret = virDomainDiskByName(vm->newDef, disk->dst, false))) + if (!(ret = virDomainDiskByTarget(vm->newDef, disk->dst))) return NULL; cfgsrc = ret->src; @@ -884,7 +884,7 @@ qemuBlockJobClearConfigChain(virDomainObjPtr vm, if (!vm->newDef || !disk) return; - if (!(cfgdisk = virDomainDiskByName(vm->newDef, disk->dst, false))) + if (!(cfgdisk = virDomainDiskByTarget(vm->newDef, disk->dst))) return; if (!virStorageSourceIsSameLocation(disk->src, cfgdisk->src)) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3deb69cb63..e330d2390c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3201,7 +3201,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, invalidData = true; if ((diskdst = virXPathString("string(./disk/@dst)", ctxt)) && - !(disk = virDomainDiskByName(vm->def, diskdst, false))) + !(disk = virDomainDiskByTarget(vm->def, diskdst))) invalidData = true; if ((mirror = virXPathString("string(./disk/@mirror)", ctxt)) && @@ -3374,7 +3374,7 @@ qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm, virDomainDiskDefPtr disk; if ((dst = virXMLPropString(nodes[i], "dev")) && - (disk = virDomainDiskByName(vm->def, dst, false))) { + (disk = virDomainDiskByTarget(vm->def, dst))) { QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating = true; if (qemuDomainObjPrivateXMLParseJobNBDSource(nodes[i], ctxt, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac5f108537..e50f2277d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15276,7 +15276,7 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, /* modify disk in persistent definition only when the source is the same */ if (vm->newDef && - (persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst, false)) && + (persistdisk = virDomainDiskByTarget(vm->newDef, dd->disk->dst)) && virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) { dd->persistdisk = persistdisk; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c65491f9d6..40aa598832 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -319,8 +319,7 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm, VIR_DEBUG("Looking up disk target '%s' (capacity=%llu)", nbd->disks[i].target, nbd->disks[i].capacity); - if (!(disk = virDomainDiskByName(vm->def, nbd->disks[i].target, - false))) { + if (!(disk = virDomainDiskByTarget(vm->def, nbd->disks[i].target))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to find disk by target: %s"), nbd->disks[i].target); -- 2.21.0

Retrieve data for individual block nodes in a hash table. Currently only capacity and allocation data is extracted but this will be extended in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++ src/qemu/qemu_monitor.h | 11 ++++++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ 4 files changed, 101 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 86d3800108..b6c890f9ce 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2220,6 +2220,24 @@ qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, return qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(mon, stats); } + +/** + * qemuMonitorBlockGetNamedNodeData: + * @mon: monitor object + * + * Uses 'query-named-block-nodes' to retrieve information about individual + * storage nodes and returns them in a hash table of qemuBlockNamedNodeDataPtrs + * filled with the data. The hash table keys are node names. + */ +virHashTablePtr +qemuMonitorBlockGetNamedNodeData(qemuMonitorPtr mon) +{ + QEMU_CHECK_MONITOR_NULL(mon); + + return qemuMonitorJSONBlockGetNamedNodeData(mon); +} + + int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 11048dc76a..36eb5f342d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -676,6 +676,17 @@ int qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, virHashTablePtr stats) ATTRIBUTE_NONNULL(2); + +typedef struct _qemuBlockNamedNodeData qemuBlockNamedNodeData; +typedef qemuBlockNamedNodeData *qemuBlockNamedNodeDataPtr; +struct _qemuBlockNamedNodeData { + unsigned long long capacity; + unsigned long long physical; +}; + +virHashTablePtr +qemuMonitorBlockGetNamedNodeData(qemuMonitorPtr mon); + int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 02308dffe6..c474ac0203 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2879,6 +2879,75 @@ qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, } +static void +qemuMonitorJSONBlockNamedNodeDataFree(qemuBlockNamedNodeDataPtr data) +{ + if (!data) + return; + + g_free(data); +} +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNamedNodeData, qemuMonitorJSONBlockNamedNodeDataFree); + + +static int +qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, + virJSONValuePtr val, + void *opaque) +{ + virHashTablePtr nodes = opaque; + virJSONValuePtr img; + const char *nodename; + g_autoptr(qemuBlockNamedNodeData) ent = NULL; + + ent = g_new0(qemuBlockNamedNodeData, 1); + + if (!(nodename = virJSONValueObjectGetString(val, "node-name")) || + !(img = virJSONValueObjectGetObject(val, "image"))) + goto broken; + + if (virJSONValueObjectGetNumberUlong(img, "virtual-size", &ent->capacity) < 0) + goto broken; + + /* if actual-size is missing, image is not thin provisioned */ + if (virJSONValueObjectGetNumberUlong(img, "actual-size", &ent->physical) < 0) + ent->physical = ent->capacity; + + if (virHashAddEntry(nodes, nodename, ent) < 0) + return -1; + + ent = NULL; + + return 1; /* we don't want to steal the value from the JSON array */ + + broken: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-named-block-nodes entry was not in expected format")); + return -1; +} + + +virHashTablePtr +qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon) +{ + g_autoptr(virJSONValue) nodes = NULL; + g_autoptr(virHashTable) ret = NULL; + + if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon))) + return NULL; + + if (!(ret = virHashNew((virHashDataFreeSimple) qemuMonitorJSONBlockNamedNodeDataFree))) + return NULL; + + if (virJSONValueArrayForeachSteal(nodes, + qemuMonitorJSONBlockGetNamedNodeDataWorker, + ret) < 0) + return NULL; + + return g_steal_pointer(&ret); +} + + int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 975de3759a..6ee3e912f9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -87,6 +87,9 @@ int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, int qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, virHashTablePtr stats); +virHashTablePtr +qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon); + int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, -- 2.21.0

Calling the monitor was convenient for the implementation in qemuDomainBlockCopyCommon, but causes the snapshot code to call query-named-block-nodes for every disk. Fix this by removing the monitor call from qemuBlockStorageSourceCreateDetectSize so that the data can be reused in loops. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 26 +++++--------------------- src/qemu/qemu_block.h | 5 ++--- src/qemu/qemu_driver.c | 33 ++++++++++++++++++++++++++++----- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4dc4f2922d..c966b24cb6 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2578,10 +2578,9 @@ qemuBlockStorageSourceCreate(virDomainObjPtr vm, /** * qemuBlockStorageSourceCreateDetectSize: - * @vm: domain object + * @blockNamedNodeData: hash table filled with qemuBlockNamedNodeData * @src: storage source to update size/capacity on * @templ: storage source template - * @asyncJob: qemu asynchronous job type * * When creating a storage source via blockdev-create we need to know the size * and capacity of the original volume (e.g. when creating a snapshot or copy). @@ -2589,28 +2588,13 @@ qemuBlockStorageSourceCreate(virDomainObjPtr vm, * to the detected sizes from @templ. */ int -qemuBlockStorageSourceCreateDetectSize(virDomainObjPtr vm, +qemuBlockStorageSourceCreateDetectSize(virHashTablePtr blockNamedNodeData, virStorageSourcePtr src, - virStorageSourcePtr templ, - qemuDomainAsyncJob asyncJob) + virStorageSourcePtr templ) { - qemuDomainObjPrivatePtr priv = vm->privateData; - g_autoptr(virHashTable) stats = NULL; - qemuBlockStatsPtr entry; - int rc; - - if (!(stats = virHashCreate(10, virHashValueFree))) - return -1; - - if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) - return -1; - - rc = qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon, stats); - - if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || rc < 0) - return -1; + qemuBlockNamedNodeDataPtr entry; - if (!(entry = virHashLookup(stats, templ->nodeformat))) { + if (!(entry = virHashLookup(blockNamedNodeData, templ->nodeformat))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to update capacity data for block node '%s'"), templ->nodeformat); diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index be39c9b472..5ddeeaf6e2 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -194,7 +194,6 @@ qemuBlockStorageSourceCreate(virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); int -qemuBlockStorageSourceCreateDetectSize(virDomainObjPtr vm, +qemuBlockStorageSourceCreateDetectSize(virHashTablePtr blockNamedNodeData, virStorageSourcePtr src, - virStorageSourcePtr templ, - qemuDomainAsyncJob asyncJob); + virStorageSourcePtr templ); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e50f2277d2..e908331a93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15252,6 +15252,7 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, qemuDomainSnapshotDiskDataPtr dd, + virHashTablePtr blockNamedNodeData, bool reuse, bool blockdev, qemuDomainAsyncJob asyncJob) @@ -15355,8 +15356,8 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) return -1; } else { - if (qemuBlockStorageSourceCreateDetectSize(vm, dd->src, dd->disk->src, - asyncJob) < 0) + if (qemuBlockStorageSourceCreateDetectSize(blockNamedNodeData, + dd->src, dd->disk->src) < 0) return -1; if (qemuBlockStorageSourceCreate(vm, dd->src, dd->disk->src, @@ -15385,6 +15386,7 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, bool reuse, bool blockdev, + virHashTablePtr blockNamedNodeData, qemuDomainAsyncJob asyncJob, qemuDomainSnapshotDiskDataPtr *rdata, size_t *rndata) @@ -15404,7 +15406,9 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, if (qemuDomainSnapshotDiskPrepareOne(driver, vm, cfg, vm->def->disks[i], snapdef->disks + i, - data + ndata++, reuse, blockdev, + data + ndata++, + blockNamedNodeData, + reuse, blockdev, asyncJob) < 0) goto cleanup; } @@ -15495,6 +15499,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, qemuDomainSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); + g_autoptr(virHashTable) blockNamedNodeData = NULL; if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15502,10 +15507,21 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (!(actions = virJSONValueNewArray())) return -1; + if (blockdev) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData) + return -1; + } + /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev, - asyncJob, &diskdata, &ndiskdata) < 0) + blockNamedNodeData, asyncJob, + &diskdata, &ndiskdata) < 0) goto cleanup; /* check whether there's anything to do */ @@ -18018,6 +18034,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, g_autoptr(qemuBlockStorageSourceChainData) crdata = NULL; virStorageSourcePtr n; virStorageSourcePtr mirrorBacking = NULL; + g_autoptr(virHashTable) blockNamedNodeData = NULL; int rc = 0; /* Preliminaries: find the disk we are editing, sanity checks */ @@ -18179,7 +18196,13 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, priv->qemuCaps))) goto endjob; } else { - if (qemuBlockStorageSourceCreateDetectSize(vm, mirror, disk->src, QEMU_ASYNC_JOB_NONE) < 0) + qemuDomainObjEnterMonitor(driver, vm); + blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData) + goto endjob; + + if (qemuBlockStorageSourceCreateDetectSize(blockNamedNodeData, + mirror, disk->src)) goto endjob; if (mirror_shallow) { -- 2.21.0

Delete/merge bitmaps when deleting checkpoints using a 'transaction' so that we don't have to deal with halfway-failed scenarios and also fix access to 'vm' while in the monitor lock. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 47 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 4b4ad8bb83..a225f04831 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -133,11 +133,14 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver, if (!metadata_only) { qemuDomainObjPrivatePtr priv = vm->privateData; - bool success = true; bool search_parents; virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk); + int rc; + g_autoptr(virJSONValue) actions = NULL; + + if (!(actions = virJSONValueNewArray())) + return -1; - qemuDomainObjEnterMonitor(driver, vm); parent = virDomainCheckpointFindByName(vm->checkpoints, chk->def->parent_name); for (i = 0; i < chkdef->ndisks; i++) { @@ -167,31 +170,29 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver, continue; search_parents = false; - arr = virJSONValueNewArray(); - if (!arr || - virJSONValueArrayAppendString(arr, disk->bitmap) < 0) { - success = false; - break; - } - if (chk == virDomainCheckpointGetCurrent(vm->checkpoints) && - qemuMonitorEnableBitmap(priv->mon, node, - disk2->bitmap) < 0) { - success = false; - break; - } - if (qemuMonitorMergeBitmaps(priv->mon, node, - disk2->bitmap, &arr) < 0) { - success = false; - break; + if (!(arr = virJSONValueNewArray())) + return -1; + + if (virJSONValueArrayAppendString(arr, disk->bitmap) < 0) + return -1; + + if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) { + if (qemuMonitorTransactionBitmapEnable(actions, node, disk2->bitmap) < 0) + return -1; } + + if (qemuMonitorTransactionBitmapMerge(actions, node, disk2->bitmap, &arr) < 0) + return -1; } } - if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) { - success = false; - break; - } + + if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 0) + return -1; } - if (qemuDomainObjExitMonitor(driver, vm) < 0 || !success) + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorTransaction(priv->mon, &actions); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) return -1; } -- 2.21.0

The 'other' variable was used to store the parent of the redefined checkpoint and then the existing version of the currently redefined checkpoint. Make it less confusing by adding a 'parent' variable for the first case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index b254dce7fd..ea2e77a50a 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -540,6 +540,7 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, { virDomainCheckpointDefPtr def = *defptr; char uuidstr[VIR_UUID_STRING_BUFLEN]; + virDomainMomentObjPtr parent = NULL; virDomainMomentObjPtr other = NULL; virDomainCheckpointDefPtr otherdef = NULL; @@ -558,12 +559,13 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, if (virDomainCheckpointAlignDisks(def) < 0) return -1; - if (def->parent.parent_name) - other = virDomainCheckpointFindByName(vm->checkpoints, - def->parent.parent_name); - if (other == virDomainCheckpointGetCurrent(vm->checkpoints)) { - *update_current = true; - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + if (def->parent.parent_name && + (parent = virDomainCheckpointFindByName(vm->checkpoints, + def->parent.parent_name))) { + if (parent == virDomainCheckpointGetCurrent(vm->checkpoints)) { + *update_current = true; + virDomainCheckpointSetCurrent(vm->checkpoints, NULL); + } } other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name); -- 2.21.0

There's no point in clearing the current checkpoint when we are just changing the definition of the current checkpoint as by the virtue of the 'update_current' flag the same checkpoint would become current in qemuCheckpointCreateXML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index ea2e77a50a..7f2cfd7f4c 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -575,11 +575,6 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, def->parent.dom, xmlopt)) return -1; - if (other == virDomainCheckpointGetCurrent(vm->checkpoints)) { - *update_current = true; - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - } - /* Drop and rebuild the parent relationship, but keep all * child relations by reusing chk. */ virDomainMomentDropParent(other); -- 2.21.0

If we are updating the current checkpoint when redefining by mentioning the current checkpoint as a parent of the newly redefined one we don't have to clear it first. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 7f2cfd7f4c..cbff2a0db0 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -562,10 +562,8 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm, if (def->parent.parent_name && (parent = virDomainCheckpointFindByName(vm->checkpoints, def->parent.parent_name))) { - if (parent == virDomainCheckpointGetCurrent(vm->checkpoints)) { + if (parent == virDomainCheckpointGetCurrent(vm->checkpoints)) *update_current = true; - virDomainCheckpointSetCurrent(vm->checkpoints, NULL); - } } other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name); -- 2.21.0

Prevent insane configurations by enforcing that disk bitmap for a checkpoint must match the name of the checkpoint. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index a225f04831..54719e7f5c 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -271,6 +271,13 @@ qemuCheckpointPrepare(virQEMUDriverPtr driver, if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) continue; + if (STRNEQ(disk->bitmap, def->parent.name)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("bitmap for disk '%s' must match checkpoint name '%s'"), + disk->name, def->parent.name); + goto cleanup; + } + if (vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("checkpoint for disk %s unsupported " -- 2.21.0

Separate out individual steps of creating a checkpoint from qemuCheckpointCreateXML into separate functions. This makes the function more readable and understandable and also some of the new functions will be reusable when we will be creating a checkpoint along with a backup in the upcoming patches. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 160 ++++++++++++++++++++++++------------- src/qemu/qemu_checkpoint.h | 8 ++ 2 files changed, 111 insertions(+), 57 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 54719e7f5c..946ae78368 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -348,6 +348,90 @@ qemuCheckpointAddActions(virDomainObjPtr vm, } +static virDomainMomentObjPtr +qemuCheckpointRedefine(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCheckpointDefPtr *def, + bool *update_current) +{ + virDomainMomentObjPtr chk = NULL; + + if (virDomainCheckpointRedefinePrep(vm, def, &chk, driver->xmlopt, + update_current) < 0) + return NULL; + + /* XXX Should we validate that the redefined checkpoint even + * makes sense, such as checking that qemu-img recognizes the + * checkpoint bitmap name in at least one of the domain's disks? */ + + if (chk) + return chk; + + chk = virDomainCheckpointAssignDef(vm->checkpoints, *def); + *def = NULL; + return chk; +} + + +int +qemuCheckpointCreateCommon(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCapsPtr caps, + virDomainCheckpointDefPtr *def, + virJSONValuePtr *actions, + virDomainMomentObjPtr *chk) +{ + g_autoptr(virJSONValue) tmpactions = NULL; + virDomainMomentObjPtr parent; + + if (qemuCheckpointPrepare(driver, caps, vm, *def) < 0) + return -1; + + if ((parent = virDomainCheckpointGetCurrent(vm->checkpoints))) { + if (VIR_STRDUP((*def)->parent.parent_name, parent->def->name) < 0) + return -1; + } + + if (!(tmpactions = virJSONValueNewArray())) + return -1; + + if (qemuCheckpointAddActions(vm, tmpactions, parent, *def) < 0) + return -1; + + if (!(*chk = virDomainCheckpointAssignDef(vm->checkpoints, *def))) + return -1; + + *def = NULL; + + *actions = g_steal_pointer(&tmpactions); + return 0; +} + + +static virDomainMomentObjPtr +qemuCheckpointCreate(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCapsPtr caps, + virDomainCheckpointDefPtr *def) +{ + g_autoptr(virJSONValue) actions = NULL; + virDomainMomentObjPtr chk = NULL; + int rc; + + if (qemuCheckpointCreateCommon(driver, vm, caps, def, &actions, &chk) < 0) + return NULL; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorTransaction(qemuDomainGetMonitor(vm), &actions); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) { + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + return NULL; + } + + return chk; +} + + virDomainCheckpointPtr qemuCheckpointCreateXML(virDomainPtr domain, virDomainObjPtr vm, @@ -361,11 +445,8 @@ qemuCheckpointCreateXML(virDomainPtr domain, bool update_current = true; bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE; unsigned int parse_flags = 0; - virDomainMomentObjPtr other = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(virCaps) caps = NULL; - g_autoptr(virJSONValue) actions = NULL; - int ret; g_autoptr(virDomainCheckpointDef) def = NULL; virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE, NULL); @@ -400,44 +481,30 @@ qemuCheckpointCreateXML(virDomainPtr domain, return NULL; if (redefine) { - if (virDomainCheckpointRedefinePrep(vm, &def, &chk, - driver->xmlopt, - &update_current) < 0) - goto endjob; - } else if (qemuCheckpointPrepare(driver, caps, vm, def) < 0) { - goto endjob; + chk = qemuCheckpointRedefine(driver, vm, &def, &update_current); + } else { + chk = qemuCheckpointCreate(driver, vm, caps, &def); } - if (!chk) { - if (!(chk = virDomainCheckpointAssignDef(vm->checkpoints, def))) - goto endjob; - - def = NULL; - } + if (!chk) + goto endjob; - other = virDomainCheckpointGetCurrent(vm->checkpoints); - if (other) { - if (!redefine && - VIR_STRDUP(chk->def->parent_name, other->def->name) < 0) - goto endjob; + if (update_current) + virDomainCheckpointSetCurrent(vm->checkpoints, chk); + + if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + /* if writing of metadata fails, error out rather than trying + * to silently carry on without completing the checkpoint */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to save metadata for checkpoint %s"), + chk->def->name); + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + goto endjob; } - /* actually do the checkpoint */ - if (redefine) { - /* XXX Should we validate that the redefined checkpoint even - * makes sense, such as checking that qemu-img recognizes the - * checkpoint bitmap name in at least one of the domain's disks? */ - } else { - if (!(actions = virJSONValueNewArray())) - goto endjob; - if (qemuCheckpointAddActions(vm, actions, other, - virDomainCheckpointObjGetDef(chk)) < 0) - goto endjob; - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorTransaction(priv->mon, &actions); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) - goto endjob; - } + virDomainCheckpointLinkParent(vm->checkpoints, chk); /* If we fail after this point, there's not a whole lot we can do; * we've successfully created the checkpoint, so we have to go @@ -446,27 +513,6 @@ qemuCheckpointCreateXML(virDomainPtr domain, checkpoint = virGetDomainCheckpoint(domain, chk->def->name); endjob: - if (checkpoint) { - if (update_current) - virDomainCheckpointSetCurrent(vm->checkpoints, chk); - if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { - /* if writing of metadata fails, error out rather than trying - * to silently carry on without completing the checkpoint */ - virObjectUnref(checkpoint); - checkpoint = NULL; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to save metadata for checkpoint %s"), - chk->def->name); - virDomainCheckpointObjListRemove(vm->checkpoints, chk); - } else { - virDomainCheckpointLinkParent(vm->checkpoints, chk); - } - } else if (chk) { - virDomainCheckpointObjListRemove(vm->checkpoints, chk); - } - qemuDomainObjEndJob(driver, vm); return checkpoint; diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 49f51b1fdd..7fcbc99541 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -53,3 +53,11 @@ int qemuCheckpointDelete(virDomainObjPtr vm, virDomainCheckpointPtr checkpoint, unsigned int flags); + +int +qemuCheckpointCreateCommon(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCapsPtr caps, + virDomainCheckpointDefPtr *def, + virJSONValuePtr *actions, + virDomainMomentObjPtr *chk); -- 2.21.0

Extract the linking and saving bits of checkpoint creation into qemuCheckpointCreateFinalize so that qemuCheckpointCreateXML is a bit simpler and also makes it reusable in the backup code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 44 +++++++++++++++++++++++++------------- src/qemu/qemu_checkpoint.h | 7 ++++++ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 946ae78368..6a272f9dab 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -432,6 +432,34 @@ qemuCheckpointCreate(virQEMUDriverPtr driver, } +int +qemuCheckpointCreateFinalize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, + virDomainMomentObjPtr chk, + bool update_current) +{ + if (update_current) + virDomainCheckpointSetCurrent(vm->checkpoints, chk); + + if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, + driver->xmlopt, + cfg->checkpointDir) < 0) { + /* if writing of metadata fails, error out rather than trying + * to silently carry on without completing the checkpoint */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to save metadata for checkpoint %s"), + chk->def->name); + virDomainCheckpointObjListRemove(vm->checkpoints, chk); + return -1; + } + + virDomainCheckpointLinkParent(vm->checkpoints, chk); + + return 0; +} + + virDomainCheckpointPtr qemuCheckpointCreateXML(virDomainPtr domain, virDomainObjPtr vm, @@ -489,22 +517,8 @@ qemuCheckpointCreateXML(virDomainPtr domain, if (!chk) goto endjob; - if (update_current) - virDomainCheckpointSetCurrent(vm->checkpoints, chk); - - if (qemuCheckpointWriteMetadata(vm, chk, driver->caps, - driver->xmlopt, - cfg->checkpointDir) < 0) { - /* if writing of metadata fails, error out rather than trying - * to silently carry on without completing the checkpoint */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to save metadata for checkpoint %s"), - chk->def->name); - virDomainCheckpointObjListRemove(vm->checkpoints, chk); + if (qemuCheckpointCreateFinalize(driver, vm, cfg, chk, update_current) < 0) goto endjob; - } - - virDomainCheckpointLinkParent(vm->checkpoints, chk); /* If we fail after this point, there's not a whole lot we can do; * we've successfully created the checkpoint, so we have to go diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 7fcbc99541..d0ea8f000f 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -61,3 +61,10 @@ qemuCheckpointCreateCommon(virQEMUDriverPtr driver, virDomainCheckpointDefPtr *def, virJSONValuePtr *actions, virDomainMomentObjPtr *chk); + +int +qemuCheckpointCreateFinalize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, + virDomainMomentObjPtr chk, + bool update_current); -- 2.21.0

Introduce qemuMonitorTransactionBitmapMergeSourceAddBitmap which adds the appropriate entry into a virJSONValue array to be used with qemuMonitorTransactionBitmapMerge. Bitmap merging supports two possible formats and this new helper implements the more universal one specifying also the source node name. In addition use the new helper in the testQemuMonitorJSONTransaction test. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 9 +++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 +++++ tests/qemumonitorjsontest.c | 4 ++-- 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8de386817b..57229a68c0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4551,6 +4551,15 @@ qemuMonitorTransactionBitmapMerge(virJSONValuePtr actions, } +int +qemuMonitorTransactionBitmapMergeSourceAddBitmap(virJSONValuePtr sources, + const char *sourcenode, + const char *sourcebitmap) +{ + return qemuMonitorJSONTransactionBitmapMergeSourceAddBitmap(sources, sourcenode, sourcebitmap); +} + + int qemuMonitorTransactionSnapshotLegacy(virJSONValuePtr actions, const char *device, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 45f2a5a7d2..a1c980e40e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1364,6 +1364,10 @@ qemuMonitorTransactionBitmapMerge(virJSONValuePtr actions, const char *node, const char *target, virJSONValuePtr *sources); +int +qemuMonitorTransactionBitmapMergeSourceAddBitmap(virJSONValuePtr sources, + const char *sourcenode, + const char *sourcebitmap); int qemuMonitorTransactionSnapshotLegacy(virJSONValuePtr actions, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0062816e3e..545911069e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9199,6 +9199,27 @@ qemuMonitorJSONTransactionBitmapMerge(virJSONValuePtr actions, } +int +qemuMonitorJSONTransactionBitmapMergeSourceAddBitmap(virJSONValuePtr sources, + const char *sourcenode, + const char *sourcebitmap) +{ + g_autoptr(virJSONValue) sourceobj = NULL; + + if (virJSONValueObjectCreate(&sourceobj, + "s:node", sourcenode, + "s:name", sourcebitmap, + NULL) < 0) + return -1; + + if (virJSONValueArrayAppend(sources, sourceobj) < 0) + return -1; + + sourceobj = NULL; + return 0; +} + + int qemuMonitorJSONTransactionSnapshotLegacy(virJSONValuePtr actions, const char *device, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index d74ef91b83..6617783797 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -652,6 +652,11 @@ qemuMonitorJSONTransactionBitmapMerge(virJSONValuePtr actions, const char *target, virJSONValuePtr *sources); +int +qemuMonitorJSONTransactionBitmapMergeSourceAddBitmap(virJSONValuePtr sources, + const char *sourcenode, + const char *sourcebitmap); + int qemuMonitorJSONTransactionSnapshotLegacy(virJSONValuePtr actions, const char *device, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 4f15d231f9..cefa0f08cf 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2982,8 +2982,8 @@ testQemuMonitorJSONTransaction(const void *opaque) !(mergebitmaps = virJSONValueNewArray())) return -1; - if (virJSONValueArrayAppendString(mergebitmaps, "mergemap1") < 0 || - virJSONValueArrayAppendString(mergebitmaps, "mergemap2") < 0) + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, "node1", "bitmap1") < 0 || + qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, "node2", "bitmap2") < 0) return -1; if (qemuMonitorTransactionBitmapAdd(actions, "node1", "bitmap1", true, true) < 0 || -- 2.21.0

Use the new helper in qemuCheckpointDiscard rather than constructing the array manually. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 6a272f9dab..937d9d6b3e 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -173,7 +173,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver, if (!(arr = virJSONValueNewArray())) return -1; - if (virJSONValueArrayAppendString(arr, disk->bitmap) < 0) + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node, disk->bitmap) < 0) return -1; if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) { -- 2.21.0

Use only one switch case selecting job type and decide what's successful outcome on a case-by-case basis. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 81 ++++++++++++---------------------------- 1 file changed, 23 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index f9b3bdaff4..127a04e840 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1254,75 +1254,40 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob) { - switch ((qemuBlockjobState) job->newstate) { - case QEMU_BLOCKJOB_STATE_COMPLETED: - switch ((qemuBlockJobType) job->type) { - case QEMU_BLOCKJOB_TYPE_PULL: + bool success = job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED; + + switch ((qemuBlockJobType) job->type) { + case QEMU_BLOCKJOB_TYPE_PULL: + if (success) qemuBlockJobProcessEventCompletedPull(driver, vm, job, asyncJob); - break; + break; - case QEMU_BLOCKJOB_TYPE_COMMIT: + case QEMU_BLOCKJOB_TYPE_COMMIT: + if (success) qemuBlockJobProcessEventCompletedCommit(driver, vm, job, asyncJob); - break; - - case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: - qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, asyncJob); - break; - - case QEMU_BLOCKJOB_TYPE_CREATE: - qemuBlockJobProcessEventConcludedCreate(driver, vm, job, asyncJob); - break; - - case QEMU_BLOCKJOB_TYPE_COPY: - if (job->state == QEMU_BLOCKJOB_STATE_PIVOTING) - qemuBlockJobProcessEventConcludedCopyPivot(driver, vm, job, asyncJob); - else - qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob); - break; - - case QEMU_BLOCKJOB_TYPE_NONE: - case QEMU_BLOCKJOB_TYPE_INTERNAL: - case QEMU_BLOCKJOB_TYPE_LAST: - default: - break; - } break; - case QEMU_BLOCKJOB_STATE_FAILED: - case QEMU_BLOCKJOB_STATE_CANCELLED: - switch ((qemuBlockJobType) job->type) { - case QEMU_BLOCKJOB_TYPE_PULL: - case QEMU_BLOCKJOB_TYPE_COMMIT: - break; - - case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + if (success) + qemuBlockJobProcessEventCompletedActiveCommit(driver, vm, job, asyncJob); + else qemuBlockJobProcessEventFailedActiveCommit(driver, vm, job); - break; + break; - case QEMU_BLOCKJOB_TYPE_CREATE: - qemuBlockJobProcessEventConcludedCreate(driver, vm, job, asyncJob); - break; + case QEMU_BLOCKJOB_TYPE_CREATE: + qemuBlockJobProcessEventConcludedCreate(driver, vm, job, asyncJob); + break; - case QEMU_BLOCKJOB_TYPE_COPY: + case QEMU_BLOCKJOB_TYPE_COPY: + if (job->state == QEMU_BLOCKJOB_STATE_PIVOTING && success) + qemuBlockJobProcessEventConcludedCopyPivot(driver, vm, job, asyncJob); + else qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob); - break; - - case QEMU_BLOCKJOB_TYPE_NONE: - case QEMU_BLOCKJOB_TYPE_INTERNAL: - case QEMU_BLOCKJOB_TYPE_LAST: - default: - break; - } break; - /* states below are impossible in this handler */ - case QEMU_BLOCKJOB_STATE_READY: - case QEMU_BLOCKJOB_STATE_NEW: - case QEMU_BLOCKJOB_STATE_RUNNING: - case QEMU_BLOCKJOB_STATE_CONCLUDED: - case QEMU_BLOCKJOB_STATE_ABORTING: - case QEMU_BLOCKJOB_STATE_PIVOTING: - case QEMU_BLOCKJOB_STATE_LAST: + case QEMU_BLOCKJOB_TYPE_NONE: + case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_LAST: default: break; } -- 2.21.0

Prepare the function for addition of new members to clean. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 127a04e840..e12dcde729 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -82,8 +82,8 @@ qemuBlockJobDataDispose(void *obj) if (job->type == QEMU_BLOCKJOB_TYPE_CREATE) virObjectUnref(job->data.create.src); - VIR_FREE(job->name); - VIR_FREE(job->errmsg); + g_free(job->name); + g_free(job->errmsg); } -- 2.21.0

From: Eric Blake <eblake@redhat.com> Prepare for new backup APIs by describing the XML that will represent a backup. The XML resembles snapshots and checkpoints in being able to select actions for a set of disks, but has other differences. It can support both push model (the hypervisor does the backup directly into the destination file) and pull model (the hypervisor exposes an access port for a third party to grab what is necessary). Add testsuite coverage for some minimal uses of the XML. The <disk> element within <domainbackup> tries to model the same elements as a <disk> under <domain>, but sharing the RNG grammar proved to be hairy. That is in part because while <domain> use <source> to describe a host resource in use by the guest, a backup job is using a host resource that is not visible to the guest: a push backup action is instead describing a <target> (which ultimately could be a remote network resource, but for simplicity the RNG just validates a local file for now), and a pull backup action is instead describing a temporary local file <scratch> (which probably should not be a remote resource). A future refactoring may thus introduce some way to parameterize RNG to accept <disk type='FOO'>...</disk> so that the name of the subelement can be <source> for domain, or <target> or <scratch> as needed for backups. Future patches may improve this area of code. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/docs.html.in | 3 +- docs/format.html.in | 1 + docs/formatbackup.html.in | 168 ++++++++++++++ docs/formatcheckpoint.html.in | 12 +- docs/index.html.in | 3 +- docs/schemas/domainbackup.rng | 219 +++++++++++++++++++ libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + tests/Makefile.am | 2 + tests/domainbackupxml2xmlin/backup-pull.xml | 9 + tests/domainbackupxml2xmlin/backup-push.xml | 9 + tests/domainbackupxml2xmlin/empty.xml | 1 + tests/domainbackupxml2xmlout/backup-pull.xml | 9 + tests/domainbackupxml2xmlout/backup-push.xml | 9 + tests/domainbackupxml2xmlout/empty.xml | 7 + tests/virschematest.c | 2 + 16 files changed, 449 insertions(+), 8 deletions(-) create mode 100644 docs/formatbackup.html.in create mode 100644 docs/schemas/domainbackup.rng create mode 100644 tests/domainbackupxml2xmlin/backup-pull.xml create mode 100644 tests/domainbackupxml2xmlin/backup-push.xml create mode 100644 tests/domainbackupxml2xmlin/empty.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull.xml create mode 100644 tests/domainbackupxml2xmlout/backup-push.xml create mode 100644 tests/domainbackupxml2xmlout/empty.xml diff --git a/docs/docs.html.in b/docs/docs.html.in index 6cf09f51bc..6eb85b8d25 100644 --- a/docs/docs.html.in +++ b/docs/docs.html.in @@ -82,7 +82,8 @@ <a href="formatnode.html">node devices</a>, <a href="formatsecret.html">secrets</a>, <a href="formatsnapshot.html">snapshots</a>, - <a href="formatcheckpoint.html">checkpoints</a></dd> + <a href="formatcheckpoint.html">checkpoints</a>, + <a href="formatbackup.html">backup jobs</a></dd> <dt><a href="uri.html">URI format</a></dt> <dd>The URI formats used for connecting to libvirt</dd> diff --git a/docs/format.html.in b/docs/format.html.in index 3be2237663..d013528fe0 100644 --- a/docs/format.html.in +++ b/docs/format.html.in @@ -27,6 +27,7 @@ <li><a href="formatsecret.html">Secrets</a></li> <li><a href="formatsnapshot.html">Snapshots</a></li> <li><a href="formatcheckpoint.html">Checkpoints</a></li> + <li><a href="formatbackup.html">Backup jobs</a></li> </ul> <h2>Command line validation</h2> diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in new file mode 100644 index 0000000000..272d9cf6ab --- /dev/null +++ b/docs/formatbackup.html.in @@ -0,0 +1,168 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html> +<html xmlns="http://www.w3.org/1999/xhtml"> + <body> + <h1>Backup XML format</h1> + + <ul id="toc"></ul> + + <h2><a id="BackupAttributes">Backup XML</a></h2> + + <p> + Creating a backup, whether full or incremental, is done + via <code>virDomainBackupBegin()</code>, which takes an XML + description of the actions to perform, as well as an optional + second XML document <a href="formatcheckpoint.html">describing a + checkpoint</a> to create at the same point in time. See + also <a href="domainstatecapture.html">a comparison</a> between + the various state capture APIs. + </p> + <p> + There are two general modes for backups: a push mode (where the + hypervisor writes out the data to the destination file, which + may be local or remote), and a pull mode (where the hypervisor + creates an NBD server that a third-party client can then read as + needed, and which requires the use of temporary storage, + typically local, until the backup is complete). + </p> + <p> + The instructions for beginning a backup job are provided as + attributes and elements of the + top-level <code>domainbackup</code> element. This element + includes an optional attribute <code>mode</code> which can be + either "push" or "pull" (default + push). <code>virDomainBackupGetXMLDesc()</code> can be used to + see the actual values selected for elements omitted during + creation (for example, learning which port the NBD server is + using in the pull model or what file names libvirt generated + when none were supplied). The following child elements and attributes + are supported: + </p> + <dl> + <dt>attribute <code>id</code></dt> + <dd>Ignored on input, this element is the job id of the backup + operation returned on success + from <code>virDomainBackupBegin()</code>, and is used for + selecting which backup operation to target + during <code>virDomainBackupGetXMLDesc()</code> + and <code>virDomainBackupEnd()</code>. </dd> + <dt><code>incremental</code></dt> + <dd>An optional element giving the name of an existing + checkpoint of the domain, which will be used to make this + backup an incremental one. In the push model, only changes + since the named checkpoint are written to the destination. In + the pull model, the NBD server uses the + NBD_OPT_SET_META_CONTEXT extension to advertise to the client + which portions of the export contain changes since the named + checkpoint. If omitted, a full backup is performed. + </dd> + <dt><code>server</code></dt> + <dd>Present only for a pull mode backup. Contains the same + attributes as + the <a href="formatdomain.html#elementsDisks"><code>protocol</code> + element of a disk</a> attached via NBD in the domain (such as + transport, socket, name, port, or tls), necessary to set up an + NBD server that exposes the content of each disk at the time + the backup is started. + </dd> + <dt><code>disks</code></dt> + <dd>An optional listing of instructions for disks participating + in the backup (if omitted, all disks participate and libvirt + attempts to generate filenames by appending the current + timestamp as a suffix). If the entire element was omitted on + input, then all disks participate in the backup, otherwise, + only the disks explicitly listed which do not also + use <code>backup='no'</code> will participate. On output, this + is the state of each of the domain's disk in relation to the + backup operation. + <dl> + <dt><code>disk</code></dt> + <dd>This sub-element describes the backup properties of a + specific disk, with the following attributes and child + elements: + <dl> + <dt><code>name</code></dt> + <dd>A mandatory attribute which must match + the <code><target dev='name'/></code> + of one of + the <a href="formatdomain.html#elementsDisks">disk + devices</a> specified for the domain at the time of + the checkpoint.</dd> + <dt><code>backup</code></dt> + <dd>Setting this attribute to <code>yes</code>(default) specifies + that the disk should take part in the backup and using + <code>no</code> excludes the disk from the backup.</dd> + <dt><code>type</code></dt> + <dd>A mandatory attribute to describe the type of the + disk, except when <code>backup='no'</code> is + used. Valid values include <code>file</code>, + <code>block</code>, or <code>network</code>. + Similar to a disk declaration for a domain, the choice of type + controls what additional sub-elements are needed to describe + the destination (such as <code>protocol</code> for a + network destination).</dd> + <dt><code>target</code></dt> + <dd>Valid only for push mode backups, this is the + primary sub-element that describes the file name of + the backup destination, similar to + the <code>source</code> sub-element of a domain + disk. An optional sub-element <code>driver</code> can + also be used, with an attribute <code>type</code> to + specify a destination format different from + qcow2. </dd> + <dt><code>scratch</code></dt> + <dd>Valid only for pull mode backups, this is the + primary sub-element that describes the file name of + the local scratch file to be used in facilitating the + backup, and is similar to the <code>source</code> + sub-element of a domain disk.</dd> + </dl> + </dd> + </dl> + </dd> + </dl> + + <h2><a id="example">Examples</a></h2> + + <p>Use <code>virDomainBackupBegin()</code> to perform a full + backup using push mode. The example lets libvirt pick the + destination and format for 'vda', fully specifies that we want a + raw backup of 'vdb', and omits 'vdc' from the operation. + </p> + <pre> +<domainbackup> + <disks/> + <disk name='vda' backup='yes'/> + <disk name='vdb' type='file'> + <target file='/path/to/vdb.backup'/> + <driver type='raw'/> + </disk> + <disk name='vdc' backup='no'/> + </disks/> +</domainbackup> + </pre> + + <p>If the previous full backup also passed a parameter describing + <a href="formatcheckpoint.html">checkpoint XML</a> that resulted + in a checkpoint named <code>1525889631</code>, we can make + another call to <code>virDomainBackupBegin()</code> to perform + an incremental backup of just the data changed since that + checkpoint, this time using the following XML to start a pull + model export of the 'vda' and 'vdb' disks, where a third-party + NBD client connecting to '/path/to/server' completes the backup + (omitting 'vdc' from the explicit list has the same effect as + the backup='no' from the previous example): + </p> + <pre> +<domainbackup mode="pull"> + <incremental>1525889631</incremental> + <server transport="unix" socket="/path/to/server"/> + <disks/> + <disk name='vda' backup='yes' type='file'> + <scratch file='/path/to/file1.scratch'/> + </disk> + </disks/> +</domainbackup> + </pre> + </body> +</html> diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in index 044bbfe4b0..ee56194523 100644 --- a/docs/formatcheckpoint.html.in +++ b/docs/formatcheckpoint.html.in @@ -28,12 +28,12 @@ first checkpoint and the second backup operation), it is possible to do an offline reconstruction of the state of the disk at the time of the second backup without having to copy as - much data as a second full backup would require. Future API - additions will make it possible to create checkpoints in - conjunction with a backup - via <code>virDomainBackupBegin()</code> or with an external - snapshot via <code>virDomainSnapshotCreateXML2</code>; but for - now, libvirt exposes enough support to create disk checkpoints + much data as a second full backup would require. Most disk + checkpoints are created in conjunction with a backup + via <code>virDomainBackupBegin()</code>, although a future API + addition of <code>virDomainSnapshotCreateXML2()</code> will also + make this possible when creating external snapshots; however, + libvirt also exposes enough support to create disk checkpoints independently from a backup operation via <code>virDomainCheckpointCreateXML()</code> <span class="since">since 5.6.0</span>. Likewise, the creation of checkpoints when diff --git a/docs/index.html.in b/docs/index.html.in index 7d0ab650e3..26e8406917 100644 --- a/docs/index.html.in +++ b/docs/index.html.in @@ -59,7 +59,8 @@ <a href="formatnode.html">node devices</a>, <a href="formatsecret.html">secrets</a>, <a href="formatsnapshot.html">snapshots</a>, - <a href="formatcheckpoint.html">checkpoints</a></dd> + <a href="formatcheckpoint.html">checkpoints</a>, + <a href="formatbackup.html">backup jobs</a></dd> <dt><a href="http://wiki.libvirt.org">Wiki</a></dt> <dd>Read further community contributed content</dd> </dl> diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng new file mode 100644 index 0000000000..92327e7077 --- /dev/null +++ b/docs/schemas/domainbackup.rng @@ -0,0 +1,219 @@ +<?xml version="1.0"?> +<!-- A Relax NG schema for the libvirt domain backup properties XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0"> + <start> + <ref name='domainbackup'/> + </start> + + <include href='domaincommon.rng'/> + + <define name='domainbackup'> + <element name='domainbackup'> + <optional> + <attribute name='id'> + <ref name="unsignedInt"/> + </attribute> + </optional> + <interleave> + <optional> + <element name='incremental'> + <text/> + </element> + </optional> + <choice> + <group> + <optional> + <attribute name='mode'> + <value>push</value> + </attribute> + </optional> + <ref name='backupDisksPush'/> + </group> + <group> + <attribute name='mode'> + <value>pull</value> + </attribute> + <interleave> + <element name='server'> + <choice> + <group> + <optional> + <attribute name='transport'> + <value>tcp</value> + </attribute> + </optional> + <attribute name='name'> + <choice> + <ref name='dnsName'/> + <ref name='ipAddr'/> + </choice> + </attribute> + <optional> + <attribute name='port'> + <ref name='unsignedInt'/> + </attribute> + </optional> + <!-- add tls? --> + </group> + <group> + <attribute name='transport'> + <value>unix</value> + </attribute> + <attribute name='socket'> + <ref name='absFilePath'/> + </attribute> + </group> + </choice> + </element> + <ref name='backupDisksPull'/> + </interleave> + </group> + </choice> + </interleave> + </element> + </define> + + <define name='backupPushDriver'> + <optional> + <element name='driver'> + <attribute name='type'> + <ref name='storageFormat'/> + </attribute> + </element> + </optional> + </define> + + <define name='backupAttr'> + <!-- valid values of <disk backup='XXX'> other than 'no' --> + <optional> + <attribute name='backup'> + <choice> + <value>begin</value> + <value>inprogress</value> + <value>ready</value> + </choice> + </attribute> + </optional> + </define> + + <define name='backupDisksPush'> + <optional> + <element name='disks'> + <oneOrMore> + <element name='disk'> + <attribute name='name'> + <choice> + <ref name='diskTarget'/> + <ref name='absFilePath'/> + </choice> + </attribute> + <optional> + <attribute name='shallow'> + <value>on</value> + </attribute> + </optional> + <choice> + <group> + <attribute name='backup'> + <value>no</value> + </attribute> + </group> + <!-- FIXME allow push to a network location, perhaps by + refactoring 'diskSource' to take element name by a + per-grammar ref --> + <group> + <ref name='backupAttr'/> + <optional> + <attribute name='type'> + <value>file</value> + </attribute> + </optional> + <interleave> + <optional> + <element name='target'> + <attribute name='file'> + <ref name='absFilePath'/> + </attribute> + </element> + </optional> + <ref name='backupPushDriver'/> + </interleave> + </group> + <group> + <ref name='backupAttr'/> + <attribute name='type'> + <value>block</value> + </attribute> + <interleave> + <optional> + <element name='target'> + <attribute name='dev'> + <ref name='absFilePath'/> + </attribute> + </element> + </optional> + <ref name='backupPushDriver'/> + </interleave> + </group> + <!-- + <group> + <ref name='backupAttr'/> + <attribute name='type'> + <value>network</value> + </attribute> + ... + </group> + --> + </choice> + </element> + </oneOrMore> + </element> + </optional> + </define> + + <define name='backupDisksPull'> + <optional> + <element name='disks'> + <oneOrMore> + <element name='disk'> + <attribute name='name'> + <choice> + <ref name='diskTarget'/> + <ref name='absFilePath'/> + </choice> + </attribute> + <choice> + <group> + <optional> + <attribute name='type'> + <value>file</value> + </attribute> + </optional> + <optional> + <element name='scratch'> + <attribute name='file'> + <ref name='absFilePath'/> + </attribute> + </element> + </optional> + </group> + <group> + <attribute name='type'> + <value>block</value> + </attribute> + <optional> + <element name='scratch'> + <attribute name='dev'> + <ref name='absFilePath'/> + </attribute> + </element> + </optional> + </group> + </choice> + </element> + </oneOrMore> + </element> + </optional> + </define> + +</grammar> diff --git a/libvirt.spec.in b/libvirt.spec.in index dcad08cb5f..0565992907 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1900,6 +1900,7 @@ exit 0 %{_datadir}/libvirt/schemas/capability.rng %{_datadir}/libvirt/schemas/cputypes.rng %{_datadir}/libvirt/schemas/domain.rng +%{_datadir}/libvirt/schemas/domainbackup.rng %{_datadir}/libvirt/schemas/domaincaps.rng %{_datadir}/libvirt/schemas/domaincheckpoint.rng %{_datadir}/libvirt/schemas/domaincommon.rng diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index c29f3eeed2..453570fc3c 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -233,6 +233,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw32_datadir}/libvirt/schemas/capability.rng %{mingw32_datadir}/libvirt/schemas/cputypes.rng %{mingw32_datadir}/libvirt/schemas/domain.rng +%{mingw32_datadir}/libvirt/schemas/domainbackup.rng %{mingw32_datadir}/libvirt/schemas/domaincaps.rng %{mingw32_datadir}/libvirt/schemas/domaincheckpoint.rng %{mingw32_datadir}/libvirt/schemas/domaincommon.rng @@ -324,6 +325,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh %{mingw64_datadir}/libvirt/schemas/capability.rng %{mingw64_datadir}/libvirt/schemas/cputypes.rng %{mingw64_datadir}/libvirt/schemas/domain.rng +%{mingw64_datadir}/libvirt/schemas/domainbackup.rng %{mingw64_datadir}/libvirt/schemas/domaincaps.rng %{mingw64_datadir}/libvirt/schemas/domaincheckpoint.rng %{mingw64_datadir}/libvirt/schemas/domaincommon.rng diff --git a/tests/Makefile.am b/tests/Makefile.am index a9acd88670..d7459de9b1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -89,6 +89,8 @@ EXTRA_DIST = \ capabilityschemadata \ commanddata \ cputestdata \ + domainbackupxml2xmlin \ + domainbackupxml2xmlout \ domaincapsschemadata \ domainconfdata \ domainschemadata \ diff --git a/tests/domainbackupxml2xmlin/backup-pull.xml b/tests/domainbackupxml2xmlin/backup-pull.xml new file mode 100644 index 0000000000..2ce5cd6711 --- /dev/null +++ b/tests/domainbackupxml2xmlin/backup-pull.xml @@ -0,0 +1,9 @@ +<domainbackup mode="pull"> + <incremental>1525889631</incremental> + <server transport='tcp' name='localhost' port='10809'/> + <disks> + <disk name='vda' type='file'> + <scratch file='/path/to/file'/> + </disk> + </disks> +</domainbackup> diff --git a/tests/domainbackupxml2xmlin/backup-push.xml b/tests/domainbackupxml2xmlin/backup-push.xml new file mode 100644 index 0000000000..1b7d3061fd --- /dev/null +++ b/tests/domainbackupxml2xmlin/backup-push.xml @@ -0,0 +1,9 @@ +<domainbackup mode="push"> + <incremental>1525889631</incremental> + <disks> + <disk name='vda' type='file'> + <driver type='raw'/> + <target file='/path/to/file'/> + </disk> + </disks> +</domainbackup> diff --git a/tests/domainbackupxml2xmlin/empty.xml b/tests/domainbackupxml2xmlin/empty.xml new file mode 100644 index 0000000000..7ed511f97b --- /dev/null +++ b/tests/domainbackupxml2xmlin/empty.xml @@ -0,0 +1 @@ +<domainbackup/> diff --git a/tests/domainbackupxml2xmlout/backup-pull.xml b/tests/domainbackupxml2xmlout/backup-pull.xml new file mode 100644 index 0000000000..2ce5cd6711 --- /dev/null +++ b/tests/domainbackupxml2xmlout/backup-pull.xml @@ -0,0 +1,9 @@ +<domainbackup mode="pull"> + <incremental>1525889631</incremental> + <server transport='tcp' name='localhost' port='10809'/> + <disks> + <disk name='vda' type='file'> + <scratch file='/path/to/file'/> + </disk> + </disks> +</domainbackup> diff --git a/tests/domainbackupxml2xmlout/backup-push.xml b/tests/domainbackupxml2xmlout/backup-push.xml new file mode 100644 index 0000000000..1b7d3061fd --- /dev/null +++ b/tests/domainbackupxml2xmlout/backup-push.xml @@ -0,0 +1,9 @@ +<domainbackup mode="push"> + <incremental>1525889631</incremental> + <disks> + <disk name='vda' type='file'> + <driver type='raw'/> + <target file='/path/to/file'/> + </disk> + </disks> +</domainbackup> diff --git a/tests/domainbackupxml2xmlout/empty.xml b/tests/domainbackupxml2xmlout/empty.xml new file mode 100644 index 0000000000..13600fbb1c --- /dev/null +++ b/tests/domainbackupxml2xmlout/empty.xml @@ -0,0 +1,7 @@ +<domainbackup mode="push"> + <disks> + <disk name="vda" type="file"> + <target file="/path/to/file1.copy"/> + </disk> + </disks> +</domainbackup> diff --git a/tests/virschematest.c b/tests/virschematest.c index dabbc02163..330c42b010 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -221,6 +221,8 @@ mymain(void) "lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata", "genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata", "qemuhotplugtestdomains"); + DO_TEST_DIR("domainbackup.rng", "domainbackupxml2xmlin", + "domainbackupxml2xmlout"); DO_TEST_DIR("domaincaps.rng", "domaincapsschemadata"); DO_TEST_DIR("domaincheckpoint.rng", "qemudomaincheckpointxml2xmlin", "qemudomaincheckpointxml2xmlout"); -- 2.21.0

From: Eric Blake <eblake@redhat.com> Accept XML describing a generic block job, and output it again as needed. This may still need a few tweaks to match the documented XML and RNG schema. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/Makefile.inc.am | 2 + src/conf/backup_conf.c | 518 +++++++++++++++++++++++++++++++++++++++ src/conf/backup_conf.h | 102 ++++++++ src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 8 + 5 files changed, 633 insertions(+) create mode 100644 src/conf/backup_conf.c create mode 100644 src/conf/backup_conf.h diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 5035b9b524..debc6f4eef 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -12,6 +12,8 @@ NETDEV_CONF_SOURCES = \ $(NULL) DOMAIN_CONF_SOURCES = \ + conf/backup_conf.c \ + conf/backup_conf.h \ conf/capabilities.c \ conf/capabilities.h \ conf/checkpoint_conf.c \ diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c new file mode 100644 index 0000000000..123e71d4d2 --- /dev/null +++ b/src/conf/backup_conf.c @@ -0,0 +1,518 @@ +/* + * backup_conf.c: domain backup XML processing + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "configmake.h" +#include "internal.h" +#include "virbuffer.h" +#include "datatypes.h" +#include "domain_conf.h" +#include "virlog.h" +#include "viralloc.h" +#include "backup_conf.h" +#include "virstoragefile.h" +#include "viruuid.h" +#include "virfile.h" +#include "virerror.h" +#include "virxml.h" +#include "virstring.h" +#include "virhash.h" +#include "virenum.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN + +VIR_LOG_INIT("conf.backup_conf"); + +VIR_ENUM_DECL(virDomainBackup); +VIR_ENUM_IMPL(virDomainBackup, + VIR_DOMAIN_BACKUP_TYPE_LAST, + "default", + "push", + "pull"); + +/* following values appear in the status XML */ +VIR_ENUM_DECL(virDomainBackupDiskState); +VIR_ENUM_IMPL(virDomainBackupDiskState, + VIR_DOMAIN_BACKUP_DISK_STATE_LAST, + "", + "running", + "complete", + "failed", + "cancelling", + "cancelled"); + +void +virDomainBackupDefFree(virDomainBackupDefPtr def) +{ + size_t i; + + if (!def) + return; + + g_free(def->incremental); + virStorageNetHostDefFree(1, def->server); + + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDefPtr disk = def->disks + i; + + g_free(disk->name); + virObjectUnref(disk->store); + } + + g_free(def->disks); + g_free(def); +} + + +static int +virDomainBackupDiskDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainBackupDiskDefPtr def, + bool push, + unsigned int flags, + virDomainXMLOptionPtr xmlopt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + g_autofree char *type = NULL; + g_autofree char *driver = NULL; + g_autofree char *backup = NULL; + g_autofree char *state = NULL; + int tmp; + xmlNodePtr srcNode; + unsigned int storageSourceParseFlags = 0; + bool internal = flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL; + + if (internal) + storageSourceParseFlags = VIR_DOMAIN_DEF_PARSE_STATUS; + + ctxt->node = node; + + if (!(def->name = virXMLPropString(node, "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing name from disk backup element")); + return -1; + } + + def->backup = VIR_TRISTATE_BOOL_YES; + + if ((backup = virXMLPropString(node, "backup"))) { + if ((tmp = virTristateBoolTypeFromString(backup)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid disk 'backup' state '%s'"), backup); + return -1; + } + + def->backup = tmp; + } + + /* don't parse anything else if backup is disabled */ + if (def->backup == VIR_TRISTATE_BOOL_NO) + return 0; + + if (internal) { + tmp = 0; + if (!(state = virXMLPropString(node, "state")) || + (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("disk '%s' backup state wrong or missing'"), def->name); + return -1; + } + + def->state = tmp; + } + + if (!(def->store = virStorageSourceNew())) + return -1; + + if ((type = virXMLPropString(node, "type"))) { + if ((def->store->type = virStorageTypeFromString(type)) <= 0 || + def->store->type == VIR_STORAGE_TYPE_VOLUME || + def->store->type == VIR_STORAGE_TYPE_DIR) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk backup type '%s'"), type); + return -1; + } + } else { + def->store->type = VIR_STORAGE_TYPE_FILE; + } + + if (push) + srcNode = virXPathNode("./target", ctxt); + else + srcNode = virXPathNode("./scratch", ctxt); + + if (srcNode && + virDomainStorageSourceParse(srcNode, ctxt, def->store, + storageSourceParseFlags, xmlopt) < 0) + return -1; + + if ((driver = virXPathString("string(./driver/@type)", ctxt))) { + def->store->format = virStorageFileFormatTypeFromString(driver); + if (def->store->format <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk backup driver '%s'"), driver); + return -1; + } else if (!push && def->store->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("pull mode requires qcow2 driver, not '%s'"), + driver); + return -1; + } + } + + return 0; +} + + +static virDomainBackupDefPtr +virDomainBackupDefParse(xmlXPathContextPtr ctxt, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + g_autoptr(virDomainBackupDef) def = NULL; + g_autofree xmlNodePtr *nodes = NULL; + xmlNodePtr node = NULL; + g_autofree char *mode = NULL; + bool push; + size_t i; + int n; + + def = g_new0(virDomainBackupDef, 1); + + def->type = VIR_DOMAIN_BACKUP_TYPE_PUSH; + + if ((mode = virXMLPropString(ctxt->node, "mode"))) { + if ((def->type = virDomainBackupTypeFromString(mode)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown backup mode '%s'"), mode); + return NULL; + } + } + + push = def->type == VIR_DOMAIN_BACKUP_TYPE_PUSH; + + if (flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL) { + g_autofree char *tmp = virXMLPropString(ctxt->node, "id"); + if (tmp && virStrToLong_i(tmp, NULL, 10, &def->id) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid 'id' value '%s'"), tmp); + return NULL; + } + } + + def->incremental = virXPathString("string(./incremental)", ctxt); + + if ((node = virXPathNode("./server", ctxt))) { + if (def->type != VIR_DOMAIN_BACKUP_TYPE_PULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("use of <server> requires pull mode backup")); + return NULL; + } + + def->server = g_new0(virStorageNetHostDef, 1); + + if (virDomainStorageNetworkParseHost(node, def->server) < 0) + return NULL; + + if (def->server->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transport rdma is not supported for <server>")); + return NULL; + } + + if (def->server->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX && + def->server->socket[0] != '/') { + virReportError(VIR_ERR_XML_ERROR, + _("backup socket path '%s' must be absolute"), + def->server->socket); + return NULL; + } + } + + if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0) + return NULL; + + def->disks = g_new0(virDomainBackupDiskDef, n); + + def->ndisks = n; + for (i = 0; i < def->ndisks; i++) { + if (virDomainBackupDiskDefParseXML(nodes[i], ctxt, + &def->disks[i], push, + flags, xmlopt) < 0) + return NULL; + } + + return g_steal_pointer(&def); +} + + +virDomainBackupDefPtr +virDomainBackupDefParseString(const char *xmlStr, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + virDomainBackupDefPtr ret = NULL; + g_autoptr(xmlDoc) xml = NULL; + int keepBlanksDefault = xmlKeepBlanksDefault(0); + + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)")))) { + xmlKeepBlanksDefault(keepBlanksDefault); + ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), + xmlopt, flags); + } + xmlKeepBlanksDefault(keepBlanksDefault); + + return ret; +} + + +virDomainBackupDefPtr +virDomainBackupDefParseNode(xmlDocPtr xml, + xmlNodePtr root, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree char *schema = NULL; + + if (!virXMLNodeNameEqual(root, "domainbackup")) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("domainbackup")); + return NULL; + } + + if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) { + if (!(schema = virFileFindResource("domainbackup.rng", + abs_top_srcdir "/docs/schemas", + PKGDATADIR "/schemas"))) + return NULL; + + if (virXMLValidateAgainstSchema(schema, xml) < 0) + return NULL; + } + + if (!(ctxt = virXMLXPathContextNew(xml))) + return NULL; + + ctxt->node = root; + return virDomainBackupDefParse(ctxt, xmlopt, flags); +} + + +static int +virDomainBackupDiskDefFormat(virBufferPtr buf, + virDomainBackupDiskDefPtr disk, + bool push, + bool internal) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + const char *sourcename = "scratch"; + unsigned int storageSourceFormatFlags = 0; + + if (push) + sourcename = "target"; + + if (internal) + storageSourceFormatFlags |= VIR_DOMAIN_DEF_FORMAT_STATUS; + + virBufferSetChildIndent(&childBuf, buf); + + virBufferEscapeString(&attrBuf, " name='%s'", disk->name); + virBufferAsprintf(&attrBuf, " backup='%s'", virTristateBoolTypeToString(disk->backup)); + if (internal) + virBufferAsprintf(&attrBuf, " state='%s'", virDomainBackupDiskStateTypeToString(disk->state)); + + if (disk->backup == VIR_TRISTATE_BOOL_YES) { + virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->store->type)); + + if (disk->store->format > 0) + virBufferEscapeString(&childBuf, "<driver type='%s'/>\n", + virStorageFileFormatTypeToString(disk->store->format)); + + if (virDomainDiskSourceFormat(&childBuf, disk->store, sourcename, + 0, false, storageSourceFormatFlags, NULL) < 0) + return -1; + } + + return virXMLFormatElement(buf, "disk", &attrBuf, &childBuf); +} + + +int +virDomainBackupDefFormat(virBufferPtr buf, + virDomainBackupDefPtr def, + bool internal) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) serverAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) disksChildBuf = VIR_BUFFER_INITIALIZER; + size_t i; + + virBufferSetChildIndent(&childBuf, buf); + virBufferSetChildIndent(&disksChildBuf, &childBuf); + + virBufferAsprintf(&attrBuf, " mode='%s'", virDomainBackupTypeToString(def->type)); + virBufferAsprintf(&attrBuf, " id='%d'", def->id); + + virBufferEscapeString(&childBuf, "<incremental>%s</incremental>\n", def->incremental); + + if (def->server) { + virBufferAsprintf(&serverAttrBuf, " transport='%s'", + virStorageNetHostTransportTypeToString(def->server->transport)); + virBufferEscapeString(&serverAttrBuf, " name='%s'", def->server->name); + if (def->server->port) + virBufferAsprintf(&serverAttrBuf, " port='%u'", def->server->port); + virBufferEscapeString(&serverAttrBuf, " socket='%s'", def->server->socket); + } + + if (virXMLFormatElement(&childBuf, "server", &serverAttrBuf, NULL) < 0) + return -1; + + for (i = 0; i < def->ndisks; i++) { + if (virDomainBackupDiskDefFormat(&disksChildBuf, &def->disks[i], + def->type == VIR_DOMAIN_BACKUP_TYPE_PUSH, + internal) < 0) + return -1; + } + + if (virXMLFormatElement(&childBuf, "disks", NULL, &disksChildBuf) < 0) + return -1; + + if (virXMLFormatElement(buf, "domainbackup", &attrBuf, &childBuf) < 0) + return -1; + + return 0; +} + + +static int +virDomainBackupDefAssignStore(virDomainBackupDiskDefPtr disk, + virStorageSourcePtr src, + const char *suffix) +{ + if (virStorageSourceIsEmpty(src)) { + if (disk->store) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' has no media"), disk->name); + return -1; + } + } else if (src->readonly) { + if (disk->store) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("backup of readonly disk '%s' makes no sense"), + disk->name); + return -1; + } + } else if (!disk->store) { + if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_FILE) { + if (!(disk->store = virStorageSourceNew())) + return -1; + + disk->store->type = VIR_STORAGE_TYPE_FILE; + disk->store->path = g_strdup_printf("%s.%s", src->path, suffix); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("refusing to generate file name for disk '%s'"), + disk->name); + return -1; + } + } + + return 0; +} + + +int +virDomainBackupAlignDisks(virDomainBackupDefPtr def, + virDomainDefPtr dom, + const char *suffix) +{ + g_autoptr(virHashTable) disks = NULL; + size_t i; + int ndisks; + bool backup_all = false; + + + if (!(disks = virHashNew(NULL))) + return -1; + + /* Unlikely to have a guest without disks but technically possible. */ + if (!dom->ndisks) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain must have at least one disk to perform backup")); + return -1; + } + + /* Double check requested disks. */ + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDefPtr backupdisk = &def->disks[i]; + virDomainDiskDefPtr domdisk; + + if (!(domdisk = virDomainDiskByTarget(dom, backupdisk->name))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("no disk named '%s'"), backupdisk->name); + return -1; + } + + if (virHashAddEntry(disks, backupdisk->name, NULL) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' specified twice"), + backupdisk->name); + return -1; + } + + if (backupdisk->backup == VIR_TRISTATE_BOOL_YES && + virDomainBackupDefAssignStore(backupdisk, domdisk->src, suffix) < 0) + return -1; + } + + if (def->ndisks == 0) + backup_all = true; + + ndisks = def->ndisks; + if (VIR_EXPAND_N(def->disks, def->ndisks, dom->ndisks - def->ndisks) < 0) + return -1; + + for (i = 0; i < dom->ndisks; i++) { + virDomainBackupDiskDefPtr backupdisk = NULL; + virDomainDiskDefPtr domdisk = dom->disks[i]; + + if (virHashHasEntry(disks, domdisk->dst)) + continue; + + backupdisk = &def->disks[ndisks++]; + + if (VIR_STRDUP(backupdisk->name, domdisk->dst) < 0) + return -1; + + if (backup_all && + !virStorageSourceIsEmpty(domdisk->src) && + !domdisk->src->readonly) { + backupdisk->backup = VIR_TRISTATE_BOOL_YES; + + if (virDomainBackupDefAssignStore(backupdisk, domdisk->src, suffix) < 0) + return -1; + } else { + backupdisk->backup = VIR_TRISTATE_BOOL_NO; + } + } + + return 0; +} diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h new file mode 100644 index 0000000000..3be594b7da --- /dev/null +++ b/src/conf/backup_conf.h @@ -0,0 +1,102 @@ +/* + * backup_conf.h: domain backup XML processing + * (based on domain_conf.h) + * + * Copyright (C) 2006-2019 Red Hat, Inc. + * Copyright (C) 2006-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "internal.h" +#include "virconftypes.h" + +/* Items related to incremental backup state */ + +typedef enum { + VIR_DOMAIN_BACKUP_TYPE_DEFAULT = 0, + VIR_DOMAIN_BACKUP_TYPE_PUSH, + VIR_DOMAIN_BACKUP_TYPE_PULL, + + VIR_DOMAIN_BACKUP_TYPE_LAST +} virDomainBackupType; + +typedef enum { + VIR_DOMAIN_BACKUP_DISK_STATE_NONE = 0, + VIR_DOMAIN_BACKUP_DISK_STATE_RUNNING, + VIR_DOMAIN_BACKUP_DISK_STATE_COMPLETE, + VIR_DOMAIN_BACKUP_DISK_STATE_FAILED, + VIR_DOMAIN_BACKUP_DISK_STATE_CANCELLING, + VIR_DOMAIN_BACKUP_DISK_STATE_CANCELLED, + VIR_DOMAIN_BACKUP_DISK_STATE_LAST +} virDomainBackupDiskState; + +/* Stores disk-backup information */ +typedef struct _virDomainBackupDiskDef virDomainBackupDiskDef; +typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr; +struct _virDomainBackupDiskDef { + char *name; /* name matching the <target dev='...' of the domain */ + virTristateBool backup; /* whether backup is requested */ + + /* details of target for push-mode, or of the scratch file for pull-mode */ + virStorageSourcePtr store; + + /* internal data */ + virDomainBackupDiskState state; +}; + +/* Stores the complete backup metadata */ +typedef struct _virDomainBackupDef virDomainBackupDef; +typedef virDomainBackupDef *virDomainBackupDefPtr; +struct _virDomainBackupDef { + /* Public XML. */ + int type; /* virDomainBackupType */ + int id; + char *incremental; + virStorageNetHostDefPtr server; /* only when type == PULL */ + + size_t ndisks; /* should not exceed dom->ndisks */ + virDomainBackupDiskDef *disks; +}; + +typedef enum { + VIR_DOMAIN_BACKUP_PARSE_INTERNAL = 1 << 0, +} virDomainBackupParseFlags; + +virDomainBackupDefPtr +virDomainBackupDefParseString(const char *xmlStr, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + +virDomainBackupDefPtr +virDomainBackupDefParseNode(xmlDocPtr xml, + xmlNodePtr root, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); +void +virDomainBackupDefFree(virDomainBackupDefPtr def); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainBackupDef, virDomainBackupDefFree); + +int +virDomainBackupDefFormat(virBufferPtr buf, + virDomainBackupDefPtr def, + bool internal); +int +virDomainBackupAlignDisks(virDomainBackupDefPtr backup, + virDomainDefPtr dom, + const char *suffix); diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 462842f324..9ed9b68b65 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -93,6 +93,9 @@ typedef virDomainABIStability *virDomainABIStabilityPtr; typedef struct _virDomainActualNetDef virDomainActualNetDef; typedef virDomainActualNetDef *virDomainActualNetDefPtr; +typedef struct _virDomainBackupDef virDomainBackupDef; +typedef virDomainBackupDef *virDomainBackupDefPtr; + typedef struct _virDomainBIOSDef virDomainBIOSDef; typedef virDomainBIOSDef *virDomainBIOSDefPtr; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fdfe1e7295..b4deab520e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -42,6 +42,14 @@ virAccessPermStorageVolTypeFromString; virAccessPermStorageVolTypeToString; +# conf/backup_conf.h +virDomainBackupAlignDisks; +virDomainBackupDefFormat; +virDomainBackupDefFree; +virDomainBackupDefParseNode; +virDomainBackupDefParseString; + + # conf/capabilities.h virCapabilitiesAddGuest; virCapabilitiesAddGuestDomain; -- 2.21.0

From: Eric Blake <eblake@redhat.com> Introduce virsh commands for performing backup jobs. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/Makefile.am | 1 + tools/virsh-backup.c | 209 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh-backup.h | 21 +++++ tools/virsh.c | 2 + tools/virsh.h | 1 + tools/virsh.pod | 37 ++++++++ 6 files changed, 271 insertions(+) create mode 100644 tools/virsh-backup.c create mode 100644 tools/virsh-backup.h diff --git a/tools/Makefile.am b/tools/Makefile.am index 68320c7246..813cfaeb53 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -233,6 +233,7 @@ virt_login_shell_helper_CFLAGS = \ virsh_SOURCES = \ virsh.c virsh.h \ + virsh-backup.c virsh-backup.h\ virsh-checkpoint.c virsh-checkpoint.h \ virsh-completer.c virsh-completer.h \ virsh-completer-domain.c virsh-completer-domain.h \ diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c new file mode 100644 index 0000000000..7719b4f3cc --- /dev/null +++ b/tools/virsh-backup.c @@ -0,0 +1,209 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> +#include "virsh-backup.h" +#include "virsh-util.h" + +#include "internal.h" +#include "virfile.h" + +/* + * "backup-begin" command + */ +static const vshCmdInfo info_backup_begin[] = { + {.name = "help", + .data = N_("Start a disk backup of a live domain") + }, + {.name = "desc", + .data = N_("Use XML to start a full or incremental disk backup of a live " + "domain, optionally creating a checkpoint") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_backup_begin[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "backupxml", + .type = VSH_OT_STRING, + .help = N_("domain backup XML"), + }, + {.name = "checkpointxml", + .type = VSH_OT_STRING, + .help = N_("domain checkpoint XML"), + }, + {.name = NULL} +}; + +static bool +cmdBackupBegin(vshControl *ctl, + const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + const char *backup_from = NULL; + g_autofree char *backup_buffer = NULL; + const char *check_from = NULL; + g_autofree char *check_buffer = NULL; + unsigned int flags = 0; + int id; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "backupxml", &backup_from) < 0) + return false; + + if (!backup_from) { + backup_buffer = vshStrdup(ctl, "<domainbackup/>"); + } else { + if (virFileReadAll(backup_from, VSH_MAX_XML_FILE, &backup_buffer) < 0) { + vshSaveLibvirtError(); + return false; + } + } + + if (vshCommandOptStringReq(ctl, cmd, "checkpointxml", &check_from) < 0) + return false; + if (check_from) { + if (virFileReadAll(check_from, VSH_MAX_XML_FILE, &check_buffer) < 0) { + vshSaveLibvirtError(); + return false; + } + } + + if ((id = virDomainBackupBegin(dom, backup_buffer, check_buffer, flags)) < 0) + return false; + + vshPrint(ctl, _("Backup id %d started\n"), id); + + return true; +} + + +/* + * "backup-dumpxml" command + */ +static const vshCmdInfo info_backup_dumpxml[] = { + {.name = "help", + .data = N_("Dump XML for an ongoing domain block backup job") + }, + {.name = "desc", + .data = N_("Backup Dump XML") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_backup_dumpxml[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "id", + .type = VSH_OT_INT, + .help = N_("backup job id"), + }, + {.name = NULL} +}; + +static bool +cmdBackupDumpXML(vshControl *ctl, + const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + g_autofree char *xml = NULL; + unsigned int flags = 0; + int id = 0; + + if (vshCommandOptBool(cmd, "security-info")) + flags |= VIR_DOMAIN_XML_SECURE; + + if (vshCommandOptInt(ctl, cmd, "id", &id) < 0) + return false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!(xml = virDomainBackupGetXMLDesc(dom, id, flags))) + return false; + + vshPrint(ctl, "%s", xml); + return true; +} + + +/* + * "backup-end" command + */ +static const vshCmdInfo info_backup_end[] = { + {.name = "help", + .data = N_("Conclude a disk backup of a live domain") + }, + {.name = "desc", + .data = N_("End a domain block backup job") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_backup_end[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "id", + .type = VSH_OT_INT, + .help = N_("backup job id"), + }, + {.name = NULL} +}; + +static bool +cmdBackupEnd(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + unsigned int flags = 0; + int id = 0; + int rc; + + if (vshCommandOptInt(ctl, cmd, "id", &id) < 0) + return false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if ((rc = virDomainBackupEnd(dom, id, flags)) < 0) + return false; + + vshPrint(ctl, _("Backup id %d terminated"), id); + + return true; +} + + +const vshCmdDef backupCmds[] = { + {.name = "backup-begin", + .handler = cmdBackupBegin, + .opts = opts_backup_begin, + .info = info_backup_begin, + .flags = 0 + }, + {.name = "backup-dumpxml", + .handler = cmdBackupDumpXML, + .opts = opts_backup_dumpxml, + .info = info_backup_dumpxml, + .flags = 0 + }, + {.name = "backup-end", + .handler = cmdBackupEnd, + .opts = opts_backup_end, + .info = info_backup_end, + .flags = 0 + }, + {.name = NULL} +}; diff --git a/tools/virsh-backup.h b/tools/virsh-backup.h new file mode 100644 index 0000000000..95c2f5a424 --- /dev/null +++ b/tools/virsh-backup.h @@ -0,0 +1,21 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "virsh.h" + +extern const vshCmdDef backupCmds[]; diff --git a/tools/virsh.c b/tools/virsh.c index a3553ddd36..59c3ddb4b7 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -47,6 +47,7 @@ #include "virstring.h" #include "virgettext.h" +#include "virsh-backup.h" #include "virsh-checkpoint.h" #include "virsh-console.h" #include "virsh-domain.h" @@ -831,6 +832,7 @@ static const vshCmdGrp cmdGroups[] = { {VIRSH_CMD_GRP_NODEDEV, "nodedev", nodedevCmds}, {VIRSH_CMD_GRP_SECRET, "secret", secretCmds}, {VIRSH_CMD_GRP_SNAPSHOT, "snapshot", snapshotCmds}, + {VIRSH_CMD_GRP_BACKUP, "backup", backupCmds}, {VIRSH_CMD_GRP_STORAGE_POOL, "pool", storagePoolCmds}, {VIRSH_CMD_GRP_STORAGE_VOL, "volume", storageVolCmds}, {VIRSH_CMD_GRP_VIRSH, "virsh", virshCmds}, diff --git a/tools/virsh.h b/tools/virsh.h index b4e610b2a4..d84659124a 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -51,6 +51,7 @@ #define VIRSH_CMD_GRP_NWFILTER "Network Filter" #define VIRSH_CMD_GRP_SECRET "Secret" #define VIRSH_CMD_GRP_SNAPSHOT "Snapshot" +#define VIRSH_CMD_GRP_BACKUP "Backup" #define VIRSH_CMD_GRP_HOST_AND_HV "Host and Hypervisor" #define VIRSH_CMD_GRP_VIRSH "Virsh itself" diff --git a/tools/virsh.pod b/tools/virsh.pod index cf2798e71a..83cb315506 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1327,6 +1327,43 @@ addresses, currently 'lease' to read DHCP leases, 'agent' to query the guest OS via an agent, or 'arp' to get IP from host's arp tables. If unspecified, 'lease' is the default. +=item B<backup-begin> I<domain> [I<backupxml>] +[I<checkpointxml> + +Begin a new backup job, and output the resulting job id on success. If +I<backupxml> is omitted, this defaults to a full backup using a push +model to filenames generated by libvirt; supplying XML allows +fine-tuning such as requesting an incremental backup relative to an +earlier checkpoint, controlling which disks participate or which +filenames are involved, or requesting the use of a pull model backup. +The B<backup-dumpxml> command shows any resulting values assigned by +libvirt. For more information on backup XML, see: +L<https://libvirt.org/formatbackup.html>. + +If I<checkpointxml> is specified, a second file with a top-level +element of <domaincheckpoint> is used to create a simultaneous +checkpoint, for doing a later incremental backup relative to the time +the backup was created. See B<checkpoint-create> for more details on +checkpoints. + +This command returns as soon as possible, and the backup job runs in +the background; the progress of a push model backup can be checked +with B<domjobinfo> or by waiting for an event with B<event> (the +progress of a pull model backup is under the control of whatever third +party connects to the NBD export). The job is ended with B<backup-end>. + +=item B<backup-dumpxml> I<domain> [I<id>] + +Output XML describing the backup job I<id>. The default for I<id> is +0, which works as long as there are no parallel jobs; it is also +possible to use the positive id printed by B<backup-begin> on success. + +=item B<backup-end> I<domain> [I<id>] + +End the current backup job I<id>. The default for I<id> is 0, which +works as long as there are no parallel jobs; it is also possible to +use the positive id printed by B<backup-begin> on success. + =item B<domiflist> I<domain> [I<--inactive>] Print a table showing the brief information of all virtual interfaces -- 2.21.0

After the individual sub-blockjobs of a backup libvirt job finish we must detect it and notify the parent job, so that it can be properly terminated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 45 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.c | 4 ++++ 2 files changed, 49 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 6f190b3485..acfc07638b 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -27,6 +27,7 @@ #include "qemu_block.h" #include "qemu_domain.h" #include "qemu_alias.h" +#include "qemu_backup.h" #include "conf/domain_conf.h" #include "conf/domain_event.h" @@ -1282,6 +1283,49 @@ qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver, } +static void +qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob, + qemuBlockjobState newstate) +{ + g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; + g_autoptr(virJSONValue) actions = NULL; + + qemuBackupNotifyBlockjobEnd(vm, job->data.backup.jobid, job->disk, newstate); + + if (job->data.backup.store && + !(backend = qemuBlockStorageSourceDetachPrepare(job->data.backup.store, NULL))) + return; + + if (job->data.backup.bitmap) { + if (!(actions = virJSONValueNewArray())) + return; + + if (qemuMonitorTransactionBitmapRemove(actions, + job->disk->src->nodeformat, + job->data.backup.bitmap) < 0) + return; + } + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return; + + if (backend) + qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), backend); + + if (actions) + qemuMonitorTransaction(qemuDomainGetMonitor(vm), &actions); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return; + + if (job->data.backup.store) + qemuDomainStorageSourceAccessRevoke(driver, vm, job->data.backup.store); +} + + static void qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, virQEMUDriverPtr driver, @@ -1320,6 +1364,7 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, break; case QEMU_BLOCKJOB_TYPE_BACKUP: + qemuBlockJobProcessEventConcludedBackup(driver, vm, job, asyncJob, job->newstate); case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 51603d2631..e4f1db284d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1152,6 +1152,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; switch ((virConnectDomainEventBlockJobStatus) event) { case VIR_DOMAIN_BLOCK_JOB_COMPLETED: @@ -4858,6 +4860,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; -- 2.21.0