Changeset 18388 in vbox for trunk/src

Timestamp:

Mar 27, 2009 1:11:42 PM (16 years ago)

Author:

vboxsync

Message:

API/HardDisk: merge the functionality of cloneTo and flattenTo, and extend the existing flattenTo code to do the job.

Location:

trunk/src/VBox

Files:

• Frontends/VBoxManage/VBoxManageDisk.cpp (modified) (1 diff)
• Main/ApplianceImpl.cpp (modified) (2 diffs)
• Main/HardDiskImpl.cpp (modified) (14 diffs)
• Main/idl/VirtualBox.xidl (modified) (5 diffs)
• Main/include/HardDiskImpl.h (modified) (1 diff)

trunk/src/VBox/Frontends/VBoxManage/VBoxManageDisk.cpp

@@ -543 +543 @@
 
         ComPtr<IProgress> progress;
-        CHECK_ERROR_BREAK(srcDisk, CloneTo(dstDisk, DiskVariant, progress.asOutParam()));
+        CHECK_ERROR_BREAK(srcDisk, CloneTo(dstDisk, DiskVariant, NULL, progress.asOutParam()));
 
         showProgress(progress);

trunk/src/VBox/Main/ApplianceImpl.cpp

@@ -2181 +2181 @@
                             if (FAILED(rc)) throw rc;
                             /* Clone the source disk image */
-                            rc = srcHdVBox->CloneTo(dstHdVBox, HardDiskVariant_Standard, pProgress2.asOutParam());
+                            rc = srcHdVBox->CloneTo(dstHdVBox, HardDiskVariant_Standard, NULL, pProgress2.asOutParam());
                             if (FAILED(rc)) throw rc;
 
@@ -3071 +3071 @@
             {
                 // create a flat copy of the source disk image
-                rc = pSourceDisk->FlattenTo(pTargetDisk, HardDiskVariant_VmdkStreamOptimized, pProgress2.asOutParam());
+                rc = pSourceDisk->CloneTo(pTargetDisk, HardDiskVariant_VmdkStreamOptimized, NULL, pProgress2.asOutParam());
                 if (FAILED(rc)) throw rc;
 

trunk/src/VBox/Main/HardDiskImpl.cpp

@@ -59 +59 @@
 {
     enum Operation { CreateBase, CreateDiff,
-                     Merge, Clone, Flatten, Delete, Reset };
+                     Merge, Clone, Delete, Reset };
 
     HardDisk *that;
@@ -85 +85 @@
     }
 
+    void setData (HardDisk *aTarget, HardDisk *aParent)
+    {
+        d.target = aTarget;
+        HRESULT rc = d.target->addCaller();
+        AssertComRC (rc);
+        d.parentDisk = aParent;
+        if (aParent)
+        {
+            rc = d.parentDisk->addCaller();
+            AssertComRC (rc);
+        }
+    }
+
     void setData (MergeChain *aChain)
     {
@@ -91 +104 @@
     }
 
