Changeset 33921 in vbox for trunk/src/VBox/Main/MachineImpl.cpp

Timestamp:

Nov 9, 2010 5:51:38 PM (14 years ago)

Author:

vboxsync

Message:

Main: rework 'save registries' logic to ensure that all media registries get saved, not just the global VirtualBox.xml file

File:

• trunk/src/VBox/Main/MachineImpl.cpp (modified) (26 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3323 +3323 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    // if this becomes true then we need to call saveSettings in the end
-    // @todo r=dj there is no error handling so far...
-    bool fNeedsMachineSaveSettings = false,
-         fNeedsGlobalSaveSettings = false;
-
     // request the host lock first, since might be calling Host methods for getting host drives;
     // next, protect the media tree all the while we're in here, as well as our member variables
@@ -3336 +3331 @@
     HRESULT rc = checkStateDependency(MutableStateDep);
     if (FAILED(rc)) return rc;
+
+    GuidList llRegistriesThatNeedSaving;
 
     /// @todo NEWMEDIA implicit machine registration
@@ -3596 +3593 @@
                         strFullSnapshotFolder.append(RTPATH_SLASH_STR),
                         medium->getFirstRegistryMachineId(),         // store this diff in the same registry as the parent
-                        &fNeedsGlobalSaveSettings);
+                        &llRegistriesThatNeedSaving);
         if (FAILED(rc)) return rc;
 
@@ -3628 +3625 @@
                                                NULL /* aProgress */,
                                                true /* aWait */,
-                                               &fNeedsGlobalSaveSettings);
+                                               &llRegistriesThatNeedSaving);
 
                 alock.enter();
@@ -3671 +3668 @@
 
         // and decide which medium registry to use now that the medium is attached:
+        Guid uuid;
         if (mData->pMachineConfigFile->canHaveOwnMediaRegistry())
             // machine XML is VirtualBox 4.0 or higher:
-            medium->addRegistry(getId(),         // machine UUID
-                                &fNeedsMachineSaveSettings);
+            uuid = getId();     // machine UUID
         else
-            medium->addRegistry(mParent->getGlobalRegistryId(), // VirtualBox global registry UUID
-                                &fNeedsGlobalSaveSettings);
+            uuid = mParent->getGlobalRegistryId(); // VirtualBox global registry UUID
+
+        if (medium->addRegistry(uuid))
+            // registry actually changed:
+            mParent->addGuidToListUniquely(llRegistriesThatNeedSaving, uuid);
     }
 
@@ -3687 +3687 @@
     mediumLock.release();
     treeLock.leave();
-
-    if (fNeedsMachineSaveSettings)
-        saveSettings(&fNeedsGlobalSaveSettings, SaveS_Force);
-
-    if (fNeedsGlobalSaveSettings)
-    {
-        // save the global settings; for that we should hold only the VirtualBox lock
-        alock.release();
-
-        AutoWriteLock vboxLock(mParent COMMA_LOCKVAL_SRC_POS);
-        mParent->saveSettings();
-    }
+    alock.release();
+
+    mParent->saveRegistries(llRegistriesThatNeedSaving);
 
     return rc;
@@ -3714 +3705 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    bool fNeedsSaveSettings = false;
+    GuidList llRegistriesThatNeedSaving;
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
@@ -3737 +3728 @@
                         aDevice, aControllerPort, aControllerName);
 
-    rc = detachDevice(pAttach, alock, NULL /* pSnapshot */, &fNeedsSaveSettings);
-
-    if (fNeedsSaveSettings)
-    {
-        bool fNeedsGlobalSaveSettings = false;
-        saveSettings(&fNeedsGlobalSaveSettings);
-
-        if (fNeedsGlobalSaveSettings)
-        {
-            // save the global settings; for that we should hold only the VirtualBox lock
-            alock.release();
-            AutoWriteLock vboxlock(this COMMA_LOCKVAL_SRC_POS);
-            mParent->saveSettings();
-        }
-    }
-
-    return S_OK;
+    rc = detachDevice(pAttach, alock, NULL /* pSnapshot */, &llRegistriesThatNeedSaving);
+
+    alock.release();
+
+    if (SUCCEEDED(rc))
+        rc = mParent->saveRegistries(llRegistriesThatNeedSaving);
+
+    return rc;
 }
 
@@ -3867 +3849 @@
     mMediaData.backup();
 
-    bool fNeedsMachineSaveSettings = false,
-         fNeedsGlobalSaveSettings = false;
+    GuidList llRegistriesThatNeedSaving;
 
     {
@@ -3885 +3866 @@
 
             // and decide which medium registry to use now that the medium is attached:
+            Guid uuid;
             if (mData->pMachineConfigFile->canHaveOwnMediaRegistry())
-                // machine XML is VirtualBox 4.0 or higher: machine UUID
-                pMedium->addRegistry(getId(),
-                                     &fNeedsMachineSaveSettings);
+                // machine XML is VirtualBox 4.0 or higher:
+                uuid = getId();     // machine UUID
             else
-                // VirtualBox global registry UUID
-                pMedium->addRegistry(mParent->getGlobalRegistryId(),
-                                     &fNeedsGlobalSaveSettings);
+                uuid = mParent->getGlobalRegistryId(); // VirtualBox global registry UUID
+
+            if (pMedium->addRegistry(uuid))
+                // registry actually changed:
+                mParent->addGuidToListUniquely(llRegistriesThatNeedSaving, uuid);
         }
 
