Changeset 24509 in vbox

Timestamp:

Nov 9, 2009 2:59:05 PM (15 years ago)

Author:

vboxsync

Message:

Main: fix deleteSnapshot failure: medium backreferences were not updated correctly after attachment of a new disk so that detachFrom() failed if the disk had been attached in the same session (e.g. through takeSnapshot())

Location:

trunk/src/VBox/Main

Files:

• MediumImpl.cpp (modified) (13 diffs)
• SnapshotImpl.cpp (modified) (1 diff)

trunk/src/VBox/Main/MediumImpl.cpp

@@ -71 +71 @@
             const Guid &aSnapshotId = Guid::Empty)
         : machineId(aMachineId),
-          inCurState(aSnapshotId.isEmpty())
+          fInCurState(aSnapshotId.isEmpty())
     {
         if (!aSnapshotId.isEmpty())
@@ -78 +78 @@
 
     Guid machineId;
-    bool inCurState : 1;
+    bool fInCurState : 1;
     GuidList llSnapshotIds;
 };
@@ -1793 +1793 @@
             /* if the medium is attached to the machine in the current state, we
              * return its ID as the first element of the array */
-            if (it->inCurState)
-                ++ size;
+            if (it->fInCurState)
+                ++size;
 
             if (size > 0)
@@ -1801 +1801 @@
 
                 size_t j = 0;
-                if (it->inCurState)
-                    it->machineId.toUtf16().detachTo(&snapshotIds [j ++]);
+                if (it->fInCurState)
+                    it->machineId.toUtf16().detachTo(&snapshotIds[j++]);
 
                 for (BackRef::GuidList::const_iterator jt = it->llSnapshotIds.begin();
@@ -1808 +1808 @@
                      ++jt, ++j)
                 {
-                     (*jt).toUtf16().detachTo(&snapshotIds [j]);
+                     (*jt).toUtf16().detachTo(&snapshotIds[j]);
                 }
             }
@@ -2689 +2689 @@
     }
 
+    // if the caller has not supplied a snapshot ID, then we're attaching
+    // to a machine a medium which represents the machine's current state,
+    // so set the flag
     if (aSnapshotId.isEmpty())
     {
         /* sanity: no duplicate attachments */
-        AssertReturn(!it->inCurState, E_FAIL);
-        it->inCurState = true;
+        AssertReturn(!it->fInCurState, E_FAIL);
+        it->fInCurState = true;
 
         return S_OK;
     }
+
+    // otherwise: a snapshot medium is being attached
 
     /* sanity: no duplicate attachments */
@@ -2715 +2720 @@
 
     it->llSnapshotIds.push_back(aSnapshotId);
+    it->fInCurState = false;
 
     LogFlowThisFuncLeave();
@@ -2747 +2753 @@
     {
         /* remove the current state attachment */
-        it->inCurState = false;
+        it->fInCurState = false;
     }
     else
@@ -2760 +2766 @@
 
     /* if the backref becomes empty, remove it */
-    if (it->inCurState == false && it->llSnapshotIds.size() == 0)
+    if (it->fInCurState == false && it->llSnapshotIds.size() == 0)
         m->backRefs.erase(it);
 
@@ -2783 +2789 @@
     {
         const BackRef &ref = *it2;
-        LogFlowThisFunc(("  Backref from machine {%RTuuid}\n", ref.machineId.raw()));
+        LogFlowThisFunc(("  Backref from machine {%RTuuid} (fInCurState: %d)\n", ref.machineId.raw(), ref.fInCurState));
 
         for (BackRef::GuidList::const_iterator jt2 = it2->llSnapshotIds.begin();
@@ -2874 +2880 @@
         std::find_if(m->backRefs.begin(), m->backRefs.end(),
                         BackRef::EqualsTo(aMachineId));
-    return it != m->backRefs.end() && it->inCurState;
+    return it != m->backRefs.end() && it->fInCurState;
 }
 
@@ -3190 +3196 @@
      * (the code calling prepareDiscard() should detach). */
     AssertReturn(m->backRefs.size() == 1, E_FAIL);
-    AssertReturn(!m->backRefs.front().inCurState, E_FAIL);
+    AssertReturn(!m->backRefs.front().fInCurState, E_FAIL);
     AssertReturn(m->backRefs.front().llSnapshotIds.size() == 1, E_FAIL);
 
@@ -4234 +4240 @@
          it != m->backRefs.end(); ++ it)
     {
-        if (it->inCurState)
+        if (it->fInCurState)
         {
             /* Note: when a VM snapshot is being taken, all normal hard disks

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -2128 +2128 @@
                         // do not lock, prepareDiscared() has a write lock which will hang otherwise
 
+#ifdef DEBUG
+                pHD->dumpBackRefs();
+#endif
+
                 Medium::MergeChain *chain = NULL;