Changeset 56563 in vbox

Timestamp:

Jun 19, 2015 2:39:16 PM (10 years ago)

Author:

vboxsync

Message:

Main/Main/src-server/MachineImplCloneVM: fix a collection of bugs around losing saved state during cloning (file was copied, but not used in "current state" as it should be) and incorrect value of the "current state modified" flag of the clone.

File:

• trunk/src/VBox/Main/src-server/MachineImplCloneVM.cpp (modified) (12 diffs)

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

@@ -103 +103 @@
     HRESULT createMachineList(const ComPtr<ISnapshot> &pSnapshot, RTCList< ComObjPtr<Machine> > &machineList) const;
     inline void updateProgressStats(MEDIUMTASKCHAIN &mtc, bool fAttachLinked, ULONG &uCount, ULONG &uTotalWeight) const;
-    inline HRESULT addSaveState(const ComObjPtr<Machine> &machine, ULONG &uCount, ULONG &uTotalWeight);
+    inline HRESULT addSaveState(const ComObjPtr<Machine> &machine, bool fAttachCurrent, ULONG &uCount, ULONG &uTotalWeight);
     inline HRESULT queryBaseName(const ComPtr<IMedium> &pMedium, Utf8Str &strBaseName) const;
     HRESULT queryMediasForMachineState(const RTCList<ComObjPtr<Machine> > &machineList,
@@ -200 +200 @@
 }
 
-HRESULT MachineCloneVMPrivate::addSaveState(const ComObjPtr<Machine> &machine, ULONG &uCount, ULONG &uTotalWeight)
+HRESULT MachineCloneVMPrivate::addSaveState(const ComObjPtr<Machine> &machine, bool fAttachCurrent, ULONG &uCount, ULONG &uTotalWeight)
 {
     Bstr bstrSrcSaveStatePath;
@@ -208 +208 @@
     {
         SAVESTATETASK sst;
-        sst.snapshotUuid     = machine->i_getSnapshotId();
+        if (fAttachCurrent)
+        {
+            /* Make this saved state part of "current state" of the target
+             * machine, whether it is part of a snapshot or not. */
+            sst.snapshotUuid.clear();
+        }
+        else
+            sst.snapshotUuid = machine->i_getSnapshotId();
         sst.strSaveStateFile = bstrSrcSaveStatePath;
         uint64_t cbSize;
@@ -241 +248 @@
 {
     /* This mode is pretty straightforward. We didn't need to know about any
-     * parent/children relationship and therefor simply adding all directly
+     * parent/children relationship and therefore simply adding all directly
      * attached images of the source VM as cloning targets. The IMedium code
      * take than care to merge any (possibly) existing parents into the new
@@ -309 +316 @@
         }
         /* Add the save state files of this machine if there is one. */
-        rc = addSaveState(machine, uCount, uTotalWeight);
+        rc = addSaveState(machine, true /*fAttachCurrent*/, uCount, uTotalWeight);
         if (FAILED(rc)) return rc;
     }
@@ -346 +353 @@
      *
      * Note: This still leads to media chains which can have the same medium
-     * included. This case is handled in "run" and therefor not critical, but
+     * included. This case is handled in "run" and therefore not critical, but
      * it leads to wrong progress infos which isn't nice. */
 
+    Assert(!fAttachLinked);
     HRESULT rc = S_OK;
     std::map<ComPtr<IMedium>, uint32_t> mediaHist; /* Our usage histogram for the medias */
@@ -412 +420 @@
         }
         /* Add the save state files of this machine if there is one. */
-        rc = addSaveState(machine, uCount, uTotalWeight);
+        rc = addSaveState(machine, false /*fAttachCurrent*/, uCount, uTotalWeight);
         if (FAILED(rc)) return rc;
+        /* If this is the newly created current state, make sure that the
+         * saved state is also attached to it. */
+        if (fCreateDiffs)
+        {
+            rc = addSaveState(machine, true /*fAttachCurrent*/, uCount, uTotalWeight);
+            if (FAILED(rc)) return rc;
+        }
     }
     /* Build up the index list of the image chain. Unfortunately we can't do
@@ -485 +500 @@
      * adding all directly and indirectly attached disk images to the worker
      * list. */
+    Assert(!fAttachLinked);
     HRESULT rc = S_OK;
     for (size_t i = 0; i < machineList.size(); ++i)
@@ -549 +565 @@
         }
         /* Add the save state files of this machine if there is one. */
-        rc = addSaveState(machine, uCount, uTotalWeight);
+        rc = addSaveState(machine, false /*fAttachCurrent*/, uCount, uTotalWeight);
         if (FAILED(rc)) return rc;
+        /* If this is the newly created current state, make sure that the
+         * saved state is also attached to it. */
+        if (fCreateDiffs)
+        {
+            rc = addSaveState(machine, true /*fAttachCurrent*/, uCount, uTotalWeight);
+            if (FAILED(rc)) return rc;
+        }
     }
     /* Build up the index list of the image chain. Unfortunately we can't do
@@ -982 +1005 @@
                                   p->tr("Could not find data to snapshots '%s'"), d->snapshotId.toString().c_str());
 
-
-
         if (d->mode == CloneMode_MachineState)
         {
@@ -1008 +1029 @@
                 /* Current state is under root snapshot. */
                 trgMCF.uuidCurrentSnapshot = sn.uuid;
-                /* There will be created a new differencing image based on this
-                 * snapshot. So reset the modified state. */
-                trgMCF.fCurrentStateModified = false;
             }
             /* The snapshot will be the root one. */
@@ -1396 +1414 @@
             rc = d->pTrgMachine->i_loadMachineDataFromSettings(trgMCF, &d->pTrgMachine->mData->mUuid);
             if (FAILED(rc)) throw rc;
+
+            /* Fix up the "current state modified" flag to what it should be,
+             * as the value guessed in i_loadMachineDataFromSettings can be
+             * quite far off the logical value for the cloned VM. */
+            if (d->mode == CloneMode_MachineState)
+                d->pTrgMachine->mData->mCurrentStateModified = FALSE;
+            else if (   d->mode == CloneMode_MachineAndChildStates
+                && sn.uuid.isValid()
+                && !sn.uuid.isZero())
+            {
+                if (!d->pOldMachineState.isNull())
+                {
+                    /* There will be created a new differencing image based on
+                     * this snapshot. So reset the modified state. */
+                    d->pTrgMachine->mData->mCurrentStateModified = FALSE;
+                }
+                else
+                    d->pTrgMachine->mData->mCurrentStateModified = p->mData->mCurrentStateModified;
+            }
+            else if (d->mode == CloneMode_AllStates)
+                d->pTrgMachine->mData->mCurrentStateModified = p->mData->mCurrentStateModified;
+
+            /* If the target machine has saved state we MUST adjust the machine
+             * state, otherwise saving settings will drop the information. */
+            if (trgMCF.strStateFile.isNotEmpty())
+                d->pTrgMachine->i_setMachineState(MachineState_Saved);
+
             /* save all VM data */
             bool fNeedsGlobalSaveSettings = false;