-    void setData (CloneChain *aChain)
-    {
-        AssertReturnVoid (aChain != NULL);
-        d.source.reset (aChain);
+    void setData (CloneChain *aSrcChain, CloneChain *aParentChain)
+    {
+        AssertReturnVoid (aSrcChain != NULL);
+        AssertReturnVoid (aParentChain != NULL);
+        d.source.reset (aSrcChain);
+        d.parent.reset (aParentChain);
     }
 
@@ -112 +127 @@
         HardDiskVariant_T variant;
 
-        /* CreateDiff, Flatten */
+        /* CreateDiff, Clone */
 
         ComObjPtr<HardDisk> target;
 
-        /* Flatten */
+        /* Clone */
 
         /** Hard disks to open, in {parent,child} order */
         std::auto_ptr <CloneChain> source;
+        /** Hard disks which are parent of target, in {parent,child} order */
+        std::auto_ptr <CloneChain> parent;
+        /** The to-be parent hard disk object */
+        ComObjPtr<HardDisk> parentDisk;
 
         /* Merge */
@@ -1437 +1456 @@
 STDMETHODIMP HardDisk::CloneTo (IHardDisk *aTarget,
                                 HardDiskVariant_T aVariant,
+                                IHardDisk *aParent,
                                 IProgress **aProgress)
 {
@@ -1448 +1468 @@
     HRESULT rc = mVirtualBox->cast (aTarget, target);
     CheckComRCReturnRC (rc);
-
-    AutoMultiWriteLock2 alock (this, target);
-
-    /* We want to be locked for reading as long as the clone hard disk is
-     * being created. */
-    rc = LockRead (NULL);
-    CheckComRCReturnRC (rc);
+    ComObjPtr <HardDisk> parent;
+    if (aParent)
+    {
+        rc = mVirtualBox->cast (aParent, parent);
+        CheckComRCReturnRC (rc);
+    }
+
+    AutoMultiWriteLock3 alock (this, target, parent);
 
     ComObjPtr <Progress> progress;
@@ -1462 +1483 @@
         if (target->m.state != MediaState_NotCreated)
             throw target->setStateError();
+
+        /** @todo separate out creating/locking an image chain from
+         * SessionMachine::lockMedia and use it from here too.
+         * logically this belongs into HardDisk functionality. */
+
+        /* Build the source chain and lock images in the proper order. */
+        std::auto_ptr <CloneChain> srcChain (new CloneChain ());
+
+        /* we walk the source tree */
+        AutoReadLock srcTreeLock (this->treeLock());
+        for (HardDisk *hd = this; hd; hd = hd->mParent)
+        {
+            rc = srcChain->addImage(hd);
+            CheckComRCThrowRC (rc);
+        }
+        rc = srcChain->lockImagesRead();
+        CheckComRCThrowRC (rc);
+
+        /* Build the parent chain and lock images in the proper order. */
+        std::auto_ptr <CloneChain> parentChain (new CloneChain ());
+
+        /* we walk the future parent tree */
+        AutoReadLock parentTreeLock;
+        if (parent)
+            parentTreeLock.attach(parent->treeLock());
+        for (HardDisk *hd = parent; hd; hd = hd->mParent)
+        {
+            rc = parentChain->addImage(hd);
+            CheckComRCThrowRC (rc);
+        }
+        rc = parentChain->lockImagesRead();
+        CheckComRCThrowRC (rc);
 
         progress.createObject();
@@ -1476 +1529 @@
         AssertComRCThrowRC (task->autoCaller.rc());
 
-        task->setData (target);
+        task->setData (target, parent);
         task->d.variant = aVariant;
+        task->setData (srcChain.release(), parentChain.release());
 
         rc = task->startThread();
@@ -1493 +1547 @@
     }
 
-    if (FAILED (rc))
-    {
-        HRESULT rc2 = UnlockRead (NULL);
-        AssertComRC (rc2);
-        /* Note: on success, taskThread() will unlock this */
-    }
-    else
-    {
-        /* return progress to the caller */
-        progress.queryInterfaceTo (aProgress);
-    }
-
-    return rc;
-}
-
-STDMETHODIMP HardDisk::FlattenTo (IHardDisk *aTarget,
-                                  HardDiskVariant_T aVariant,
-                                  IProgress **aProgress)
-{
-    CheckComArgNotNull (aTarget);
-    CheckComArgOutPointerValid (aProgress);
-
-    AutoCaller autoCaller (this);
-    CheckComRCReturnRC (autoCaller.rc());
-
-    ComObjPtr <HardDisk> target;
-    HRESULT rc = mVirtualBox->cast (aTarget, target);
-    CheckComRCReturnRC (rc);
-
-    AutoMultiWriteLock2 alock (this, target);
-
-    ComObjPtr <Progress> progress;
-
-    try
-    {
-        if (target->m.state != MediaState_NotCreated)
-            throw target->setStateError();
-
-        /** @todo separate out creating/locking an image chain from
-         * SessionMachine::lockMedia and use it from here too.
-         * logically this belongs into HardDisk functionality. */
-
-        /* we walk the tree */
-        AutoReadLock treeLock (this->treeLock());
-
-        /* Build the chain and at the end lock images in the proper order. */
-        std::auto_ptr <CloneChain> chain (new CloneChain ());
-        HardDisk *hd = this;
-        do
-        {
-            rc = chain->addImage(hd);
-            CheckComRCThrowRC (rc);
-
-            hd = hd->mParent;
-        } while (hd);
-        rc = chain->lockImagesRead();
-        CheckComRCThrowRC (rc);
-
-        progress.createObject();
-        rc = progress->init (mVirtualBox, static_cast <IHardDisk *> (this),
-            BstrFmt (tr ("Creating flattened clone hard disk '%ls'"),
-                     target->m.locationFull.raw()),
-            TRUE /* aCancelable */);
-        CheckComRCThrowRC (rc);
-
-        /* setup task object and thread to carry out the operation
-         * asynchronously */
-
-        std::auto_ptr <Task> task (new Task (this, progress, Task::Flatten));
-        AssertComRCThrowRC (task->autoCaller.rc());
-
-        task->setData (target);
-        task->d.variant = aVariant;
-        task->setData (chain.release());
-
-        rc = task->startThread();
-        CheckComRCThrowRC (rc);
-
-        /* go to Creating state before leaving the lock */
-        target->m.state = MediaState_Creating;
-
-        /* task is now owned (or already deleted) by taskThread() so release it */
-        task.release();
-    }
-    catch (HRESULT aRC)
-    {
-        rc = aRC;
-    }
-
-    if (FAILED (rc))
-    {
-        HRESULT rc2 = UnlockRead (NULL);
-        AssertComRC (rc2);
-        /* Note: on success, taskThread() will unlock this */
-    }
-    else
+    if (SUCCEEDED (rc))
     {
         /* return progress to the caller */
@@ -4136 +4095 @@
         {
             ComObjPtr<HardDisk> &target = task->d.target;
-
-            /* Lock both in {parent,child} order. The lock is also used as a
+            ComObjPtr<HardDisk> &parent = task->d.parentDisk;
+
+            /* Lock all in {parent,child} order. The lock is also used as a
              * signal from the task initiator (which releases it only after
              * RTThreadCreate()) that we can start the job. */
-            AutoMultiWriteLock2 thatLock (that, target);
+            AutoMultiWriteLock3 thatLock (that, target, parent);
+
+            CloneChain *srcChain = task->d.source.get();
+            CloneChain *parentChain = task->d.parent.get();
 
             uint64_t size = 0, logicalSize = 0;
@@ -4161 +4124 @@
                 ComAssertRCThrow (vrc, E_FAIL);
 
-                Utf8Str format (that->mm.format);
-                Utf8Str location (that->m.locationFull);
-
-                Utf8Str targetFormat (target->mm.format);
-                Utf8Str targetLocation (target->m.locationFull);
-
-                Assert (target->m.state == MediaState_Creating);
-
-                Assert (that->m.state == MediaState_LockedRead);
-
-                /* unlock before the potentially lengthy operation */
-                thatLock.leave();
-
                 try
                 {
-                    vrc = VDOpen (hdd, format, location,
-                                  VD_OPEN_FLAGS_READONLY | VD_OPEN_FLAGS_INFO,
-                                  that->mm.vdDiskIfaces);
-                    if (RT_FAILURE (vrc))
-                    {
-                        throw setError (E_FAIL,
-                            tr ("Could not open the hard disk storage "
-                                "unit '%s'%s"),
-                            location.raw(), that->vdError (vrc).raw());
-                    }
-
-                    /* ensure the target directory exists */
-                    rc = VirtualBox::ensureFilePathExists (targetLocation);
-                    CheckComRCThrowRC (rc);
-
-                    /* needed for vdProgressCallback */
-                    that->mm.vdProgress = task->progress;
-
-                    PVBOXHDD targetHdd;
-                    int vrc = VDCreate (that->mm.vdDiskIfaces, &targetHdd);
-                    ComAssertRCThrow (vrc, E_FAIL);
-
-                    vrc = VDCopy (hdd, 0, targetHdd, targetFormat,
-                                  targetLocation, false, 0, task->d.variant,
-                                  targetId.raw(), NULL,
-                                  target->mm.vdDiskIfaces,
-                                  that->mm.vdDiskIfaces);
-
-                    that->mm.vdProgress = NULL;
-
-                    if (RT_FAILURE (vrc))
-                    {
-                        VDDestroy (targetHdd);
-
-                        throw setError (E_FAIL,
-                            tr ("Could not create the clone hard disk "
-                                "'%s'%s"),
-                            targetLocation.raw(), that->vdError (vrc).raw());
-                    }
-
-                    size = VDGetFileSize (targetHdd, 0);
-                    logicalSize = VDGetSize (targetHdd, 0) / _1M;
-
-                    VDDestroy (targetHdd);
-                }
-                catch (HRESULT aRC) { rc = aRC; }
-
-                VDDestroy (hdd);
-            }
-            catch (HRESULT aRC) { rc = aRC; }
-
-            if (SUCCEEDED (rc))
-            {
-                /* we set mParent & children() (note that thatLock is released
-                 * here), but lock VirtualBox first to follow the rule */
-                AutoMultiWriteLock2 alock (that->mVirtualBox->lockHandle(),
-                                           that->treeLock());
-
-                Assert (target->mParent.isNull());
-
-                if (!that->mParent.isNull())
-                {
-                    /* associate the clone with the original's parent and
-                     * deassociate from VirtualBox */
-                    target->mParent = that->mParent;
-                    that->mParent->addDependentChild (target);
-                    target->mVirtualBox->removeDependentChild (target);
-
-                    /* register with mVirtualBox as the last step and move to
-                     * Created state only on success (leaving an orphan file is
-                     * better than breaking media registry consistency) */
-                    rc = that->mVirtualBox->registerHardDisk(target);
-
-                    if (FAILED (rc))
-                    {
-                        /* break the parent association on failure to register */
-                        target->mVirtualBox->addDependentChild (target);
-                        that->mParent->removeDependentChild (target);
-                        target->mParent.setNull();
-                    }
-                }
-                else
-                {
-                    /* just register  */
-                    rc = that->mVirtualBox->registerHardDisk(target);
-                }
-            }
-
-            thatLock.maybeEnter();
-
-            if (SUCCEEDED (rc))
-            {
-                target->m.state = MediaState_Created;
-
-                target->m.size = size;
-                target->mm.logicalSize = logicalSize;
-            }
-            else
-            {
-                /* back to NotCreated on failure */
-                target->m.state = MediaState_NotCreated;
-
-                /* reset UUID to prevent it from being reused next time */
-                if (generateUuid)
-                    unconst (target->m.id).clear();
-            }
-
-            if (isAsync)
-            {
-                /* unlock ourselves when done (unless in MediaState_LockedWrite
-                 * state because of taking the online snapshot*/
-                if (that->m.state != MediaState_LockedWrite)
-                {
-                    HRESULT rc2 = that->UnlockRead (NULL);
-                    AssertComRC (rc2);
-                }
-            }
-
-            /* Note that in sync mode, it's the caller's responsibility to
-             * unlock the hard disk */
-
-            break;
-        }
-
-        ////////////////////////////////////////////////////////////////////////
-
-        case Task::Flatten:
-        {
-            ComObjPtr<HardDisk> &target = task->d.target;
-
-            /* Lock both in {parent,child} order. The lock is also used as a
-             * signal from the task initiator (which releases it only after
-             * RTThreadCreate()) that we can start the job. */
-            AutoMultiWriteLock2 thatLock (that, target);
-
-            CloneChain *chain = task->d.source.get();
-
-            uint64_t size = 0, logicalSize = 0;
-
-            /* The object may request a specific UUID (through a special form of
-             * the setLocation() argument). Otherwise we have to generate it */
-            Guid targetId = target->m.id;
-            bool generateUuid = targetId.isEmpty();
-            if (generateUuid)
-            {
-                targetId.create();
-                /* VirtualBox::registerHardDisk() will need UUID */
-                unconst (target->m.id) = targetId;
-            }
-
-            try
-            {
-                PVBOXHDD hdd;
-                int vrc = VDCreate (that->mm.vdDiskIfaces, &hdd);
-                ComAssertRCThrow (vrc, E_FAIL);
-
-                try
-                {
-                    /* Open all hard disk images in the chain. */
-                    for (List::const_iterator it = chain->begin();
-                         it != chain->end(); ++ it)
+                    /* Open all hard disk images in the source chain. */
+                    for (List::const_iterator it = srcChain->begin();
+                         it != srcChain->end(); ++ it)
                     {
                         /* sanity check */
                         Assert ((*it)->m.state == MediaState_LockedRead);
 
-                        /** Open all diff images in read-only mode. */
+                        /** Open all images in read-only mode. */
                         vrc = VDOpen (hdd, Utf8Str ((*it)->mm.format),
                                       Utf8Str ((*it)->m.locationFull),
@@ -4363 +4155 @@
 
                     Assert (target->m.state == MediaState_Creating);
+                    Assert (that->m.state == MediaState_LockedRead);
+                    Assert (parent.isNull() || parent->m.state == MediaState_LockedRead);
 
                     /* ensure the target directory exists */
@@ -4375 +4169 @@
                     ComAssertRCThrow (vrc, E_FAIL);
 
-                    vrc = VDCopy (hdd, VD_LAST_IMAGE, targetHdd, targetFormat,
-                                  targetLocation, false, 0, task->d.variant,
-                                  targetId.raw(), NULL,
-                                  target->mm.vdDiskIfaces,
-                                  that->mm.vdDiskIfaces);
-
-                    that->mm.vdProgress = NULL;
-
-                    if (RT_FAILURE (vrc))
+                    try
                     {
-                        VDDestroy (targetHdd);
-
-                        throw setError (E_FAIL,
-                            tr ("Could not create the flattened hard disk "
-                                "'%s'%s"),
-                            targetLocation.raw(), that->vdError (vrc).raw());
+                        /* Open all hard disk images in the parent chain. */
+                        for (List::const_iterator it = parentChain->begin();
+                             it != parentChain->end(); ++ it)
+                        {
+                            /* sanity check */
+                            Assert ((*it)->m.state == MediaState_LockedRead);
+
+                            /** Open all images in read-only mode. */
+                            vrc = VDOpen (hdd, Utf8Str ((*it)->mm.format),
+                                          Utf8Str ((*it)->m.locationFull),
+                                          VD_OPEN_FLAGS_READONLY,
+                                          (*it)->mm.vdDiskIfaces);
+                            if (RT_FAILURE (vrc))
+                            {
+                                throw setError (E_FAIL,
+                                    tr ("Could not open the hard disk storage "
+                                        "unit '%s'%s"),
+                                    Utf8Str ((*it)->m.locationFull).raw(),
+                                    that->vdError (vrc).raw());
+                            }
+                        }
+
+                        vrc = VDCopy (hdd, VD_LAST_IMAGE, targetHdd,
+                                      targetFormat, targetLocation, false, 0,
+                                      task->d.variant, targetId.raw(), NULL,
+                                      target->mm.vdDiskIfaces,
+                                      that->mm.vdDiskIfaces);
+
+                        that->mm.vdProgress = NULL;
+
+                        if (RT_FAILURE (vrc))
+                        {
+                            throw setError (E_FAIL,
+                                tr ("Could not create the clone hard disk "
+                                    "'%s'%s"),
+                                targetLocation.raw(), that->vdError (vrc).raw());
+                        }
+                        size = VDGetFileSize (targetHdd, 0);
+                        logicalSize = VDGetSize (targetHdd, 0) / _1M;
                     }
-
-                    size = VDGetFileSize (targetHdd, 0);
-                    logicalSize = VDGetSize (targetHdd, 0) / _1M;
+                    catch (HRESULT aRC) { rc = aRC; }
 
                     VDDestroy (targetHdd);
@@ -4406 +4223 @@
             if (SUCCEEDED (rc))
             {
+                /* we set mParent & children() (note that thatLock is released
+                 * here), but lock VirtualBox first to follow the rule */
+                AutoMultiWriteLock2 alock (that->mVirtualBox->lockHandle(),
+                                           that->treeLock());
+
                 Assert (target->mParent.isNull());
 
-                /* just register  */
-                rc = that->mVirtualBox->registerHardDisk(target);
+                if (parent)
+                {
+                    /* associate the clone with the parent and deassociate
+                     * from VirtualBox */
+                    target->mParent = parent;
+                    parent->addDependentChild (target);
+                    target->mVirtualBox->removeDependentChild (target);
+
+                    /* register with mVirtualBox as the last step and move to
+                     * Created state only on success (leaving an orphan file is
+                     * better than breaking media registry consistency) */
+                    rc = parent->mVirtualBox->registerHardDisk(target);
+
+                    if (FAILED (rc))
+                    {
+                        /* break parent association on failure to register */
+                        target->mVirtualBox->addDependentChild (target);
+                        parent->removeDependentChild (target);
+                        target->mParent.setNull();
+                    }
+                }
+                else
+                {
+                    /* just register  */
+                    rc = that->mVirtualBox->registerHardDisk(target);
+                }
             }
 

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

@@ -8801 +8801 @@
   <interface
     name="IHardDisk" extends="IMedium"
-    uuid="3498d065-dee6-48bf-bcc5-47018fee4f42"
+    uuid="91648dc6-bb19-46bf-9e1c-4bf5b960c8e2"
     wsmap="managed"
   >
@@ -9535 +9535 @@
     </method>
 
-    <!-- clone methods -->
+    <!-- clone method -->
 
     <method name="cloneTo">
@@ -9545 +9545 @@
         state (i.e. must not have an existing storage unit). Upon successful
         completion, the cloned hard disk will contain exactly the same sector
-        data as the hard disk being cloned, except that a new UUID for the clone
-        will be randomly generated.
+        data as the hard disk being cloned, except that a new UUID for the
+        clone will be randomly generated.
+
+        The @a parent argument defines which hard disk will be the parent
+        of the clone. Passing a NULL reference indicates that the clone will
+        be a base image, i.e. completely independent. It is possible to specify
+        an arbitrary hard disk for this parameter, including the parent of the
+        hard disk which is being cloned. Even cloning to a child of the source
+        hard disk is possible.
 
         After the returned progress object reports that the operation is
@@ -9553 +9560 @@
 
         <note>
-          If the cloned hard disk is a differencing hard disk, it will inherit
-          parent dependency of the original hard disk.
-        </note>
-        <note>
           This hard disk will be placed to <link to="MediaState_LockedRead"/>
           state for the duration of this operation.
@@ -9567 +9570 @@
         <desc>Exact image variant which should be created.</desc>
       </param>
-      <param name="progress" type="IProgress" dir="return">
-        <desc>Progress object to track the operation completion.</desc>
-      </param>
-    </method>
-
-    <method name="flattenTo">
-      <desc>
-        Starts creating a deep (independent) clone of this hard disk in the
-        format and at the location defined by the @a target argument.
-
-        This operation is similar to <link to="#cloneTo"/> except that when
-        applied to a differencing hard disk, it will also copy missing hard disk
-        data from all parent hard disks it is linked to. This will make the
-        created clone an independent base hard disk that contains all hard disk
-        data and does not need any other hard disks to operate.
-
-        After the returned progress object reports that the operation is
-        successfully complete, the target hard disk gets remembered by this
-        VirtualBox installation and may be attached to virtual machines.
-
-        <note>
-          For base hard disks, this operation is identical to
-          <link to="#cloneTo"/>.
-        </note>
-        <note>
-          This hard disk and all its parent hard disks will be placed to <link
-          to="MediaState_LockedRead"/> state for the duration of this
-          operation.
-        </note>
-      </desc>
-      <param name="target" type="IHardDisk" dir="in">
-        <desc>Target hard disk.</desc>
-      </param>
-      <param name="variant" type="HardDiskVariant" dir="in">
-        <desc>Exact image variant which should be created.</desc>
+      <param name="parent" type="IHardDisk" dir="in">
+        <desc>Parent of the cloned hard disk.</desc>
       </param>
       <param name="progress" type="IProgress" dir="return">

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

@@ -126 +126 @@
     STDMETHOD(MergeTo) (IN_GUID aTargetId, IProgress **aProgress);
     STDMETHOD(CloneTo) (IHardDisk *aTarget, HardDiskVariant_T aVariant,
-                        IProgress **aProgress);
-    STDMETHOD(FlattenTo) (IHardDisk *aTarget, HardDiskVariant_T aVariant,
-                          IProgress **aProgress);
+                        IHardDisk *aParent, IProgress **aProgress);
     STDMETHOD(Compact) (IProgress **aProgress);
     STDMETHOD(Reset) (IProgress **aProgress);