Changeset 41231 in vbox for trunk

Timestamp:

May 10, 2012 11:27:52 AM (13 years ago)

Author:

vboxsync

Message:

Main/VirtualBox: very tricky fix for renaming linked clones (media registry needs saving) which avoids locking problems by using a separate thread

Location:

trunk/src/VBox/Main

Files:

• include/MediumImpl.h (modified) (2 diffs)
• src-server/MediumImpl.cpp (modified) (1 diff)
• src-server/VirtualBoxImpl.cpp (modified) (4 diffs)

trunk/src/VBox/Main/include/MediumImpl.h

@@ -182 +182 @@
     bool isInRegistry(const Guid& id);
     bool getFirstRegistryMachineId(Guid &uuid) const;
+    void markRegistriesModified();
 
     HRESULT addBackReference(const Guid &aMachineId,
@@ -268 +269 @@
     HRESULT canClose();
     HRESULT unregisterWithVirtualBox();
-    void markRegistriesModified();
 
     HRESULT setStateError();

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

@@ -3600 +3600 @@
 
         LogFlowThisFunc(("locationFull.after='%s'\n", m->strLocationFull.c_str()));
-    }
-
-    return S_OK;
+        // we changed something
+        return S_OK;
+    }
+
+    // no change was necessary, signal error which the caller needs to interpret
+    return VBOX_E_FILE_ERROR;
 }
 

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

@@ -3264 +3264 @@
 }
 
+struct SaveMediaRegistriesDesc
+{
+    MediaList llMedia;
+    ComObjPtr<VirtualBox> pVirtualBox;
+};
+
+static int fntSaveMediaRegistries(RTTHREAD ThreadSelf, void *pvUser)
+{
+    NOREF(ThreadSelf);
+    SaveMediaRegistriesDesc *pDesc = (SaveMediaRegistriesDesc *)pvUser;
+    if (!pDesc)
+    {
+        LogRelFunc(("Thread for saving media registries lacks parameters\n"));
+        return VERR_INVALID_PARAMETER;
+    }
+
+    for (MediaList::const_iterator it = pDesc->llMedia.begin();
+         it != pDesc->llMedia.end();
+         ++it)
+    {
+        Medium *pMedium = *it;
+        pMedium->markRegistriesModified();
+    }
+
+    pDesc->pVirtualBox->saveModifiedRegistries();
+
+    pDesc->llMedia.clear();
+    pDesc->pVirtualBox.setNull();
+    delete pDesc;
+
+    return VINF_SUCCESS;
+}
+
 /**
  * Goes through all known media (hard disks, floppies and DVDs) and saves
@@ -3291 +3324 @@
  * @param mediaRegistry Settings structure to fill.
  * @param uuidRegistry The UUID of the media registry; either a machine UUID (if machine registry) or the UUID of the global registry.
- * @param hardDiskFolder The machine folder for relative paths, if machine registry, or an empty string otherwise.
+ * @param strMachineFolder The machine folder for relative paths, if machine registry, or an empty string otherwise.
  */
 void VirtualBox::saveMediaRegistry(settings::MediaRegistry &mediaRegistry,
@@ -3314 +3347 @@
             llAllMedia.push_back(*it);
 
+        SaveMediaRegistriesDesc *pDesc = new SaveMediaRegistriesDesc();
         for (MediaList::iterator it = llAllMedia.begin();
              it != llAllMedia.end();
@@ -3324 +3358 @@
             {
                 const Data::PendingMachineRename &pmr = *it2;
-                pMedium->updatePath(pmr.strConfigDirOld,
-                                    pmr.strConfigDirNew);
+                HRESULT rc = pMedium->updatePath(pmr.strConfigDirOld,
+                                                 pmr.strConfigDirNew);
+                if (SUCCEEDED(rc))
+                {
+                    // Remember which medium objects has been changed,
+                    // to trigger saving their registries later.
+                    pDesc->llMedia.push_back(pMedium);
+                } else if (rc == VBOX_E_FILE_ERROR)
+                    /* nothing */;
+                else
+                    AssertComRC(rc);
             }
         }
         // done, don't do it again until we have more machine renames
         m->llPendingMachineRenames.clear();
+
+        if (pDesc->llMedia.size())
+        {
+            // Handle the media registry saving in a separate thread, to
+            // avoid giant locking problems and passing up the list many
+            // levels up to whoever triggered saveSettings, as there are
+            // lots of places which would need to handle saving more settings.
+            pDesc->pVirtualBox = this;
+            int vrc = RTThreadCreate(NULL,
+                                     fntSaveMediaRegistries,
+                                     (void *)pDesc,
+                                     0,     // cbStack (default)
+                                     RTTHREADTYPE_MAIN_WORKER,
+                                     0,     // flags
+                                     "SaveMediaReg");
+            ComAssertRC(vrc);
+            // failure means that settings aren't saved, but there isn't
+            // much we can do besides avoiding memory leaks
+            if (RT_FAILURE(vrc))
+            {
+                LogRelFunc(("Failed to create thread for saving media registries (%Rrc)\n", vrc));
+                delete pDesc;
+            }
+        }
+        else
+            delete pDesc;
     }