Changeset 49687 in vbox

Timestamp:

Nov 27, 2013 6:00:49 PM (11 years ago)

Author:

vboxsync

Message:

Main/Snapshot: block deleting of current snapshot if it has child snapshots, as this would lead to VM config corruption, plus safety belts in the GUI and settings reader

Location:

trunk

Files:

• doc/manual/user_ChangeLogImpl.xml (modified) (1 diff)
• include/VBox/settings.h (modified) (1 diff)
• src/VBox/Frontends/VirtualBox/src/selector/VBoxSnapshotsWgt.cpp (modified) (1 diff)
• src/VBox/Main/src-server/SnapshotImpl.cpp (modified) (1 diff)
• src/VBox/Main/xml/Settings.cpp (modified) (5 diffs)

trunk/doc/manual/user_ChangeLogImpl.xml

@@ -18 +18 @@
 
     <itemizedlist>
+
+      <listitem>
+        <para>API: block the removal of the current snapshot if it has child
+           snapshots (only relevant for VMs without snapshottable hard disks,
+           their presence always prevented removal), which resulted in VM
+           config corruption</para>
+      </listitem>
+
+      <listitem>
+        <para>API: mark VM configs with snapshots but without current snapshot
+           as inaccessible, as this combination is nonsense</para>
+      </listitem>
 
       <listitem>

trunk/include/VBox/settings.h

@@ -1276 +1276 @@
     void readAutostart(const xml::ElementNode *pElmAutostart, Autostart *pAutostart);
     void readGroups(const xml::ElementNode *elmGroups, StringsList *pllGroups);
-    void readSnapshot(uint32_t depth, const xml::ElementNode &elmSnapshot, Snapshot &snap);
+    bool readSnapshot(const com::Guid &curSnapshotUuid, uint32_t depth, const xml::ElementNode &elmSnapshot, Snapshot &snap);
     void convertOldOSType_pre1_5(com::Utf8Str &str);
     void readMachine(const xml::ElementNode &elmMachine);

trunk/src/VBox/Frontends/VirtualBox/src/selector/VBoxSnapshotsWgt.cpp

@@ -1018 +1018 @@
     item->recache();
 
-    if (mMachine.GetCurrentSnapshot().GetId() == aSnapshot.GetId())
+    CSnapshot curSnapshot = mMachine.GetCurrentSnapshot();
+    if (!curSnapshot.isNull() && curSnapshot.GetId() == aSnapshot.GetId())
     {
         item->setBold (true);

trunk/src/VBox/Main/src-server/SnapshotImpl.cpp

@@ -2169 +2169 @@
                         mUserData->s.strName.c_str(),
                         childrenCount);
+    if (pSnapshot == mData->mCurrentSnapshot && childrenCount >= 1)
+        return setError(VBOX_E_INVALID_OBJECT_STATE,
+                        tr("Snapshot '%s' of the machine '%s' cannot be deleted, because it is the current snapshot and has one child snapshot"),
+                        pSnapshot->getName().c_str(),
+                        mUserData->s.strName.c_str());
 
     /* If the snapshot being deleted is the current one, ensure current

trunk/src/VBox/Main/xml/Settings.cpp

@@ -3698 +3698 @@
  * Snapshot structure.
  *
+ * @param curSnapshotUuid
  * @param depth
  * @param elmSnapshot
  * @param snap
- */
-void MachineConfigFile::readSnapshot(uint32_t depth,
+ * @returns true if curSnapshotUuid is in this snapshot subtree, otherwise false
+ */
+bool MachineConfigFile::readSnapshot(const Guid &curSnapshotUuid,
+                                     uint32_t depth,
                                      const xml::ElementNode &elmSnapshot,
                                      Snapshot &snap)
@@ -3714 +3717 @@
         throw ConfigFileError(this, &elmSnapshot, N_("Required Snapshot/@uuid attribute is missing"));
     parseUUID(snap.uuid, strTemp);
+    bool foundCurrentSnapshot = (snap.uuid == curSnapshotUuid);
 
     if (!elmSnapshot.getAttributeValue("name", snap.strName))
@@ -3758 +3762 @@
                     // small, especially with XPCOM.
                     Snapshot *child = new Snapshot();
-                    readSnapshot(depth + 1, *pelmChildSnapshot, *child);
+                    bool found = readSnapshot(curSnapshotUuid, depth + 1, *pelmChildSnapshot, *child);
+                    foundCurrentSnapshot = foundCurrentSnapshot || found;
                     snap.llChildSnapshots.push_back(*child);
                     delete child;
@@ -3774 +3779 @@
     readAutostart(elmSnapshot.findChildElement("Autostart"), &snap.autostart);
     // note: Groups exist only for Machine, not for Snapshot
+
+    return foundCurrentSnapshot;
 }
 
@@ -3894 +3901 @@
             else if (pelmMachineChild->nameEquals("Snapshot"))
             {
+                if (uuidCurrentSnapshot.isZero())
+                    throw ConfigFileError(this, &elmMachine, N_("Snapshots present but required Machine/@currentSnapshot attribute is missing"));
+                bool foundCurrentSnapshot = false;
                 Snapshot snap;
                 // this will recurse into child snapshots, if necessary
-                readSnapshot(1, *pelmMachineChild, snap);
+                foundCurrentSnapshot = readSnapshot(uuidCurrentSnapshot, 1, *pelmMachineChild, snap);
+                if (!foundCurrentSnapshot)
+                    throw ConfigFileError(this, &elmMachine, N_("Snapshots present but none matches the UUID in the Machine/@currentSnapshot attribute"));
                 llFirstSnapshot.push_back(snap);
             }