Changeset 35602 in vbox for trunk

Timestamp:

Jan 18, 2011 11:27:20 AM (14 years ago)

Author:

vboxsync

Message:

Main: fix and clarify that ISnapshot::SetName and SetDescription automatically save the machine settings; API docs

Location:

trunk/src/VBox/Main

Files:

• idl/VirtualBox.xidl (modified) (6 diffs)
• src-server/SnapshotImpl.cpp (modified) (5 diffs)

trunk/src/VBox/Main/idl/VirtualBox.xidl

@@ -5381 +5381 @@
         Returns a snapshot of this machine with the given UUID.
         A @c null argument can be used to obtain the first snapshot
-        taken on this machine. This is useful if you want to traverse
-        the whole tree of snapshots starting from the root.
+        taken on this machine. To traverse the whole tree of snapshots
+        starting from the root, inspect the root snapshot's
+        <link to="ISnapshot::children" /> attribute and recurse over those children.
 
         <result name="VBOX_E_OBJECT_NOT_FOUND">
@@ -8667 +8668 @@
               only the differencing images need to be deleted.
 
-              The current machine state is not changed by taking a snapshot.
-              If the machine is running, it will resume execution after the
-              snapshot has been taken. After calling this,
-              <link to="IMachine::currentSnapshot" /> is set to the snapshot
-              just created.
+              The current machine state is not changed by taking a snapshot
+              except that <link to="IMachine::currentSnapshot" /> is set to
+              the newly created snapshot, which is also added to the machine's
+              snapshots tree.
           </li>
 
@@ -8680 +8680 @@
 
               This destroys the machine's current state. After calling this,
-              <link to="IMachine::currentSnapshot" /> is set to the snapshot that was
-              restored.
+              <link to="IMachine::currentSnapshot" /> points to the snapshot
+              that was restored.
           </li>
 
@@ -8702 +8702 @@
       </ul>
 
-      Each snapshot contains the settings of the virtual machine (hardware
-      configuration etc.). In addition, if the machine was running when the
+      Each snapshot contains a copy of virtual machine's settings (hardware
+      configuration etc.). This copy is contained in an immutable (read-only)
+      instance of <link to="IMachine" /> which is available from the snapshot's
+      <link to="#machine" /> attribute. When restoring the snapshot, these
+      settings are copied back to the original machine.
+
+      In addition, if the machine was running when the
       snapshot was taken (<link to="IMachine::state"/> is <link to="MachineState_Running"/>),
       the current VM state is saved in the snapshot (similarly to what happens
-      when a VM's state is saved). The snapshot is then said to
-      be <i>online</i> because when restoring it, the VM will be running.
-
-      If the machine is saved (<link to="MachineState_Saved"/>), the snapshot
-      receives a copy of the execution state file (<link to="IMachine::stateFilePath"/>).
+      when a VM's state is saved). The snapshot is then said to be <i>online</i>
+      because when restoring it, the VM will be running.
+
+      If the machine was in <link to="MachineState_Saved">saved</link> saved,
+      the snapshot receives a copy of the execution state file
+      (<link to="IMachine::stateFilePath"/>).
 
       Otherwise, if the machine was not running (<link to="MachineState_PoweredOff"/>
@@ -8723 +8729 @@
 
     <attribute name="name" type="wstring">
-      <desc>Short name of the snapshot.</desc>
+      <desc>Short name of the snapshot.
+      <note>Setting this attribute causes <link to="IMachine::saveSettings" /> to
+      be called implicitly.</note>
+      </desc>
     </attribute>
 
     <attribute name="description" type="wstring">
-      <desc>Optional description of the snapshot.</desc>
+      <desc>Optional description of the snapshot.
+      <note>Setting this attribute causes <link to="IMachine::saveSettings" /> to
+      be called implicitly.</note>
+      </desc>
     </attribute>
 
@@ -8769 +8781 @@
       <desc>
         Child snapshots (all snapshots having this one as a parent).
+        By inspecting this attribute starting with a machine's root snapshot
+        (which can be obtained by calling <link to="IMachine::findSnapshot" />
+        with a @c null UUID), a machine's snapshots tree can be iterated over.
       </desc>
     </attribute>

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

@@ -346 +346 @@
 STDMETHODIMP Snapshot::COMSETTER(Name)(IN_BSTR aName)
 {
+    HRESULT rc = S_OK;
     CheckComArgStrNotEmptyOrNull(aName);
 
@@ -358 +359 @@
     {
         m->strName = strName;
-
         alock.leave(); /* Important! (child->parent locks are forbidden) */
-
-        // flag the machine as dirty or change won't get saved
-        AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS);
-        m->pMachine->setModified(Machine::IsModified_Snapshots);
-        mlock.leave();
-
-        return m->pMachine->onSnapshotChange(this);
-    }
-
-    return S_OK;
+        rc = m->pMachine->onSnapshotChange(this);
+    }
+
+    return rc;
 }
 
@@ -387 +381 @@
 STDMETHODIMP Snapshot::COMSETTER(Description)(IN_BSTR aDescription)
 {
+    HRESULT rc = S_OK;
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
@@ -397 +392 @@
     {
         m->strDescription = strDescription;
-
         alock.leave(); /* Important! (child->parent locks are forbidden) */
-
-        // flag the machine as dirty or change won't get saved
-        AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS);
-        m->pMachine->setModified(Machine::IsModified_Snapshots);
-        mlock.leave();
-
-        return m->pMachine->onSnapshotChange(this);
-    }
-
-    return S_OK;
+        rc = m->pMachine->onSnapshotChange(this);
+    }
+
+    return rc;
 }
 
@@ -1193 +1181 @@
  *  snapshot data such as name or description is changed.
  *
- *  @note Locks this object for writing.
+ *  @warning Caller must hold no locks when calling this.
  */
 HRESULT SnapshotMachine::onSnapshotChange(Snapshot *aSnapshot)
 {
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    //     mPeer->saveAllSnapshots();  @todo
+    AutoMultiWriteLock2 mlock(this, aSnapshot COMMA_LOCKVAL_SRC_POS);
+    Guid uuidMachine(mData->mUuid),
+         uuidSnapshot(aSnapshot->getId());
+    bool fNeedsGlobalSaveSettings = false;
+
+    // flag the machine as dirty or change won't get saved
+    mPeer->setModified(Machine::IsModified_Snapshots);
+    HRESULT rc = mPeer->saveSettings(&fNeedsGlobalSaveSettings,
+                                     SaveS_Force);        // we know we need saving, no need to check
+    mlock.leave();
+
+    if (SUCCEEDED(rc) && fNeedsGlobalSaveSettings)
+    {
+        // save the global settings
+        AutoWriteLock vboxlock(mParent COMMA_LOCKVAL_SRC_POS);
+        rc = mParent->saveSettings();
+    }
 
     /* inform callbacks */
-    mParent->onSnapshotChange(mData->mUuid, aSnapshot->getId());
-
-    return S_OK;
+    mParent->onSnapshotChange(uuidMachine, uuidSnapshot);
+
+    return rc;
 }