Changeset 24298 in vbox

Timestamp:

Nov 3, 2009 5:20:02 PM (15 years ago)

Author:

vboxsync

Message:

Main: snapshots code cleanup, renames, documentation, coding style

Location:

trunk/src/VBox/Main

Files:

• SnapshotImpl.cpp (modified) (23 diffs)
• VirtualBoxImpl.cpp (modified) (5 diffs)
• include/MachineImpl.h (modified) (1 diff)
• include/VirtualBoxImpl.h (modified) (1 diff)

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -1055 +1055 @@
 ////////////////////////////////////////////////////////////////////////////////
 
-/** Task structure for asynchronous VM operations */
-struct SessionMachine::Task
-{
-    Task (SessionMachine *m, Progress *p)
-        : machine (m), progress (p)
-        , state (m->mData->mMachineState) // save the current machine state
-        , subTask (false)
+/**
+ * Abstract base class for SessionMachine::DeleteSnapshotTask and
+ * SessionMachine::RestoreSnapshotTask. This is necessary since
+ * RTThreadCreate cannot call a method as its thread function, so
+ * instead we have it call the static SessionMachine::taskHandler,
+ * which can then call the handler() method in here (implemented
+ * by the children).
+ */
+struct SessionMachine::SnapshotTask
+{
+    SnapshotTask(SessionMachine *m,
+                 Progress *p,
+                 Snapshot *s)
+        : pMachine(m),
+          pProgress(p),
+          machineStateBackup(m->mData->mMachineState), // save the current machine state
+          pSnapshot(s)
     {}
 
-    void modifyLastState (MachineState_T s)
-    {
-        *const_cast <MachineState_T *> (&state) = s;
+    void modifyBackedUpState(MachineState_T s)
+    {
+        *const_cast<MachineState_T*>(&machineStateBackup) = s;
     }
 
     virtual void handler() = 0;
 
-    ComObjPtr<SessionMachine> machine;
-    ComObjPtr<Progress> progress;
-    const MachineState_T state;
-
-    bool subTask : 1;
+    ComObjPtr<SessionMachine>       pMachine;
+    ComObjPtr<Progress>             pProgress;
+    const MachineState_T            machineStateBackup;
+    ComObjPtr<Snapshot>             pSnapshot;
 };
 
 /** Discard snapshot task */
 struct SessionMachine::DeleteSnapshotTask
-    : public SessionMachine::Task
-{
-    DeleteSnapshotTask(SessionMachine *m, Progress *p, Snapshot *s)
-        : Task(m, p),
-          snapshot(s)
+    : public SessionMachine::SnapshotTask
+{
+    DeleteSnapshotTask(SessionMachine *m,
+                       Progress *p,
+                       Snapshot *s)
+        : SnapshotTask(m, p, s)
     {}
 
-    DeleteSnapshotTask (const Task &task, Snapshot *s)
-        : Task(task)
-        , snapshot(s)
+    void handler()
+    {
+        pMachine->deleteSnapshotHandler(*this);
+    }
+
+private:
+    DeleteSnapshotTask(const SnapshotTask &task)
+        : SnapshotTask(task)
     {}
-
-    void handler()
-    {
-        machine->deleteSnapshotHandler(*this);
-    }
-
-    ComObjPtr<Snapshot> snapshot;
 };
 
 /** Restore snapshot state task */
 struct SessionMachine::RestoreSnapshotTask
-    : public SessionMachine::Task
+    : public SessionMachine::SnapshotTask
 {
     RestoreSnapshotTask(SessionMachine *m,
-                        ComObjPtr<Snapshot> &aSnapshot,
                         Progress *p,
+                        Snapshot *s,
                         ULONG ulStateFileSizeMB)
-        : Task(m, p),
-          m_pSnapshot(aSnapshot),
+        : SnapshotTask(m, p, s),
           m_ulStateFileSizeMB(ulStateFileSizeMB)
     {}
@@ -1115 +1122 @@
     void handler()
     {
-        machine->restoreSnapshotHandler(*this);
-    }
-
-    ComObjPtr<Snapshot> m_pSnapshot;
-    ULONG m_ulStateFileSizeMB;
+        pMachine->restoreSnapshotHandler(*this);
+    }
+
+    ULONG       m_ulStateFileSizeMB;
 };
 
@@ -1474 +1480 @@
     /* create and start the task on a separate thread (note that it will not
      * start working until we release alock) */
-    RestoreSnapshotTask *task = new RestoreSnapshotTask(this, pSnapshot, progress, ulStateFileSizeMB);
+    RestoreSnapshotTask *task = new RestoreSnapshotTask(this,
+                                                        progress,
+                                                        pSnapshot,
+                                                        ulStateFileSizeMB);
     int vrc = RTThreadCreate(NULL,
                              taskHandler,
@@ -1509 +1518 @@
 
 /* static */
-DECLCALLBACK(int) SessionMachine::taskHandler (RTTHREAD /* thread */, void *pvUser)
+DECLCALLBACK(int) SessionMachine::taskHandler(RTTHREAD /* thread */, void *pvUser)
 {
     AssertReturn(pvUser, VERR_INVALID_POINTER);
 
-    Task *task = static_cast <Task *> (pvUser);
+    SnapshotTask *task = static_cast<SnapshotTask*>(pvUser);
     task->handler();
 
@@ -1609 +1618 @@
 struct MediumDiscardRec
 {
-    MediumDiscardRec() : chain (NULL) {}
-
-    MediumDiscardRec (const ComObjPtr<Medium> &aHd,
-                      Medium::MergeChain *aChain = NULL)
-        : hd (aHd), chain (aChain) {}
-
-    MediumDiscardRec (const ComObjPtr<Medium> &aHd,
-                      Medium::MergeChain *aChain,
-                      const ComObjPtr<Medium> &aReplaceHd,
-                      const ComObjPtr<MediumAttachment> &aReplaceHda,
-                      const Guid &aSnapshotId)
-        : hd (aHd), chain (aChain)
-        , replaceHd (aReplaceHd), replaceHda (aReplaceHda)
-        , snapshotId (aSnapshotId) {}
+    MediumDiscardRec()
+        : chain(NULL)
+    {}
+
+    MediumDiscardRec(const ComObjPtr<Medium> &aHd,
+                     Medium::MergeChain *aChain = NULL)
+        : hd(aHd),
+          chain(aChain)
+    {}
+
+    MediumDiscardRec(const ComObjPtr<Medium> &aHd,
+                     Medium::MergeChain *aChain,
+                     const ComObjPtr<Medium> &aReplaceHd,
+                     const ComObjPtr<MediumAttachment> &aReplaceHda,
+                     const Guid &aSnapshotId)
+        : hd(aHd),
+          chain (aChain),
+          replaceHd(aReplaceHd),
+          replaceHda(aReplaceHda),
+          snapshotId(aSnapshotId)
+    {}
 
     ComObjPtr<Medium> hd;
@@ -1655 +1671 @@
         /* we might have been uninitialized because the session was accidentally
          * closed by the client, so don't assert */
-        aTask.progress->notifyComplete(E_FAIL,
-                                       COM_IIDOF(IMachine),
-                                       getComponentName(),
-                                       tr("The session has been accidentally closed"));
+        aTask.pProgress->notifyComplete(E_FAIL,
+                                        COM_IIDOF(IMachine),
+                                        getComponentName(),
+                                        tr("The session has been accidentally closed"));
         LogFlowThisFuncLeave();
         return;
@@ -1666 +1682 @@
     AutoMultiWriteLock3 alock(this->lockHandle(),
                               this->snapshotsTreeLockHandle(),
-                              aTask.snapshot->lockHandle());
-
-    ComPtr<SnapshotMachine> sm = aTask.snapshot->getSnapshotMachine();
+                              aTask.pSnapshot->lockHandle());
+
+    ComPtr<SnapshotMachine> sm = aTask.pSnapshot->getSnapshotMachine();
     /* no need to lock the snapshot machine since it is const by definiton */
 
@@ -1674 +1690 @@
 
     /* save the snapshot ID (for callbacks) */
-    Guid snapshotId = aTask.snapshot->getId();
+    Guid snapshotId = aTask.pSnapshot->getId();
 
     MediumDiscardRecList toDiscard;
@@ -1705 +1721 @@
                 /* skip writethrough hard disks */
                 Assert(hd->type() == MediumType_Writethrough);
-                rc = aTask.progress->SetNextOperation(BstrFmt(tr("Skipping writethrough hard disk '%s'"),
-                                                              hd->base()->name().raw()),
-                                                      1); // weight
+                rc = aTask.pProgress->SetNextOperation(BstrFmt(tr("Skipping writethrough hard disk '%s'"),
+                                                               hd->base()->name().raw()),
+                                                       1); // weight
                 CheckComRCThrowRC(rc);
                 continue;
@@ -1796 +1812 @@
 
         {
-            ComObjPtr<Snapshot> parentSnapshot = aTask.snapshot->parent();
-            Utf8Str stateFilePath = aTask.snapshot->stateFilePath();
+            ComObjPtr<Snapshot> parentSnapshot = aTask.pSnapshot->parent();
+            Utf8Str stateFilePath = aTask.pSnapshot->stateFilePath();
 
             /* Note that discarding the snapshot will deassociate it from the
              * hard disks which will allow the merge+delete operation for them*/
-            aTask.snapshot->beginDiscard();
-            aTask.snapshot->uninit();
+            aTask.pSnapshot->beginDiscard();
+            aTask.pSnapshot->uninit();
 
             rc = saveAllSnapshots();
@@ -1812 +1828 @@
             if (!stateFilePath.isEmpty())
             {
-                aTask.progress->SetNextOperation(Bstr(tr("Discarding the execution state")),
-                                                 1);        // weight
+                aTask.pProgress->SetNextOperation(Bstr(tr("Discarding the execution state")),
+                                                  1);        // weight
 
                 RTFileDelete(stateFilePath.c_str());
@@ -1846 +1862 @@
              it != toDiscard.end();)
         {
-            rc = it->hd->discard (aTask.progress, it->chain);
+            rc = it->hd->discard(aTask.pProgress, it->chain);
             CheckComRCBreakRC(rc);
 
@@ -1885 +1901 @@
     }
 
-    if (!aTask.subTask || FAILED(rc))
-    {
-        if (!aTask.subTask)
-        {
-            /* saveSettings() below needs a VirtualBox write lock and we need to
-             * leave this object's lock to do this to follow the {parent-child}
-             * locking rule. This is the last chance to do that while we are
-             * still in a protective state which allows us to temporarily leave
-             * the lock */
-            alock.unlock();
-            AutoWriteLock vboxLock(mParent);
-            alock.lock();
-
-            /* preserve existing error info */
-            ErrorInfoKeeper eik;
-
-            /* restore the machine state */
-            setMachineState(aTask.state);
-            updateMachineStateOnClient();
-
-            if (settingsChanged)
-                saveSettings(SaveS_InformCallbacksAnyway);
-        }
+    if (FAILED(rc))
+    {
+        /* saveSettings() below needs a VirtualBox write lock and we need to
+         * leave this object's lock to do this to follow the {parent-child}
+         * locking rule. This is the last chance to do that while we are
+         * still in a protective state which allows us to temporarily leave
+         * the lock */
+        alock.unlock();
+        AutoWriteLock vboxLock(mParent);
+        alock.lock();
+
+        /* preserve existing error info */
+        ErrorInfoKeeper eik;
+
+        /* restore the machine state */
+        setMachineState(aTask.machineStateBackup);
+        updateMachineStateOnClient();
+
+        if (settingsChanged)
+            saveSettings(SaveS_InformCallbacksAnyway);
 
         /* set the result (this will try to fetch current error info on failure) */
-        aTask.progress->notifyComplete (rc);
+        aTask.pProgress->notifyComplete(rc);
     }
 
     if (SUCCEEDED(rc))
-        mParent->onSnapshotDiscarded (mData->mUuid, snapshotId);
-
-    LogFlowThisFunc(("Done discarding snapshot (rc=%08X)\n", rc));
+        mParent->onSnapshotDeleted(mData->mUuid, snapshotId);
+
+    LogFlowThisFunc(("Done deleting snapshot (rc=%08X)\n", rc));
     LogFlowThisFuncLeave();
 }
@@ -1937 +1950 @@
         /* we might have been uninitialized because the session was accidentally
          * closed by the client, so don't assert */
-        aTask.progress->notifyComplete(E_FAIL,
-                                       COM_IIDOF(IMachine),
-                                       getComponentName(),
-                                       tr("The session has been accidentally closed"));
+        aTask.pProgress->notifyComplete(E_FAIL,
+                                        COM_IIDOF(IMachine),
+                                        getComponentName(),
+                                        tr("The session has been accidentally closed"));
 
         LogFlowThisFuncLeave();
@@ -1970 +1983 @@
         /* discard the saved state file if the machine was Saved prior to this
          * operation */
-        if (aTask.state == MachineState_Saved)
+        if (aTask.machineStateBackup == MachineState_Saved)
         {
             Assert(!mSSData->mStateFilePath.isEmpty());
             RTFileDelete(mSSData->mStateFilePath.c_str());
             mSSData->mStateFilePath.setNull();
-            aTask.modifyLastState(MachineState_PoweredOff);
+            aTask.modifyBackedUpState(MachineState_PoweredOff);
             rc = saveStateSettings(SaveSTS_StateFilePath);
             CheckComRCThrowRC(rc);
@@ -1984 +1997 @@
 
         {
-            AutoReadLock snapshotLock(aTask.m_pSnapshot);
+            AutoReadLock snapshotLock(aTask.pSnapshot);
 
             /* remember the timestamp of the snapshot we're restoring from */
-            snapshotTimeStamp = aTask.m_pSnapshot->getTimeStamp();
-
-            ComPtr<SnapshotMachine> pSnapshotMachine(aTask.m_pSnapshot->getSnapshotMachine());
+            snapshotTimeStamp = aTask.pSnapshot->getTimeStamp();
+
+            ComPtr<SnapshotMachine> pSnapshotMachine(aTask.pSnapshot->getSnapshotMachine());
 
             /* copy all hardware data from the snapshot */
@@ -2005 +2018 @@
 
             rc = createImplicitDiffs(mUserData->mSnapshotFolderFull,
-                                     aTask.progress,
+                                     aTask.pProgress,
                                      1,
                                      false /* aOnline */);
@@ -2022 +2035 @@
             Assert(mSSData->mStateFilePath.isEmpty());
 
-            if (!aTask.m_pSnapshot->stateFilePath().isEmpty())
+            if (!aTask.pSnapshot->stateFilePath().isEmpty())
             {
-                Utf8Str snapStateFilePath = aTask.m_pSnapshot->stateFilePath();
+                Utf8Str snapStateFilePath = aTask.pSnapshot->stateFilePath();
 
                 Utf8Str stateFilePath = Utf8StrFmt("%ls%c{%RTuuid}.sav",
@@ -2034 +2047 @@
                                   snapStateFilePath.raw(), stateFilePath.raw()));
 
-                aTask.progress->SetNextOperation(Bstr(tr("Restoring the execution state")),
-                                                 aTask.m_ulStateFileSizeMB);        // weight
+                aTask.pProgress->SetNextOperation(Bstr(tr("Restoring the execution state")),
+                                                  aTask.m_ulStateFileSizeMB);        // weight
 
                 /* leave the lock before the potentially lengthy operation */
@@ -2046 +2059 @@
                                        0,
                                        progressCallback,
-                                       aTask.progress);
+                                       aTask.pProgress);
 
                 alock.enter();
@@ -2061 +2074 @@
             }
 
-            LogFlowThisFunc(("Setting new current snapshot {%RTuuid}\n", aTask.m_pSnapshot->getId().raw()));
+            LogFlowThisFunc(("Setting new current snapshot {%RTuuid}\n", aTask.pSnapshot->getId().raw()));
             /* make the snapshot we restored from the current snapshot */
-            mData->mCurrentSnapshot = aTask.m_pSnapshot;
+            mData->mCurrentSnapshot = aTask.pSnapshot;
         }
 
@@ -2172 +2185 @@
         {
             /* restore the machine state */
-            setMachineState(aTask.state);
+            setMachineState(aTask.machineStateBackup);
             updateMachineStateOnClient();
         }
@@ -2178 +2191 @@
 
     /* set the result (this will try to fetch current error info on failure) */
-    aTask.progress->notifyComplete(rc);
+    aTask.pProgress->notifyComplete(rc);
 
     if (SUCCEEDED(rc))
-        mParent->onSnapshotDiscarded(mData->mUuid, Guid());
+        mParent->onSnapshotDeleted(mData->mUuid, Guid());
 
     LogFlowThisFunc(("Done restoring snapshot (rc=%08X)\n", rc));

trunk/src/VBox/Main/VirtualBoxImpl.cpp

@@ -2491 +2491 @@
 void VirtualBox::onSnapshotTaken (const Guid &aMachineId, const Guid &aSnapshotId)
 {
-    postEvent (new SnapshotEvent (this, aMachineId, aSnapshotId, SnapshotEvent::Taken));
+    postEvent(new SnapshotEvent(this, aMachineId, aSnapshotId, SnapshotEvent::Taken));
 }
 
@@ -2497 +2497 @@
  *  @note Doesn't lock any object.
  */
-void VirtualBox::onSnapshotDiscarded (const Guid &aMachineId, const Guid &aSnapshotId)
-{
-    postEvent (new SnapshotEvent (this, aMachineId, aSnapshotId, SnapshotEvent::Discarded));
+void VirtualBox::onSnapshotDeleted(const Guid &aMachineId, const Guid &aSnapshotId)
+{
+    postEvent(new SnapshotEvent(this, aMachineId, aSnapshotId, SnapshotEvent::Discarded));
 }
 
@@ -2505 +2505 @@
  *  @note Doesn't lock any object.
  */
-void VirtualBox::onSnapshotChange (const Guid &aMachineId, const Guid &aSnapshotId)
-{
-    postEvent (new SnapshotEvent (this, aMachineId, aSnapshotId, SnapshotEvent::Changed));
+void VirtualBox::onSnapshotChange(const Guid &aMachineId, const Guid &aSnapshotId)
+{
+    postEvent(new SnapshotEvent(this, aMachineId, aSnapshotId, SnapshotEvent::Changed));
 }
 
@@ -2513 +2513 @@
 struct GuestPropertyEvent : public VirtualBox::CallbackEvent
 {
-    GuestPropertyEvent (VirtualBox *aVBox, const Guid &aMachineId,
-                        IN_BSTR aName, IN_BSTR aValue, IN_BSTR aFlags)
-        : CallbackEvent (aVBox), machineId (aMachineId)
-        , name (aName), value (aValue), flags(aFlags)
-        {}
-
-    void handleCallback (const ComPtr<IVirtualBoxCallback> &aCallback)
-    {
-        LogFlow (("OnGuestPropertyChange: machineId={%RTuuid}, name='%ls', value='%ls', flags='%ls'\n",
-                  machineId.ptr(), name.raw(), value.raw(), flags.raw()));
+    GuestPropertyEvent(VirtualBox *aVBox, const Guid &aMachineId,
+                       IN_BSTR aName, IN_BSTR aValue, IN_BSTR aFlags)
+        : CallbackEvent(aVBox),
+          machineId(aMachineId),
+          name(aName),
+          value(aValue),
+          flags(aFlags)
+    {}
+
+    void handleCallback(const ComPtr<IVirtualBoxCallback> &aCallback)
+    {
+        LogFlow(("OnGuestPropertyChange: machineId={%RTuuid}, name='%ls', value='%ls', flags='%ls'\n",
+                 machineId.ptr(), name.raw(), value.raw(), flags.raw()));
         aCallback->OnGuestPropertyChange (machineId.toUtf16(), name, value, flags);
     }
@@ -2533 +2536 @@
  *  @note Doesn't lock any object.
  */
-void VirtualBox::onGuestPropertyChange (const Guid &aMachineId, IN_BSTR aName,
-                                        IN_BSTR aValue, IN_BSTR aFlags)
-{
-    postEvent (new GuestPropertyEvent (this, aMachineId, aName, aValue, aFlags));
+void VirtualBox::onGuestPropertyChange(const Guid &aMachineId, IN_BSTR aName,
+                                       IN_BSTR aValue, IN_BSTR aFlags)
+{
+    postEvent(new GuestPropertyEvent(this, aMachineId, aName, aValue, aFlags));
 }
 

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

@@ -1034 +1034 @@
     };
 
-    struct Task;
+    struct SnapshotTask;
     struct DeleteSnapshotTask;
     struct RestoreSnapshotTask;

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

@@ -196 +196 @@
     void updateClientWatcher();
 
-    void onMachineStateChange (const Guid &aId, MachineState_T aState);
-    void onMachineDataChange (const Guid &aId);
+    void onMachineStateChange(const Guid &aId, MachineState_T aState);
+    void onMachineDataChange(const Guid &aId);
     BOOL onExtraDataCanChange(const Guid &aId, IN_BSTR aKey, IN_BSTR aValue,
                               Bstr &aError);
     void onExtraDataChange(const Guid &aId, IN_BSTR aKey, IN_BSTR aValue);
-    void onMachineRegistered (const Guid &aId, BOOL aRegistered);
-    void onSessionStateChange (const Guid &aId, SessionState_T aState);
-
-    void onSnapshotTaken (const Guid &aMachineId, const Guid &aSnapshotId);
-    void onSnapshotDiscarded (const Guid &aMachineId, const Guid &aSnapshotId);
-    void onSnapshotChange (const Guid &aMachineId, const Guid &aSnapshotId);
-    void onGuestPropertyChange (const Guid &aMachineId, IN_BSTR aName, IN_BSTR aValue,
-                                IN_BSTR aFlags);
+    void onMachineRegistered(const Guid &aId, BOOL aRegistered);
+    void onSessionStateChange(const Guid &aId, SessionState_T aState);
+
+    void onSnapshotTaken(const Guid &aMachineId, const Guid &aSnapshotId);
+    void onSnapshotDeleted(const Guid &aMachineId, const Guid &aSnapshotId);
+    void onSnapshotChange(const Guid &aMachineId, const Guid &aSnapshotId);
+    void onGuestPropertyChange(const Guid &aMachineId, IN_BSTR aName, IN_BSTR aValue,
+                               IN_BSTR aFlags);
 
     ComObjPtr<GuestOSType> getUnknownOSType();