Changeset 29036 in vbox for trunk

Timestamp:

May 4, 2010 2:54:12 PM (15 years ago)

Author:

vboxsync

Message:

Main/Snapshot: handle late failures more gracefully when the actual merging succeeded but something else failed. If the merge source is gone assume the merge worked and update the snapshot accordingly. Avoids inaccessible VM configs due to referring to an already gone image. Small additional cleanup with handling the flag whether VirtualBox.xml needs to be saved - do not depend on getting the "OR" done by the called method, do it explicitly.

File:

• trunk/src/VBox/Main/SnapshotImpl.cpp (modified) (6 diffs)

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -23 +23 @@
 #include "MachineImpl.h"
 #include "MediumImpl.h"
+#include "MediumFormatImpl.h"
 #include "Global.h"
 #include "ProgressImpl.h"
@@ -2474 +2475 @@
                     /* No need to hold the lock any longer. */
                     mLock.release();
+                    bool fNeedsSave = false;
                     rc = pMedium->deleteStorage(&aTask.pProgress,
                                                 true /* aWait */,
-                                                &fNeedsSaveSettings);
+                                                &fNeedsSave);
+                    fNeedsSaveSettings |= fNeedsSave;
                     if (FAILED(rc))
                         throw rc;
@@ -2486 +2489 @@
             else
             {
+                bool fNeedsSave = false;
                 if (it->mfNeedsOnlineMerge)
                 {
@@ -2504 +2508 @@
                                            it->mpMediumLockList,
                                            aTask.pProgress,
-                                           &fNeedsSaveSettings);
+                                           &fNeedsSave);
                 }
                 else
@@ -2516 +2520 @@
                                                &aTask.pProgress,
                                                true /* aWait */,
-                                               &fNeedsSaveSettings);
+                                               &fNeedsSave);
                 }
-
+                fNeedsSaveSettings |= fNeedsSave;
+
+                // If the merge failed, we need to do our best to have a usable
+                // VM configuration afterwards. The return code doesn't tell
+                // whether the merge completed and so we have to check if the
+                // source medium (diff images are always file based at the
+                // moment) is still there or not. Be careful not to lose the
+                // error code below, before the "Delayed failure exit".
                 if (FAILED(rc))
-                    throw rc;
+                {
+                    AutoReadLock mlock(it->mpSource COMMA_LOCKVAL_SRC_POS);
+                    const ComObjPtr<MediumFormat> &sourceFormat = it->mpSource->getMediumFormat();
+                    // No medium format description? get out of here.
+                    if (sourceFormat.isNull())
+                        throw rc;
+                    // Diff medium not backed by a file - cannot get status so
+                    // be pessimistic.
+                    if (!(sourceFormat->capabilities() & MediumFormatCapabilities_File))
+                        throw rc;
+                    const Utf8Str &loc = it->mpSource->getLocationFull();
+                    // Source medium is still there, so merge failed early.
+                    if (RTFileExists(loc.raw()))
+                        throw rc;
+
+                    // Source medium is gone. Assume the merge succeeded and
+                    // thus it's safe to remove the attachment. We use the
+                    // "Delayed failure exit" below.
+                }
 
                 // need to change the medium attachment for backward merges
@@ -2583 +2612 @@
             fMachineSettingsChanged = true;
 
-            /* prevent from calling cancelDeleteSnapshotMedium() */
+            // prevent calling cancelDeleteSnapshotMedium() for this attachment
             it = toDelete.erase(it);
+
+            // Delayed failure exit when the merge cleanup failed but the
+            // merge actually succeeded.
+            if (FAILED(rc))
+                throw rc;
         }