@@ -3927 +3910 @@
     multiLock.release();
 
-    if (fNeedsMachineSaveSettings)
-    {
-        AutoWriteLock machineLock(this COMMA_LOCKVAL_SRC_POS);
-        saveSettings(&fNeedsGlobalSaveSettings, SaveS_Force);
-    }
-
-    if (fNeedsGlobalSaveSettings)
-    {
-        // save the global settings; for that we should hold only the VirtualBox lock
-        AutoWriteLock vboxLock(mParent COMMA_LOCKVAL_SRC_POS);
-        mParent->saveSettings();
-    }
+    mParent->saveRegistries(llRegistriesThatNeedSaving);
 
     return rc;
@@ -4347 +4319 @@
     std::list<Utf8Str>          llFilesToDelete;
     ComObjPtr<Progress>         pProgress;
-    bool                        fNeedsGlobalSaveSettings;
+    GuidList                    llRegistriesThatNeedSaving;
 };
 
@@ -4373 +4345 @@
     pTask->llFilesToDelete = mData->llFilesToDelete;            // saved states pushed here by Unregister()
 
-    pTask->fNeedsGlobalSaveSettings = false;
     for (size_t i = 0; i < sfaMedia.size(); ++i)
     {
@@ -4384 +4355 @@
         // close the medium now; if that succeeds, then that means the medium is no longer
         // in use and we can add it to the list of files to delete
-        rc = pMedium->close(&pTask->fNeedsGlobalSaveSettings,
+        rc = pMedium->close(&pTask->llRegistriesThatNeedSaving,
                             mediumAutoCaller);
         if (SUCCEEDED(rc))
@@ -4549 +4520 @@
     alock.release();
 
-    if (task.fNeedsGlobalSaveSettings)
-    {
-        AutoWriteLock vboxlock(mParent COMMA_LOCKVAL_SRC_POS);
-        mParent->saveSettings();
-    }
+    mParent->saveRegistries(task.llRegistriesThatNeedSaving);
 
     return S_OK;
@@ -7465 +7432 @@
             if (puuidRegistry)
                 // caller wants registry ID to be set on all attached media (OVF import case)
-                medium->addRegistry(*puuidRegistry,
-                                    NULL /* pfNeedsSaveSettings */);
+                medium->addRegistry(*puuidRegistry);
         }
 
@@ -8458 +8424 @@
  *                          attached).
  * @param aOnline           Whether the VM was online prior to this operation.
- * @param pfNeedsSaveSettings Optional pointer to a bool that must have been initialized to false and that will be set to true
- *                by this function if the caller should invoke VirtualBox::saveSettings() because the global settings have changed.
+ * @param pllRegistriesThatNeedSaving Optional pointer to a list of UUIDs to receive the registry IDs that need saving
  *
  * @note The progress object is not marked as completed, neither on success nor
@@ -8469 +8434 @@
                                      ULONG aWeight,
                                      bool aOnline,
-                                     bool *pfNeedsSaveSettings)
+                                     GuidList *pllRegistriesThatNeedSaving)
 {
     LogFlowThisFunc(("aOnline=%d\n", aOnline));
@@ -8594 +8559 @@
                             strFullSnapshotFolder.append(RTPATH_SLASH_STR),
                             pMedium->getFirstRegistryMachineId(),        // store the diff in the same registry as the parent
-                            pfNeedsSaveSettings);
+                            pllRegistriesThatNeedSaving);
             if (FAILED(rc)) throw rc;
 
@@ -8618 +8583 @@
                                             NULL /* aProgress */,
                                             true /* aWait */,
-                                            pfNeedsSaveSettings);
+                                            pllRegistriesThatNeedSaving);
             alock.enter();
             if (FAILED(rc)) throw rc;
@@ -8665 +8630 @@
         MultiResult mrc = rc;
 
-        mrc = deleteImplicitDiffs(pfNeedsSaveSettings);
+        mrc = deleteImplicitDiffs(pllRegistriesThatNeedSaving);
     }
 
@@ -8683 +8648 @@
  * @note Locks this object for writing.
  */
-HRESULT Machine::deleteImplicitDiffs(bool *pfNeedsSaveSettings)
+HRESULT Machine::deleteImplicitDiffs(GuidList *pllRegistriesThatNeedSaving)
 {
     AutoCaller autoCaller(this);
@@ -8761 +8726 @@
 
             rc = hd->deleteStorage(NULL /*aProgress*/, true /*aWait*/,
-                                   pfNeedsSaveSettings);
+                                   pllRegistriesThatNeedSaving);
             AssertMsg(SUCCEEDED(rc), ("rc=%Rhrc it=%s hd=%s\n", rc, (*it)->getLogName(), hd->getLocationFull().c_str() ));
             mrc = rc;
@@ -8769 +8734 @@
 
         if (mData->mMachineState == MachineState_SettingUp)
-        {
             setMachineState(oldState);
-        }
     }
 
@@ -8873 +8836 @@
                               AutoWriteLock &writeLock,
                               Snapshot *pSnapshot,
-                              bool *pfNeedsSaveSettings)
+                              GuidList *pllRegistriesThatNeedSaving)
 {
     ComObjPtr<Medium> oldmedium = pAttach->getMedium();
@@ -8898 +8861 @@
 
         HRESULT rc = oldmedium->deleteStorage(NULL /*aProgress*/, true /*aWait*/,
-                                              pfNeedsSaveSettings);
+                                              pllRegistriesThatNeedSaving);
 
         writeLock.acquire();