[libvirt] [PATCH 0/4] virObject for snapshot def [incremental backup saga]
Peter rightly complained that my attempt to leave a todo in virdomainmomentobjlist.c about not being polymorphic enough gives no incentive to get it fixed later once incremental backups are in, so instead fix it now. My v9 backup patches will be changed similarly to the changes to snapshot shown here. Eric Blake (4): snapshot: s/parent/parent_name/ as prep for virObject snapshot: s/current/parent/ as prep for virObject snapshot: Add virDomainSnapshotDefNew snapshot: Make virDomainSnapshotDef a virObject src/conf/moment_conf.h | 7 +- src/conf/snapshot_conf.h | 4 +- cfg.mk | 2 - src/conf/moment_conf.c | 30 ++++- src/conf/snapshot_conf.c | 164 ++++++++++++++++------------ src/conf/virdomainmomentobjlist.c | 7 +- src/conf/virdomainsnapshotobjlist.c | 2 +- src/esx/esx_driver.c | 19 ++-- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 10 +- src/qemu/qemu_driver.c | 29 +++-- src/test/test_driver.c | 25 ++--- src/vbox/vbox_common.c | 95 ++++++++-------- src/vz/vz_driver.c | 5 +- src/vz/vz_sdk.c | 10 +- tests/domainsnapshotxml2xmltest.c | 3 +- 16 files changed, 230 insertions(+), 184 deletions(-) -- 2.20.1
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as the name of their inherited member at offset 0. While it would be possible to write a new class-creation macro that takes the actual field name, and rewrite VIR_CLASS_NEW to call the new macro with the hard-coded name 'parent', so that we could make virDomainMomentDef use a custom name for its base class, it seems less confusing if all object code uses similar naming. Thus, this is a mechanical rename in preparation of making virDomainSnapshotDef a descendent of virObject, when we can no longer use 'parent' for a different purpose than the base class. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/moment_conf.h | 2 +- src/conf/moment_conf.c | 2 +- src/conf/snapshot_conf.c | 22 ++++++++++++---------- src/conf/virdomainmomentobjlist.c | 4 ++-- src/esx/esx_driver.c | 2 +- src/qemu/qemu_domain.c | 8 ++++---- src/qemu/qemu_driver.c | 12 ++++++------ src/test/test_driver.c | 18 +++++++++--------- src/vbox/vbox_common.c | 14 +++++++------- src/vz/vz_sdk.c | 2 +- 10 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h index e06a4a7b3c..04e0c0648b 100644 --- a/src/conf/moment_conf.h +++ b/src/conf/moment_conf.h @@ -31,7 +31,7 @@ struct _virDomainMomentDef { /* Common portion of public XML. */ char *name; char *description; - char *parent; + char *parent_name; long long creationTime; /* in seconds */ virDomainDefPtr dom; diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c index 0eb32eeb52..9829775b3c 100644 --- a/src/conf/moment_conf.c +++ b/src/conf/moment_conf.c @@ -38,7 +38,7 @@ void virDomainMomentDefClear(virDomainMomentDefPtr def) { VIR_FREE(def->name); VIR_FREE(def->description); - VIR_FREE(def->parent); + VIR_FREE(def->parent_name); virDomainDefFree(def->dom); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index eaa9b3c5e6..b571c5cc41 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -227,7 +227,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, goto cleanup; } - def->common.parent = virXPathString("string(./parent/name)", ctxt); + def->common.parent_name = virXPathString("string(./parent/name)", ctxt); state = virXPathString("string(./state)", ctxt); if (state == NULL) { @@ -809,10 +809,11 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf, virBufferAsprintf(buf, "<state>%s</state>\n", virDomainSnapshotStateTypeToString(def->state)); - if (def->common.parent) { + if (def->common.parent_name) { virBufferAddLit(buf, "<parent>\n"); virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<name>%s</name>\n", def->common.parent); + virBufferEscapeString(buf, "<name>%s</name>\n", + def->common.parent_name); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</parent>\n"); } @@ -932,30 +933,31 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, bool check_if_stolen; /* Prevent circular chains */ - if (def->common.parent) { - if (STREQ(def->common.name, def->common.parent)) { + if (def->common.parent_name) { + if (STREQ(def->common.name, def->common.parent_name)) { virReportError(VIR_ERR_INVALID_ARG, _("cannot set snapshot %s as its own parent"), def->common.name); return -1; } - other = virDomainSnapshotFindByName(vm->snapshots, def->common.parent); + other = virDomainSnapshotFindByName(vm->snapshots, + def->common.parent_name); if (!other) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s for snapshot %s not found"), - def->common.parent, def->common.name); + def->common.parent_name, def->common.name); return -1; } otherdef = virDomainSnapshotObjGetDef(other); - while (otherdef->common.parent) { - if (STREQ(otherdef->common.parent, def->common.name)) { + while (otherdef->common.parent_name) { + if (STREQ(otherdef->common.parent_name, def->common.name)) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s would create cycle to %s"), otherdef->common.name, def->common.name); return -1; } other = virDomainSnapshotFindByName(vm->snapshots, - otherdef->common.parent); + otherdef->common.parent_name); if (!other) { VIR_WARN("snapshots are inconsistent for %s", vm->def->name); diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 7f90632a53..e9df66c65b 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -464,7 +464,7 @@ virDomainMomentForEach(virDomainMomentObjListPtr moments, /* Struct and callback function used as a hash table callback; each call - * inspects the pre-existing moment->def->parent field, and adjusts + * inspects the pre-existing moment->def->parent_name field, and adjusts * the moment->parent field as well as the parent's child fields to * wire up the hierarchical relations for the given moment. The error * indicator gets set if a parent is missing or a requested parent would @@ -483,7 +483,7 @@ virDomainMomentSetRelations(void *payload, virDomainMomentObjPtr tmp; virDomainMomentObjPtr parent; - parent = virDomainMomentFindByName(curr->moments, obj->def->parent); + parent = virDomainMomentFindByName(curr->moments, obj->def->parent_name); if (!parent) { curr->err = -1; parent = &curr->moments->metaroot; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index d80fef0a58..dff801f946 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4191,7 +4191,7 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, def.common.name = snapshot->name; def.common.description = snapshotTree->description; - def.common.parent = snapshotTreeParent ? snapshotTreeParent->name : NULL; + def.common.parent_name = snapshotTreeParent ? snapshotTreeParent->name : NULL; if (esxVI_DateTime_ConvertToCalendarTime(snapshotTree->createTime, &def.common.creationTime) < 0) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index db67fe2193..6b16da2998 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8606,19 +8606,19 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); - if (update_parent && snap->def->parent) { + if (update_parent && snap->def->parent_name) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->parent_name); if (!parentsnap) { VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); + snap->def->parent_name); } else { virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, driver->caps, driver->xmlopt, cfg->snapshotDir) < 0) { VIR_WARN("failed to set parent snapshot '%s' as current", - snap->def->parent); + snap->def->parent_name); virDomainSnapshotSetCurrent(vm->snapshots, NULL); } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 27be173e46..c12b3dff6c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15752,7 +15752,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, current = virDomainSnapshotGetCurrent(vm->snapshots); if (current) { if (!redefine && - VIR_STRDUP(snap->def->parent, current->def->name) < 0) + VIR_STRDUP(snap->def->parent_name, current->def->name) < 0) goto endjob; if (update_current) { virDomainSnapshotSetCurrent(vm->snapshots, NULL); @@ -15820,7 +15820,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotObjListRemove(vm->snapshots, snap); } else { other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->parent_name); virDomainMomentSetParent(snap, other); } } else if (snap) { @@ -16080,14 +16080,14 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->parent) { + if (!snap->def->parent_name) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), snap->def->name); goto cleanup; } - parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent); + parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent_name); cleanup: virDomainObjEndAPI(&vm); @@ -16668,10 +16668,10 @@ qemuDomainMomentReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(moment->def->parent); + VIR_FREE(moment->def->parent_name); if (rep->parent->def && - VIR_STRDUP(moment->def->parent, rep->parent->def->name) < 0) { + VIR_STRDUP(moment->def->parent_name, rep->parent->def->name) < 0) { rep->err = -1; return 0; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 460c896ef6..626167f947 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6195,14 +6195,14 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - if (!snap->def->parent) { + if (!snap->def->parent_name) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, _("snapshot '%s' does not have a parent"), snap->def->name); goto cleanup; } - parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent); + parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent_name); cleanup: virDomainObjEndAPI(&vm); @@ -6421,7 +6421,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, } if (!redefine) { - if (VIR_STRDUP(snap->def->parent, + if (VIR_STRDUP(snap->def->parent_name, virDomainSnapshotGetCurrentName(vm->snapshots)) < 0) goto cleanup; @@ -6442,7 +6442,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) virDomainSnapshotSetCurrent(vm->snapshots, snap); other = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->parent_name); virDomainMomentSetParent(snap, other); } virDomainObjEndAPI(&vm); @@ -6491,10 +6491,10 @@ testDomainSnapshotReparentChildren(void *payload, if (rep->err < 0) return 0; - VIR_FREE(snap->def->parent); + VIR_FREE(snap->def->parent_name); if (rep->parent->def && - VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { + VIR_STRDUP(snap->def->parent_name, rep->parent->def->name) < 0) { rep->err = -1; return 0; } @@ -6549,12 +6549,12 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, } else { virDomainMomentDropParent(snap); if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { - if (snap->def->parent) { + if (snap->def->parent_name) { parentsnap = virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent); + snap->def->parent_name); if (!parentsnap) VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); + snap->def->parent_name); } virDomainSnapshotSetCurrent(vm->snapshots, parentsnap); } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 7a491b0b5f..94830f5ddb 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -5061,7 +5061,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(tmp); } - if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->common.parent) < 0) { + if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->common.parent_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to add the snapshot to the machine description")); goto cleanup; @@ -6305,7 +6305,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, } VBOX_UTF16_TO_UTF8(str16, &str8); VBOX_UTF16_FREE(str16); - if (VIR_STRDUP(def->common.parent, str8) < 0) { + if (VIR_STRDUP(def->common.parent_name, str8) < 0) { VBOX_UTF8_FREE(str8); goto cleanup; } @@ -6999,7 +6999,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) * disks. The first thing to do is to manipulate VirtualBox API to create * differential read-write disks if the parent snapshot is not null. */ - if (def->common.parent != NULL) { + if (def->common.parent_name != NULL) { for (it = 0; it < def->common.dom->ndisks; it++) { virVBoxSnapshotConfHardDiskPtr readOnly = NULL; IMedium *medium = NULL; @@ -7058,7 +7058,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_UTF8_TO_UTF16("VDI", &formatUtf16); if (virAsprintf(&newLocationUtf8, "%sfakedisk-%s-%d.vdi", - machineLocationPath, def->common.parent, it) < 0) + machineLocationPath, def->common.parent_name, it) < 0) goto cleanup; VBOX_UTF8_TO_UTF16(newLocationUtf8, &newLocation); rc = gVBoxAPI.UIVirtualBox.CreateHardDisk(data->vboxObj, @@ -7209,7 +7209,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } } /*If the parent snapshot is not NULL, we remove the-read only disks from the media registry*/ - if (def->common.parent != NULL) { + if (def->common.parent_name != NULL) { for (it = 0; it < def->common.dom->ndisks; it++) { const char *uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, @@ -7289,8 +7289,8 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) if (isCurrent) { VIR_FREE(snapshotMachineDesc->currentSnapshot); - if (def->common.parent != NULL) { - virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->common.parent); + if (def->common.parent_name != NULL) { + virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->common.parent_name); if (!snap) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to get the snapshot to remove")); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 73d5c89c08..500de5f8ba 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4703,7 +4703,7 @@ prlsdkParseSnapshotTree(const char *treexml) goto cleanup; } - def->common.parent = virXPathString("string(../@guid)", ctxt); + def->common.parent_name = virXPathString("string(../@guid)", ctxt); xmlstr = virXPathString("string(./DateTime)", ctxt); if (!xmlstr) { -- 2.20.1
On Wed, May 08, 2019 at 17:24:09 -0500, Eric Blake wrote:
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as the name of their inherited member at offset 0. While it would be possible to write a new class-creation macro that takes the actual field name, and rewrite VIR_CLASS_NEW to call the new macro with the hard-coded name 'parent', so that we could make virDomainMomentDef use a custom name for its base class, it seems less confusing if all object code uses similar naming. Thus, this is a mechanical rename in preparation of making virDomainSnapshotDef a descendent of virObject, when we can no longer use 'parent' for a different purpose than the base class.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/moment_conf.h | 2 +- src/conf/moment_conf.c | 2 +- src/conf/snapshot_conf.c | 22 ++++++++++++---------- src/conf/virdomainmomentobjlist.c | 4 ++-- src/esx/esx_driver.c | 2 +- src/qemu/qemu_domain.c | 8 ++++---- src/qemu/qemu_driver.c | 12 ++++++------ src/test/test_driver.c | 18 +++++++++--------- src/vbox/vbox_common.c | 14 +++++++------- src/vz/vz_sdk.c | 2 +- 10 files changed, 44 insertions(+), 42 deletions(-)
ACK
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as the name of their inherited member at offset 0. While it would be possible to write a new class-creation macro that takes the actual field name 'current', and rewrite VIR_CLASS_NEW to call the new macro with the hard-coded name 'parent', it seems less confusing if all object code uses similar naming. Thus, this is a mechanical rename in preparation of making virDomainSnapshotDef a descendent of virObject. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/snapshot_conf.h | 2 +- src/conf/snapshot_conf.c | 120 ++++++++++++++-------------- src/conf/virdomainsnapshotobjlist.c | 2 +- src/esx/esx_driver.c | 16 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 12 +-- src/test/test_driver.c | 2 +- src/vbox/vbox_common.c | 88 ++++++++++---------- src/vz/vz_driver.c | 2 +- src/vz/vz_sdk.c | 10 +-- 10 files changed, 128 insertions(+), 128 deletions(-) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 5a762ccd96..f54be11619 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -74,7 +74,7 @@ struct _virDomainSnapshotDiskDef { /* Stores the complete snapshot metadata */ struct _virDomainSnapshotDef { - virDomainMomentDef common; + virDomainMomentDef parent; /* Additional public XML. */ int state; /* virDomainSnapshotState */ diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index b571c5cc41..dd281d57fe 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -88,7 +88,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) if (!def) return; - virDomainMomentDefClear(&def->common); + virDomainMomentDefClear(&def->parent); VIR_FREE(def->file); for (i = 0; i < def->ndisks; i++) virDomainSnapshotDiskDefClear(&def->disks[i]); @@ -208,8 +208,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, if (VIR_ALLOC(def) < 0) goto cleanup; - def->common.name = virXPathString("string(./name)", ctxt); - if (def->common.name == NULL) { + def->parent.name = virXPathString("string(./name)", ctxt); + if (def->parent.name == NULL) { if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { virReportError(VIR_ERR_XML_ERROR, "%s", _("a redefined snapshot must have a name")); @@ -217,17 +217,17 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, } } - def->common.description = virXPathString("string(./description)", ctxt); + def->parent.description = virXPathString("string(./description)", ctxt); if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { if (virXPathLongLong("string(./creationTime)", ctxt, - &def->common.creationTime) < 0) { + &def->parent.creationTime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; } - def->common.parent_name = virXPathString("string(./parent/name)", ctxt); + def->parent.parent_name = virXPathString("string(./parent/name)", ctxt); state = virXPathString("string(./state)", ctxt); if (state == NULL) { @@ -263,14 +263,14 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, _("missing domain in snapshot")); goto cleanup; } - def->common.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, + def->parent.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, caps, xmlopt, NULL, domainflags); - if (!def->common.dom) + if (!def->parent.dom) goto cleanup; } else { VIR_WARN("parsing older snapshot that lacks domain"); } - } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->common) < 0) { + } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) { goto cleanup; } @@ -422,7 +422,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, /* Perform sanity checking on a redefined snapshot definition. If - * @other is non-NULL, this may include swapping def->common.dom from other + * @other is non-NULL, this may include swapping def->parent.dom from other * into def. */ int virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, @@ -440,17 +440,17 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("disk-only flag for snapshot %s requires " "disk-snapshot state"), - def->common.name); + def->parent.name); return -1; } - if (def->common.dom && memcmp(def->common.dom->uuid, domain_uuid, + if (def->parent.dom && memcmp(def->parent.dom->uuid, domain_uuid, VIR_UUID_BUFLEN)) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain_uuid, uuidstr); virReportError(VIR_ERR_INVALID_ARG, _("definition for snapshot %s must use uuid %s"), - def->common.name, uuidstr); + def->parent.name, uuidstr); return -1; } @@ -464,7 +464,7 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("cannot change between online and offline " "snapshot state in snapshot %s"), - def->common.name); + def->parent.name); return -1; } @@ -473,23 +473,23 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("cannot change between disk only and " "full system in snapshot %s"), - def->common.name); + def->parent.name); return -1; } - if (otherdef->common.dom) { - if (def->common.dom) { - if (!virDomainDefCheckABIStability(otherdef->common.dom, - def->common.dom, xmlopt)) + if (otherdef->parent.dom) { + if (def->parent.dom) { + if (!virDomainDefCheckABIStability(otherdef->parent.dom, + def->parent.dom, xmlopt)) return -1; } else { /* Transfer the domain def */ - VIR_STEAL_PTR(def->common.dom, otherdef->common.dom); + VIR_STEAL_PTR(def->parent.dom, otherdef->parent.dom); } } } - if (def->common.dom) { + if (def->parent.dom) { if (external) { align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; align_match = false; @@ -536,7 +536,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def) return -1; } - if (!(origpath = virDomainDiskGetSource(def->common.dom->disks[i]))) { + if (!(origpath = virDomainDiskGetSource(def->parent.dom->disks[i]))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name " "for disk '%s' without source"), @@ -560,7 +560,7 @@ virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def) if ((tmp = strrchr(tmppath, '.')) && !strchr(tmp, '/')) *tmp = '\0'; - if (virAsprintf(&disk->src->path, "%s.%s", tmppath, def->common.name) < 0) { + if (virAsprintf(&disk->src->path, "%s.%s", tmppath, def->parent.name) < 0) { VIR_FREE(tmppath); return -1; } @@ -593,7 +593,7 @@ virDomainSnapshotCompareDiskIndex(const void *a, const void *b) return diska->idx - diskb->idx; } -/* Align def->disks to def->common.dom. Sort the list of def->disks, +/* 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 * to disk targets for uniformity. Issue an error and return -1 if @@ -610,31 +610,31 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, size_t i; int ndisks; - if (!def->common.dom) { + if (!def->parent.dom) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing domain in snapshot")); goto cleanup; } - if (def->ndisks > def->common.dom->ndisks) { + if (def->ndisks > def->parent.dom->ndisks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("too many disk snapshot requests for domain")); goto cleanup; } /* Unlikely to have a guest without disks but technically possible. */ - if (!def->common.dom->ndisks) { + if (!def->parent.dom->ndisks) { ret = 0; goto cleanup; } - if (!(map = virBitmapNew(def->common.dom->ndisks))) + if (!(map = virBitmapNew(def->parent.dom->ndisks))) goto cleanup; /* Double check requested disks. */ for (i = 0; i < def->ndisks; i++) { virDomainSnapshotDiskDefPtr disk = &def->disks[i]; - int idx = virDomainDiskIndexByName(def->common.dom, disk->name, false); + int idx = virDomainDiskIndexByName(def->parent.dom, disk->name, false); int disk_snapshot; if (idx < 0) { @@ -652,7 +652,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, ignore_value(virBitmapSetBit(map, idx)); disk->idx = idx; - disk_snapshot = def->common.dom->disks[idx]->snapshot; + disk_snapshot = def->parent.dom->disks[idx]->snapshot; if (!disk->snapshot) { if (disk_snapshot && (!require_match || @@ -680,9 +680,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, disk->src->path, disk->name); goto cleanup; } - if (STRNEQ(disk->name, def->common.dom->disks[idx]->dst)) { + if (STRNEQ(disk->name, def->parent.dom->disks[idx]->dst)) { VIR_FREE(disk->name); - if (VIR_STRDUP(disk->name, def->common.dom->disks[idx]->dst) < 0) + if (VIR_STRDUP(disk->name, def->parent.dom->disks[idx]->dst) < 0) goto cleanup; } } @@ -690,10 +690,10 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, /* Provide defaults for all remaining disks. */ ndisks = def->ndisks; if (VIR_EXPAND_N(def->disks, def->ndisks, - def->common.dom->ndisks - def->ndisks) < 0) + def->parent.dom->ndisks - def->ndisks) < 0) goto cleanup; - for (i = 0; i < def->common.dom->ndisks; i++) { + for (i = 0; i < def->parent.dom->ndisks; i++) { virDomainSnapshotDiskDefPtr disk; if (virBitmapIsBitSet(map, i)) @@ -701,15 +701,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, disk = &def->disks[ndisks++]; if (!(disk->src = virStorageSourceNew())) goto cleanup; - if (VIR_STRDUP(disk->name, def->common.dom->disks[i]->dst) < 0) + if (VIR_STRDUP(disk->name, def->parent.dom->disks[i]->dst) < 0) goto cleanup; disk->idx = i; /* Don't snapshot empty drives */ - if (virStorageSourceIsEmpty(def->common.dom->disks[i]->src)) + if (virStorageSourceIsEmpty(def->parent.dom->disks[i]->src)) disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; else - disk->snapshot = def->common.dom->disks[i]->snapshot; + disk->snapshot = def->parent.dom->disks[i]->snapshot; disk->src->type = VIR_STORAGE_TYPE_FILE; if (!disk->snapshot) @@ -801,26 +801,26 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "<domainsnapshot>\n"); virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<name>%s</name>\n", def->common.name); - if (def->common.description) + virBufferEscapeString(buf, "<name>%s</name>\n", def->parent.name); + if (def->parent.description) virBufferEscapeString(buf, "<description>%s</description>\n", - def->common.description); + def->parent.description); if (def->state) virBufferAsprintf(buf, "<state>%s</state>\n", virDomainSnapshotStateTypeToString(def->state)); - if (def->common.parent_name) { + if (def->parent.parent_name) { virBufferAddLit(buf, "<parent>\n"); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<name>%s</name>\n", - def->common.parent_name); + def->parent.parent_name); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</parent>\n"); } - if (def->common.creationTime) + if (def->parent.creationTime) virBufferAsprintf(buf, "<creationTime>%lld</creationTime>\n", - def->common.creationTime); + def->parent.creationTime); if (def->memory) { virBufferAsprintf(buf, "<memory snapshot='%s'", @@ -840,8 +840,8 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "</disks>\n"); } - if (def->common.dom) { - if (virDomainDefFormatInternal(def->common.dom, caps, domainflags, buf, + if (def->parent.dom) { + if (virDomainDefFormatInternal(def->parent.dom, caps, domainflags, buf, xmlopt) < 0) goto error; } else if (uuidstr) { @@ -933,31 +933,31 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, bool check_if_stolen; /* Prevent circular chains */ - if (def->common.parent_name) { - if (STREQ(def->common.name, def->common.parent_name)) { + if (def->parent.parent_name) { + if (STREQ(def->parent.name, def->parent.parent_name)) { virReportError(VIR_ERR_INVALID_ARG, _("cannot set snapshot %s as its own parent"), - def->common.name); + def->parent.name); return -1; } other = virDomainSnapshotFindByName(vm->snapshots, - def->common.parent_name); + def->parent.parent_name); if (!other) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s for snapshot %s not found"), - def->common.parent_name, def->common.name); + def->parent.parent_name, def->parent.name); return -1; } otherdef = virDomainSnapshotObjGetDef(other); - while (otherdef->common.parent_name) { - if (STREQ(otherdef->common.parent_name, def->common.name)) { + while (otherdef->parent.parent_name) { + if (STREQ(otherdef->parent.parent_name, def->parent.name)) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s would create cycle to %s"), - otherdef->common.name, def->common.name); + otherdef->parent.name, def->parent.name); return -1; } other = virDomainSnapshotFindByName(vm->snapshots, - otherdef->common.parent_name); + otherdef->parent.parent_name); if (!other) { VIR_WARN("snapshots are inconsistent for %s", vm->def->name); @@ -967,14 +967,14 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } } - other = virDomainSnapshotFindByName(vm->snapshots, def->common.name); + other = virDomainSnapshotFindByName(vm->snapshots, def->parent.name); otherdef = other ? virDomainSnapshotObjGetDef(other) : NULL; - check_if_stolen = other && otherdef->common.dom; + check_if_stolen = other && otherdef->parent.dom; if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, flags) < 0) { /* revert any stealing of the snapshot domain definition */ - if (check_if_stolen && def->common.dom && !otherdef->common.dom) - VIR_STEAL_PTR(otherdef->common.dom, def->common.dom); + if (check_if_stolen && def->parent.dom && !otherdef->parent.dom) + VIR_STEAL_PTR(otherdef->parent.dom, def->parent.dom); return -1; } if (other) { @@ -987,7 +987,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, * child relations by reusing snap. */ virDomainMomentDropParent(other); virDomainSnapshotDefFree(otherdef); - other->def = &(*defptr)->common; + other->def = &(*defptr)->parent; *defptr = NULL; *snap = other; } diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c index 46b7d8b0e7..9bcc8d1036 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -44,7 +44,7 @@ virDomainMomentObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotDefPtr def) { - return virDomainMomentAssignDef(snapshots->base, &def->common); + return virDomainMomentAssignDef(snapshots->base, &def->parent); } diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index dff801f946..f45d96a4f5 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4118,7 +4118,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, priv->parsedUri->autoAnswer) < 0 || esxVI_LookupRootSnapshotTreeList(priv->primary, domain->uuid, &rootSnapshotList) < 0 || - esxVI_GetSnapshotTreeByName(rootSnapshotList, def->common.name, + esxVI_GetSnapshotTreeByName(rootSnapshotList, def->parent.name, &snapshotTree, NULL, esxVI_Occurrence_OptionalItem) < 0) { goto cleanup; @@ -4126,12 +4126,12 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, if (snapshotTree) { virReportError(VIR_ERR_OPERATION_INVALID, - _("Snapshot '%s' already exists"), def->common.name); + _("Snapshot '%s' already exists"), def->parent.name); goto cleanup; } if (esxVI_CreateSnapshot_Task(priv->primary, virtualMachine->obj, - def->common.name, def->common.description, + def->parent.name, def->parent.description, diskOnly ? esxVI_Boolean_False : esxVI_Boolean_True, quiesce ? esxVI_Boolean_True : esxVI_Boolean_False, &task) < 0 || @@ -4148,7 +4148,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, goto cleanup; } - snapshot = virGetDomainSnapshot(domain, def->common.name); + snapshot = virGetDomainSnapshot(domain, def->parent.name); cleanup: virDomainSnapshotDefFree(def); @@ -4189,12 +4189,12 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, goto cleanup; } - def.common.name = snapshot->name; - def.common.description = snapshotTree->description; - def.common.parent_name = snapshotTreeParent ? snapshotTreeParent->name : NULL; + def.parent.name = snapshot->name; + def.parent.description = snapshotTree->description; + def.parent.parent_name = snapshotTreeParent ? snapshotTreeParent->name : NULL; if (esxVI_DateTime_ConvertToCalendarTime(snapshotTree->createTime, - &def.common.creationTime) < 0) { + &def.parent.creationTime) < 0) { goto cleanup; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b16da2998..53012f4116 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8467,7 +8467,7 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, goto cleanup; } - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->common.name) < 0) + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->parent.name) < 0) goto cleanup; ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c12b3dff6c..833c1e88ee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14446,7 +14446,7 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, * create them correctly. */ for (i = 0; i < snapdef->ndisks && !reuse; i++) { snapdisk = &(snapdef->disks[i]); - defdisk = snapdef->common.dom->disks[snapdisk->idx]; + defdisk = snapdef->parent.dom->disks[snapdisk->idx]; if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; @@ -15621,19 +15621,19 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, /* reject snapshot names containing slashes or starting with dot as * snapshot definitions are saved in files named by the snapshot name */ if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) { - if (strchr(def->common.name, '/')) { + if (strchr(def->parent.name, '/')) { virReportError(VIR_ERR_XML_DETAIL, _("invalid snapshot name '%s': " "name can't contain '/'"), - def->common.name); + def->parent.name); goto cleanup; } - if (def->common.name[0] == '.') { + if (def->parent.name[0] == '.') { virReportError(VIR_ERR_XML_DETAIL, _("invalid snapshot name '%s': " "name can't start with '.'"), - def->common.name); + def->parent.name); goto cleanup; } } @@ -15704,7 +15704,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * conversion in and back out of xml. */ if (!(xml = qemuDomainDefFormatLive(driver, vm->def, priv->origCPU, true, true)) || - !(def->common.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL, + !(def->parent.dom = virDomainDefParseString(xml, caps, driver->xmlopt, NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 626167f947..aaf67eaac9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6403,7 +6403,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, &update_current, flags) < 0) goto cleanup; } else { - if (!(def->common.dom = virDomainDefCopy(vm->def, + if (!(def->parent.dom = virDomainDefCopy(vm->def, privconn->caps, privconn->xmlopt, NULL, diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 94830f5ddb..af557690c4 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4811,7 +4811,7 @@ vboxSnapshotRedefine(virDomainPtr dom, * read-only disks are in the redefined snapshot's media registry (the disks need to * be open to query their uuid). */ - for (it = 0; it < def->common.dom->ndisks; it++) { + for (it = 0; it < def->parent.dom->ndisks; it++) { int diskInMediaRegistry = 0; IMedium *readOnlyMedium = NULL; PRUnichar *locationUtf = NULL; @@ -4825,7 +4825,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); diskInMediaRegistry = virVBoxSnapshotConfDiskIsInMediaRegistry(snapshotMachineDesc, - def->common.dom->disks[it]->src->path); + def->parent.dom->disks[it]->src->path); if (diskInMediaRegistry == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to know if disk is in media registry")); @@ -4835,7 +4835,7 @@ vboxSnapshotRedefine(virDomainPtr dom, continue; /*The read only disk is not in the media registry*/ - VBOX_UTF8_TO_UTF16(def->common.dom->disks[it]->src->path, &locationUtf); + VBOX_UTF8_TO_UTF16(def->parent.dom->disks[it]->src->path, &locationUtf); rc = gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj, locationUtf, DeviceType_HardDisk, @@ -4908,7 +4908,7 @@ vboxSnapshotRedefine(virDomainPtr dom, readOnlyDisk->format = format; readOnlyDisk->uuid = uuid; - if (VIR_STRDUP(readOnlyDisk->location, def->common.dom->disks[it]->src->path) < 0) { + if (VIR_STRDUP(readOnlyDisk->location, def->parent.dom->disks[it]->src->path) < 0) { VIR_FREE(readOnlyDisk); goto cleanup; } @@ -5013,12 +5013,12 @@ vboxSnapshotRedefine(virDomainPtr dom, goto cleanup; VIR_DEBUG("New snapshot UUID: %s", newSnapshotPtr->uuid); - if (VIR_STRDUP(newSnapshotPtr->name, def->common.name) < 0) + if (VIR_STRDUP(newSnapshotPtr->name, def->parent.name) < 0) goto cleanup; - newSnapshotPtr->timeStamp = virTimeStringThen(def->common.creationTime * 1000); + newSnapshotPtr->timeStamp = virTimeStringThen(def->parent.creationTime * 1000); - if (VIR_STRDUP(newSnapshotPtr->description, def->common.description) < 0) + if (VIR_STRDUP(newSnapshotPtr->description, def->parent.description) < 0) goto cleanup; if (VIR_STRDUP(newSnapshotPtr->hardware, snapshotMachineDesc->hardware) < 0) @@ -5028,12 +5028,12 @@ vboxSnapshotRedefine(virDomainPtr dom, goto cleanup; /*We get the parent disk uuid from the parent disk location to correctly fill the storage controller.*/ - for (it = 0; it < def->common.dom->ndisks; it++) { + for (it = 0; it < def->parent.dom->ndisks; it++) { char *location = NULL; const char *uuidReplacing = NULL; char *tmp = NULL; - location = def->common.dom->disks[it]->src->path; + location = def->parent.dom->disks[it]->src->path; if (!location) goto cleanup; /*Replacing the uuid*/ @@ -5061,7 +5061,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(tmp); } - if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->common.parent_name) < 0) { + if (virVBoxSnapshotConfAddSnapshotToXmlMachine(newSnapshotPtr, snapshotMachineDesc, def->parent.parent_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to add the snapshot to the machine description")); goto cleanup; @@ -5081,10 +5081,10 @@ vboxSnapshotRedefine(virDomainPtr dom, * Open the snapshot's read-write disk's full ancestry to allow opening the * read-write disk itself. */ - for (it = 0; it < def->common.dom->ndisks; it++) { + for (it = 0; it < def->parent.dom->ndisks; it++) { char *location = NULL; - location = def->common.dom->disks[it]->src->path; + location = def->parent.dom->disks[it]->src->path; if (!location) goto cleanup; @@ -5228,7 +5228,7 @@ vboxSnapshotRedefine(virDomainPtr dom, } } else { /*Create a "fake" disk to avoid corrupting children snapshot disks.*/ - for (it = 0; it < def->common.dom->ndisks; it++) { + for (it = 0; it < def->parent.dom->ndisks; it++) { IMedium *medium = NULL; PRUnichar *locationUtf16 = NULL; char *parentUuid = NULL; @@ -5244,7 +5244,7 @@ vboxSnapshotRedefine(virDomainPtr dom, VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); - VBOX_UTF8_TO_UTF16(def->common.dom->disks[it]->src->path, &locationUtf16); + VBOX_UTF8_TO_UTF16(def->parent.dom->disks[it]->src->path, &locationUtf16); rc = gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj, locationUtf16, DeviceType_HardDisk, @@ -5393,8 +5393,8 @@ vboxSnapshotRedefine(virDomainPtr dom, * All the snapshot structure manipulation is done, we close the disks we have * previously opened. */ - for (it = 0; it < def->common.dom->ndisks; it++) { - char *location = def->common.dom->disks[it]->src->path; + for (it = 0; it < def->parent.dom->ndisks; it++) { + char *location = def->parent.dom->disks[it]->src->path; if (!location) goto cleanup; @@ -5513,7 +5513,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) { if (vboxSnapshotRedefine(dom, def, isCurrent) < 0) goto cleanup; - ret = virGetDomainSnapshot(dom, def->common.name); + ret = virGetDomainSnapshot(dom, def->parent.name); goto cleanup; } } @@ -5540,14 +5540,14 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, goto cleanup; } - VBOX_UTF8_TO_UTF16(def->common.name, &name); + VBOX_UTF8_TO_UTF16(def->parent.name, &name); if (!name) { virReportOOMError(); goto cleanup; } - if (def->common.description) { - VBOX_UTF8_TO_UTF16(def->common.description, &description); + if (def->parent.description) { + VBOX_UTF8_TO_UTF16(def->parent.description, &description); if (!description) { virReportOOMError(); goto cleanup; @@ -5577,7 +5577,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, goto cleanup; } - ret = virGetDomainSnapshot(dom, def->common.name); + ret = virGetDomainSnapshot(dom, def->parent.name); cleanup: VBOX_RELEASE(progress); @@ -5981,7 +5981,7 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotDefPtr def, vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER; size_t i = 0, diskCount = 0, sdCount = 0; int ret = -1; - virDomainDefPtr defdom = def->common.dom; + virDomainDefPtr defdom = def->parent.dom; if (!data->vboxObj) return ret; @@ -6220,10 +6220,10 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) goto cleanup; - if (VIR_ALLOC(def) < 0 || !(def->common.dom = virDomainDefNew())) + if (VIR_ALLOC(def) < 0 || !(def->parent.dom = virDomainDefNew())) goto cleanup; - defdom = def->common.dom; - if (VIR_STRDUP(def->common.name, snapshot->name) < 0) + defdom = def->parent.dom; + if (VIR_STRDUP(def->parent.name, snapshot->name) < 0) goto cleanup; if (gVBoxAPI.vboxSnapshotRedefine) { @@ -6271,7 +6271,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (str16) { VBOX_UTF16_TO_UTF8(str16, &str8); VBOX_UTF16_FREE(str16); - if (VIR_STRDUP(def->common.description, str8) < 0) { + if (VIR_STRDUP(def->parent.description, str8) < 0) { VBOX_UTF8_FREE(str8); goto cleanup; } @@ -6286,7 +6286,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, goto cleanup; } /* timestamp is in milliseconds while creationTime in seconds */ - def->common.creationTime = timestamp / 1000; + def->parent.creationTime = timestamp / 1000; rc = gVBoxAPI.UISnapshot.GetParent(snap, &parent); if (NS_FAILED(rc)) { @@ -6305,7 +6305,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, } VBOX_UTF16_TO_UTF8(str16, &str8); VBOX_UTF16_FREE(str16); - if (VIR_STRDUP(def->common.parent_name, str8) < 0) { + if (VIR_STRDUP(def->parent.parent_name, str8) < 0) { VBOX_UTF8_FREE(str8); goto cleanup; } @@ -6987,7 +6987,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) goto cleanup; } - isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, def->common.name); + isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc, def->parent.name); if (isCurrent < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to know if the snapshot is the current snapshot")); @@ -6999,8 +6999,8 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) * disks. The first thing to do is to manipulate VirtualBox API to create * differential read-write disks if the parent snapshot is not null. */ - if (def->common.parent_name != NULL) { - for (it = 0; it < def->common.dom->ndisks; it++) { + if (def->parent.parent_name != NULL) { + for (it = 0; it < def->parent.dom->ndisks; it++) { virVBoxSnapshotConfHardDiskPtr readOnly = NULL; IMedium *medium = NULL; PRUnichar *locationUtf16 = NULL; @@ -7020,7 +7020,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_IID_INITIALIZE(&iid); VBOX_IID_INITIALIZE(&parentiid); readOnly = virVBoxSnapshotConfHardDiskPtrByLocation(snapshotMachineDesc, - def->common.dom->disks[it]->src->path); + def->parent.dom->disks[it]->src->path); if (!readOnly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get hard disk by location")); @@ -7058,7 +7058,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_UTF8_TO_UTF16("VDI", &formatUtf16); if (virAsprintf(&newLocationUtf8, "%sfakedisk-%s-%d.vdi", - machineLocationPath, def->common.parent_name, it) < 0) + machineLocationPath, def->parent.parent_name, it) < 0) goto cleanup; VBOX_UTF8_TO_UTF16(newLocationUtf8, &newLocation); rc = gVBoxAPI.UIVirtualBox.CreateHardDisk(data->vboxObj, @@ -7156,15 +7156,15 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } } } else { - for (it = 0; it < def->common.dom->ndisks; it++) { + for (it = 0; it < def->parent.dom->ndisks; it++) { const char *uuidRO = NULL; char *tmp = NULL; uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, - def->common.dom->disks[it]->src->path); + def->parent.dom->disks[it]->src->path); if (!uuidRO) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No such disk in media registry %s"), - def->common.dom->disks[it]->src->path); + def->parent.dom->disks[it]->src->path); goto cleanup; } @@ -7209,14 +7209,14 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } } /*If the parent snapshot is not NULL, we remove the-read only disks from the media registry*/ - if (def->common.parent_name != NULL) { - for (it = 0; it < def->common.dom->ndisks; it++) { + if (def->parent.parent_name != NULL) { + for (it = 0; it < def->parent.dom->ndisks; it++) { const char *uuidRO = virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc, - def->common.dom->disks[it]->src->path); + def->parent.dom->disks[it]->src->path); if (!uuidRO) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find UUID for location %s"), def->common.dom->disks[it]->src->path); + _("Unable to find UUID for location %s"), def->parent.dom->disks[it]->src->path); goto cleanup; } if (virVBoxSnapshotConfRemoveHardDisk(snapshotMachineDesc->mediaRegistry, uuidRO) < 0) { @@ -7281,16 +7281,16 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) } /*removing the snapshot*/ - if (virVBoxSnapshotConfRemoveSnapshot(snapshotMachineDesc, def->common.name) < 0) { + if (virVBoxSnapshotConfRemoveSnapshot(snapshotMachineDesc, def->parent.name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to remove snapshot %s"), def->common.name); + _("Unable to remove snapshot %s"), def->parent.name); goto cleanup; } if (isCurrent) { VIR_FREE(snapshotMachineDesc->currentSnapshot); - if (def->common.parent_name != NULL) { - virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->common.parent_name); + if (def->parent.parent_name != NULL) { + virVBoxSnapshotConfSnapshotPtr snap = virVBoxSnapshotConfSnapshotByName(snapshotMachineDesc->snapshot, def->parent.parent_name); if (!snap) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to get the snapshot to remove")); diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 83bffd95b2..419e9d215f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2619,7 +2619,7 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; /* snaphot name is ignored, it will be set to auto generated by sdk uuid */ - if (prlsdkCreateSnapshot(dom, def->common.description) < 0) + if (prlsdkCreateSnapshot(dom, def->parent.description) < 0) goto cleanup; if (!(snapshots = prlsdkLoadSnapshots(dom))) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 500de5f8ba..478443298f 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4696,14 +4696,14 @@ prlsdkParseSnapshotTree(const char *treexml) ctxt->node = nodes[i]; - def->common.name = virXPathString("string(./@guid)", ctxt); - if (!def->common.name) { + def->parent.name = virXPathString("string(./@guid)", ctxt); + if (!def->parent.name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing 'guid' attribute")); goto cleanup; } - def->common.parent_name = virXPathString("string(../@guid)", ctxt); + def->parent.parent_name = virXPathString("string(../@guid)", ctxt); xmlstr = virXPathString("string(./DateTime)", ctxt); if (!xmlstr) { @@ -4711,11 +4711,11 @@ prlsdkParseSnapshotTree(const char *treexml) _("missing 'DateTime' element")); goto cleanup; } - if ((def->common.creationTime = prlsdkParseDateTime(xmlstr)) < 0) + if ((def->parent.creationTime = prlsdkParseDateTime(xmlstr)) < 0) goto cleanup; VIR_FREE(xmlstr); - def->common.description = virXPathString("string(./Description)", ctxt); + def->parent.description = virXPathString("string(./Description)", ctxt); def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; xmlstr = virXPathString("string(./@state)", ctxt); -- 2.20.1
On Wed, May 08, 2019 at 17:24:10 -0500, Eric Blake wrote:
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as the name of their inherited member at offset 0. While it would be possible to write a new class-creation macro that takes the actual field name 'current', and rewrite VIR_CLASS_NEW to call the new macro with the hard-coded name 'parent', it seems less confusing if all object code uses similar naming. Thus, this is a mechanical rename in preparation of making virDomainSnapshotDef a descendent of virObject.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/snapshot_conf.h | 2 +- src/conf/snapshot_conf.c | 120 ++++++++++++++-------------- src/conf/virdomainsnapshotobjlist.c | 2 +- src/esx/esx_driver.c | 16 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 12 +-- src/test/test_driver.c | 2 +- src/vbox/vbox_common.c | 88 ++++++++++---------- src/vz/vz_driver.c | 2 +- src/vz/vz_sdk.c | 10 +-- 10 files changed, 128 insertions(+), 128 deletions(-)
ACK
In preparation for making virDomainSnapshotDef a descendant of virObject, it is time to fix all callers that allocate an object to use virDomainSnapshotDefNew() instead of VIR_ALLOC(). Fortunately, there aren't very many :) Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/snapshot_conf.h | 1 + src/conf/snapshot_conf.c | 16 +++++++++++++--- src/libvirt_private.syms | 1 + src/vbox/vbox_common.c | 3 ++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index f54be11619..0ce9dda355 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -114,6 +114,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml, virDomainXMLOptionPtr xmlopt, bool *current, unsigned int flags); +virDomainSnapshotDefPtr virDomainSnapshotDefNew(void); void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); char *virDomainSnapshotDefFormat(const char *uuidstr, virDomainSnapshotDefPtr def, diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index dd281d57fe..e5771ae635 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -81,7 +81,17 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) disk->src = NULL; } -void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) +virDomainSnapshotDefPtr +virDomainSnapshotDefNew(void) +{ + virDomainSnapshotDefPtr def; + + ignore_value(VIR_ALLOC(def)); + return def; +} + +void +virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) { size_t i; @@ -205,8 +215,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE); virSaveCookieCallbacksPtr saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt); - if (VIR_ALLOC(def) < 0) - goto cleanup; + if (!(def = virDomainSnapshotDefNew())) + return NULL; def->parent.name = virXPathString("string(./name)", ctxt); if (def->parent.name == NULL) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a03cf0b645..0474a4e44c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -894,6 +894,7 @@ virDomainSnapshotAlignDisks; virDomainSnapshotDefFormat; virDomainSnapshotDefFree; virDomainSnapshotDefIsExternal; +virDomainSnapshotDefNew; virDomainSnapshotDefParseString; virDomainSnapshotFormatConvertXMLFlags; virDomainSnapshotIsExternal; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index af557690c4..7e42f6a4fe 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -6220,7 +6220,8 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name))) goto cleanup; - if (VIR_ALLOC(def) < 0 || !(def->parent.dom = virDomainDefNew())) + if (!(def = virDomainSnapshotDefNew()) || + !(def->parent.dom = virDomainDefNew())) goto cleanup; defdom = def->parent.dom; if (VIR_STRDUP(def->parent.name, snapshot->name) < 0) -- 2.20.1
On Wed, May 08, 2019 at 17:24:11 -0500, Eric Blake wrote:
In preparation for making virDomainSnapshotDef a descendant of virObject, it is time to fix all callers that allocate an object to use virDomainSnapshotDefNew() instead of VIR_ALLOC(). Fortunately, there aren't very many :)
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/snapshot_conf.h | 1 + src/conf/snapshot_conf.c | 16 +++++++++++++--- src/libvirt_private.syms | 1 + src/vbox/vbox_common.c | 3 ++- 4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index f54be11619..0ce9dda355 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -114,6 +114,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml, virDomainXMLOptionPtr xmlopt, bool *current, unsigned int flags); +virDomainSnapshotDefPtr virDomainSnapshotDefNew(void); void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); char *virDomainSnapshotDefFormat(const char *uuidstr, virDomainSnapshotDefPtr def, diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index dd281d57fe..e5771ae635 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -81,7 +81,17 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) disk->src = NULL; }
-void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) +virDomainSnapshotDefPtr +virDomainSnapshotDefNew(void) +{ + virDomainSnapshotDefPtr def; + + ignore_value(VIR_ALLOC(def)); + return def; +} + +void +virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
This is changed/deleted again in next patch so this patch shouldn't tweak the style. ACK
This brings about a couple of benefits: - use of VIR_AUTOUNREF() simplifies several callers - Fixes a todo about virDomainMomentObjList not being polymorphic enough Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/moment_conf.h | 5 ++++- src/conf/snapshot_conf.h | 1 - cfg.mk | 2 -- src/conf/moment_conf.c | 28 ++++++++++++++++++++++++- src/conf/snapshot_conf.c | 34 ++++++++++++++++++++++--------- src/conf/virdomainmomentobjlist.c | 3 +-- src/esx/esx_driver.c | 3 +-- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 5 ++--- src/test/test_driver.c | 5 ++--- src/vbox/vbox_common.c | 6 ++---- src/vz/vz_driver.c | 3 +-- tests/domainsnapshotxml2xmltest.c | 3 +-- 13 files changed, 65 insertions(+), 34 deletions(-) diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h index 04e0c0648b..00ec1c1904 100644 --- a/src/conf/moment_conf.h +++ b/src/conf/moment_conf.h @@ -25,9 +25,12 @@ # include "internal.h" # include "virconftypes.h" +# include "virobject.h" /* Base class for a domain moment */ struct _virDomainMomentDef { + virObject parent; + /* Common portion of public XML. */ char *name; char *description; @@ -37,7 +40,7 @@ struct _virDomainMomentDef { virDomainDefPtr dom; }; -void virDomainMomentDefClear(virDomainMomentDefPtr def); +virClassPtr virClassForDomainMomentDef(void); int virDomainMomentDefPostParse(virDomainMomentDefPtr def); diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 0ce9dda355..55b7487cfb 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -115,7 +115,6 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseNode(xmlDocPtr xml, bool *current, unsigned int flags); virDomainSnapshotDefPtr virDomainSnapshotDefNew(void); -void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); char *virDomainSnapshotDefFormat(const char *uuidstr, virDomainSnapshotDefPtr def, virCapsPtr caps, diff --git a/cfg.mk b/cfg.mk index b785089910..786aa6e80a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -133,7 +133,6 @@ useless_free_options = \ --name=virDomainNetDefFree \ --name=virDomainObjFree \ --name=virDomainSmartcardDefFree \ - --name=virDomainSnapshotDefFree \ --name=virDomainSnapshotObjFree \ --name=virDomainSoundDefFree \ --name=virDomainVideoDefFree \ @@ -211,7 +210,6 @@ useless_free_options = \ # y virDomainInputDefFree # y virDomainNetDefFree # y virDomainObjFree -# y virDomainSnapshotDefFree # n virDomainSnapshotFree (returns int) # n virDomainSnapshotFreeName (returns int) # y virDomainSnapshotObjFree diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c index 9829775b3c..fea13f0f97 100644 --- a/src/conf/moment_conf.c +++ b/src/conf/moment_conf.c @@ -34,8 +34,34 @@ VIR_LOG_INIT("conf.moment_conf"); -void virDomainMomentDefClear(virDomainMomentDefPtr def) +static virClassPtr virDomainMomentDefClass; +static void virDomainMomentDefDispose(void *obj); + +static int +virDomainMomentOnceInit(void) { + if (!VIR_CLASS_NEW(virDomainMomentDef, virClassForObject())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virDomainMoment); + +virClassPtr +virClassForDomainMomentDef(void) +{ + if (virDomainMomentInitialize() < 0) + return NULL; + + return virDomainMomentDefClass; +} + +static void +virDomainMomentDefDispose(void *obj) +{ + virDomainMomentDefPtr def = obj; + VIR_FREE(def->name); VIR_FREE(def->description); VIR_FREE(def->parent_name); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index e5771ae635..c7f29360e7 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -50,6 +50,20 @@ VIR_LOG_INIT("conf.snapshot_conf"); +static virClassPtr virDomainSnapshotDefClass; +static void virDomainSnapshotDefDispose(void *obj); + +static int +virDomainSnapshotOnceInit(void) +{ + if (!VIR_CLASS_NEW(virDomainSnapshotDef, virClassForDomainMomentDef())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virDomainSnapshot); + VIR_ENUM_IMPL(virDomainSnapshotLocation, VIR_DOMAIN_SNAPSHOT_LOCATION_LAST, "default", @@ -81,30 +95,30 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) disk->src = NULL; } +/* Allocate a new virDomainSnapshotDef; free with virObjectUnref() */ virDomainSnapshotDefPtr virDomainSnapshotDefNew(void) { virDomainSnapshotDefPtr def; - ignore_value(VIR_ALLOC(def)); + if (virDomainSnapshotInitialize() < 0) + return NULL; + + def = virObjectNew(virDomainSnapshotDefClass); return def; } -void -virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) +static void +virDomainSnapshotDefDispose(void *obj) { + virDomainSnapshotDefPtr def = obj; size_t i; - if (!def) - return; - - virDomainMomentDefClear(&def->parent); VIR_FREE(def->file); for (i = 0; i < def->ndisks; i++) virDomainSnapshotDiskDefClear(&def->disks[i]); VIR_FREE(def->disks); virObjectUnref(def->cookie); - VIR_FREE(def); } static int @@ -374,7 +388,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, VIR_FREE(nodes); VIR_FREE(memorySnapshot); VIR_FREE(memoryFile); - virDomainSnapshotDefFree(def); + virObjectUnref(def); return ret; } @@ -996,7 +1010,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, /* Drop and rebuild the parent relationship, but keep all * child relations by reusing snap. */ virDomainMomentDropParent(other); - virDomainSnapshotDefFree(otherdef); + virObjectUnref(otherdef); other->def = &(*defptr)->parent; *defptr = NULL; *snap = other; diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index e9df66c65b..f56b516343 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -208,8 +208,7 @@ virDomainMomentObjFree(virDomainMomentObjPtr moment) VIR_DEBUG("obj=%p", moment); - /* FIXME: Make this polymorphic by inheriting from virObject */ - virDomainSnapshotDefFree(virDomainSnapshotObjGetDef(moment)); + virObjectUnref(moment->def); VIR_FREE(moment); } diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f45d96a4f5..deb800a6b7 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4081,7 +4081,6 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags) { esxPrivate *priv = domain->conn->privateData; - virDomainSnapshotDefPtr def = NULL; esxVI_ObjectContent *virtualMachine = NULL; esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; @@ -4091,6 +4090,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, virDomainSnapshotPtr snapshot = NULL; bool diskOnly = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0; bool quiesce = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; /* ESX supports disk-only and quiesced snapshots; libvirt tracks no * snapshot metadata so supporting that flag is trivial. */ @@ -4151,7 +4151,6 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, snapshot = virGetDomainSnapshot(domain, def->parent.name); cleanup: - virDomainSnapshotDefFree(def); esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0474a4e44c..909975750c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -892,7 +892,6 @@ virSecretEventValueChangedNew; # conf/snapshot_conf.h virDomainSnapshotAlignDisks; virDomainSnapshotDefFormat; -virDomainSnapshotDefFree; virDomainSnapshotDefIsExternal; virDomainSnapshotDefNew; virDomainSnapshotDefParseString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 833c1e88ee..2877642c4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -481,7 +481,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm, snap = virDomainSnapshotAssignDef(vm->snapshots, def); if (snap == NULL) { - virDomainSnapshotDefFree(def); + virObjectUnref(def); } else if (cur) { if (current) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -15552,7 +15552,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, char *xml = NULL; virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; - virDomainSnapshotDefPtr def = NULL; virDomainMomentObjPtr current = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; @@ -15564,6 +15563,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; + VIR_AUTOFREE(virDomainSnapshotDefPtr) def = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | @@ -15831,7 +15831,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainObjEndAPI(&vm); - virDomainSnapshotDefFree(def); VIR_FREE(xml); virObjectUnref(caps); virObjectUnref(cfg); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index aaf67eaac9..a06d1fc402 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -848,7 +848,7 @@ testParseDomainSnapshots(testDriverPtr privconn, goto error; if (!(snap = virDomainSnapshotAssignDef(domobj->snapshots, def))) { - virDomainSnapshotDefFree(def); + virObjectUnref(def); goto error; } @@ -6348,13 +6348,13 @@ testDomainSnapshotCreateXML(virDomainPtr domain, { testDriverPtr privconn = domain->conn->privateData; virDomainObjPtr vm = NULL; - virDomainSnapshotDefPtr def = NULL; virDomainMomentObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virObjectEventPtr event = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; /* * DISK_ONLY: Not implemented yet @@ -6448,7 +6448,6 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainObjEndAPI(&vm); } virObjectEventStateQueue(privconn->eventState, event); - virDomainSnapshotDefFree(def); return snapshot; } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 7e42f6a4fe..54e31bec9d 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -5476,7 +5476,6 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, unsigned int flags) { vboxDriverPtr data = dom->conn->privateData; - virDomainSnapshotDefPtr def = NULL; vboxIID domiid; IMachine *machine = NULL; IConsole *console = NULL; @@ -5488,6 +5487,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, nsresult rc; resultCodeUnion result; virDomainSnapshotPtr ret = NULL; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; if (!data->vboxObj) return ret; @@ -5587,7 +5587,6 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, gVBoxAPI.UISession.Close(data->vboxSession); VBOX_RELEASE(machine); vboxIIDUnalloc(&domiid); - virDomainSnapshotDefFree(def); return ret; } @@ -6200,7 +6199,6 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, ISnapshot *snap = NULL; ISnapshot *parent = NULL; nsresult rc; - virDomainSnapshotDefPtr def = NULL; PRUnichar *str16; char *str8; PRInt64 timestamp; @@ -6208,6 +6206,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, char uuidstr[VIR_UUID_STRING_BUFLEN]; char *ret = NULL; virDomainDefPtr defdom; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; if (!data->vboxObj) return ret; @@ -6330,7 +6329,6 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, ret = virDomainSnapshotDefFormat(uuidstr, def, data->caps, data->xmlopt, 0); cleanup: - virDomainSnapshotDefFree(def); VBOX_RELEASE(parent); VBOX_RELEASE(snap); VBOX_RELEASE(machine); diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 419e9d215f..dfd49e7cc7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2576,7 +2576,6 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, unsigned int flags) { - virDomainSnapshotDefPtr def = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainObjPtr dom; vzConnPtr privconn = domain->conn->privateData; @@ -2585,6 +2584,7 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotObjListPtr snapshots = NULL; virDomainMomentObjPtr current; bool job = false; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; virCheckFlags(0, NULL); @@ -2636,7 +2636,6 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainSnapshotObjListFree(snapshots); - virDomainSnapshotDefFree(def); if (job) vzDomainObjEndJob(dom); virDomainObjEndAPI(&dom); diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 18ff2dc34c..c2c7bedb56 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -35,10 +35,10 @@ testCompareXMLToXMLFiles(const char *inxml, char *outXmlData = NULL; char *actual = NULL; int ret = -1; - virDomainSnapshotDefPtr def = NULL; unsigned int parseflags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; unsigned int formatflags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE; bool cur = false; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; if (flags & TEST_INTERNAL) { parseflags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; @@ -85,7 +85,6 @@ testCompareXMLToXMLFiles(const char *inxml, VIR_FREE(inXmlData); VIR_FREE(outXmlData); VIR_FREE(actual); - virDomainSnapshotDefFree(def); return ret; } -- 2.20.1
On Wed, May 08, 2019 at 17:24:12 -0500, Eric Blake wrote:
This brings about a couple of benefits: - use of VIR_AUTOUNREF() simplifies several callers - Fixes a todo about virDomainMomentObjList not being polymorphic enough
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/moment_conf.h | 5 ++++- src/conf/snapshot_conf.h | 1 - cfg.mk | 2 -- src/conf/moment_conf.c | 28 ++++++++++++++++++++++++- src/conf/snapshot_conf.c | 34 ++++++++++++++++++++++--------- src/conf/virdomainmomentobjlist.c | 3 +-- src/esx/esx_driver.c | 3 +-- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 5 ++--- src/test/test_driver.c | 5 ++--- src/vbox/vbox_common.c | 6 ++---- src/vz/vz_driver.c | 3 +-- tests/domainsnapshotxml2xmltest.c | 3 +-- 13 files changed, 65 insertions(+), 34 deletions(-)
ACK
participants (2)
-
Eric Blake -
Peter Krempa