virDomainSnapshotAlignDisks: refactor extension to all disks

Last step of the algorithm in virDomainSnapshotAlignDisks is to extend
the array of disks to all VM's disk and provide defaults. This was done
by extending the array, adding defaults at the end and then sorting it.
This requires the 'idx' variable and also a separate sorting function.

If we store the pointer to existing snapshot disk definitions in a hash
table and create a new array of snapshot disk definitions, we can fill
the new array directly by either copying the definition from the old
array or adding the default.

This avoids the sorting step and thus even the need to store the index
of the domain disk altogether.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Peter Krempa 2020-09-21 19:36:17 +02:00
parent 17c238626b
commit d3c029bb10

View File

@ -627,16 +627,6 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def)
}
static int
virDomainSnapshotCompareDiskIndex(const void *a, const void *b)
{
const virDomainSnapshotDiskDef *diska = a;
const virDomainSnapshotDiskDef *diskb = b;
/* Integer overflow shouldn't be a problem here. */
return diska->idx - diskb->idx;
}
/* Align def->disks to def->parent.dom. Sort the list of def->disks,
* filling in any missing disks or snapshot state defaults given by
* the domain, with a fallback to a passed in default. Convert paths
@ -650,9 +640,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
bool require_match)
{
virDomainDefPtr domdef = snapdef->parent.dom;
g_autoptr(virBitmap) map = NULL;
g_autoptr(virHashTable) map = virHashNew(NULL);
g_autofree virDomainSnapshotDiskDefPtr olddisks = NULL;
size_t i;
int ndisks;
if (!domdef) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@ -670,9 +660,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
if (!domdef->ndisks)
return 0;
if (!(map = virBitmapNew(domdef->ndisks)))
return -1;
/* Double check requested disks. */
for (i = 0; i < snapdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
@ -687,14 +674,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
domdisk = domdef->disks[idx];
if (virBitmapIsBitSet(map, idx)) {
if (virHashHasEntry(map, domdisk->dst)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' specified twice"),
snapdisk->name);
return -1;
}
ignore_value(virBitmapSetBit(map, idx));
snapdisk->idx = idx;
if (virHashAddEntry(map, domdisk->dst, snapdisk) < 0)
return -1;
if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
if (domdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
@ -729,21 +717,24 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
}
}
/* Provide defaults for all remaining disks. */
ndisks = snapdef->ndisks;
if (VIR_EXPAND_N(snapdef->disks, snapdef->ndisks,
domdef->ndisks - snapdef->ndisks) < 0)
return -1;
olddisks = g_steal_pointer(&snapdef->disks);
snapdef->disks = g_new0(virDomainSnapshotDiskDef, domdef->ndisks);
snapdef->ndisks = domdef->ndisks;
for (i = 0; i < domdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk;
virDomainDiskDefPtr domdisk = domdef->disks[i];
virDomainSnapshotDiskDefPtr snapdisk = snapdef->disks + i;
virDomainSnapshotDiskDefPtr existing;
if (virBitmapIsBitSet(map, i))
/* copy existing disks */
if ((existing = virHashLookup(map, domdisk->dst))) {
memcpy(snapdisk, existing, sizeof(*snapdisk));
continue;
snapdisk = &snapdef->disks[ndisks++];
}
/* Provide defaults for all remaining disks. */
snapdisk->src = virStorageSourceNew();
snapdisk->name = g_strdup(domdef->disks[i]->dst);
snapdisk->idx = i;
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(domdef->disks[i]->src))
@ -756,9 +747,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
snapdisk->snapshot = default_snapshot;
}
qsort(&snapdef->disks[0], snapdef->ndisks, sizeof(snapdef->disks[0]),
virDomainSnapshotCompareDiskIndex);
/* Generate default external file names for external snapshot locations */
if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
return -1;