Changeset 17651 in vbox for trunk

Timestamp:

Mar 10, 2009 10:27:08 PM (16 years ago)

Author:

vboxsync

Message:

Main: Reset differencing hard disks for which auto-reset is set to true (#3425).

Location:

trunk/src/VBox/Main

Files:

• ConsoleImpl.cpp (modified) (8 diffs)
• MachineImpl.cpp (modified) (2 diffs)
• idl/VirtualBox.xidl (modified) (2 diffs)
• include/MachineImpl.h (modified) (5 diffs)

trunk/src/VBox/Main/ConsoleImpl.cpp

@@ -189 +189 @@
         , mSetVMErrorCallback (NULL), mConfigConstructor (NULL), mStartPaused (false) {}
 
-    ~VMPowerUpTask()
-    {
-        /* No null output parameters in IPC*/
-        MediaState_T dummy;
-
-        /* we may be holding important error info on the current thread;
-         * preserve it */
-        ErrorInfoKeeper eik;
-
-        /* if the locked media list is not empty, treat as a failure and
-         * unlock all */
-        for (LockedMedia::const_iterator it = lockedMedia.begin();
-             it != lockedMedia.end(); ++ it)
-        {
-            if (it->second)
-                it->first->UnlockWrite (&dummy);
-            else
-                it->first->UnlockRead (&dummy);
-        }
-    }
-
     PFNVMATERROR mSetVMErrorCallback;
     PFNCFGMCONSTRUCTOR mConfigConstructor;
@@ -216 +195 @@
     bool mStartPaused;
 
-    /**
-     * Successfully locked media list. The 2nd value in the pair is true if the
-     * medium is locked for writing and false if locked for reading.
-     */
-    typedef std::list <std::pair <ComPtr <IMedium>, bool > > LockedMedia;
-    LockedMedia lockedMedia;
-
-    /** Media that need an accessibility check */
-    typedef std::list <ComPtr <IMedium> > Media;
-    Media mediaToCheck;
+    typedef std::list <ComPtr <IHardDisk> > HardDiskList;
+    HardDiskList hardDisks;
+
+    /* array of progress objects for hard disk reset operations */
+    typedef std::list <ComPtr <IProgress> > ProgressList;
+    ProgressList hardDiskProgresses;
 };
 
@@ -4382 +4357 @@
     }
 
-    /* create an IProgress object to track progress of this operation */
-    ComObjPtr <Progress> progress;
-    progress.createObject();
+    /* create a progress object to track progress of this operation */
+    ComObjPtr <Progress> powerupProgress;
+    powerupProgress.createObject();
     Bstr progressDesc;
     if (mMachineState == MachineState_Saved)
@@ -4390 +4365 @@
     else
         progressDesc = tr ("Starting virtual machine");
-    rc = progress->init (static_cast <IConsole *> (this),
-                         progressDesc, FALSE /* aCancelable */);
+    rc = powerupProgress->init (static_cast <IConsole *> (this),
+                                progressDesc, FALSE /* aCancelable */);
     CheckComRCReturnRC (rc);
-
-    /* pass reference to caller if requested */
-    if (aProgress)
-        progress.queryInterfaceTo (aProgress);
 
     /* setup task object and thread to carry out the operation
      * asynchronously */
 
-    std::auto_ptr <VMPowerUpTask> task (new VMPowerUpTask (this, progress));
+    std::auto_ptr <VMPowerUpTask> task (new VMPowerUpTask (this, powerupProgress));
     ComAssertComRCRetRC (task->rc());
 
@@ -4411 +4382 @@
         task->mSavedStateFile = savedStateFile;
 
-    /* Lock all attached media in necessary mode. Note that until
-     * setMachineState() is called below, it is OUR responsibility to unlock
-     * media on failure (and VMPowerUpTask::lockedMedia is used for that). After
-     * the setMachineState() call, VBoxSVC (SessionMachine::setMachineState())
-     * will unlock all the media upon the appropriate state change. Note that
-     * media accessibility checks are performed on the powerup thread because
-     * they may block. */
-
-    MediaState_T mediaState;
-
-    /* lock all hard disks for writing and their parents for reading */
+    /* Reset differencing hard disks for which autoReset is true */
     {
         com::SafeIfaceArray <IHardDiskAttachment> atts;
@@ -4434 +4395 @@
             CheckComRCReturnRC (rc);
 
-            bool first = true;
-
-            while (!hardDisk.isNull())
+            /* save for later use on the powerup thread */
+            task->hardDisks.push_back (hardDisk);
+
+            /* needs autoreset? */
+            BOOL autoReset = FALSE;
+            rc = hardDisk->COMGETTER(AutoReset)(&autoReset);
+            CheckComRCReturnRC (rc);
+
+            if (autoReset)
             {
-                if (first)
-                {
-                    rc = hardDisk->LockWrite (&mediaState);
-                    CheckComRCReturnRC (rc);
-
-                    task->lockedMedia.push_back (VMPowerUpTask::LockedMedia::
-                                                 value_type (hardDisk, true));
-                    first = false;
-                }
-                else
-                {
-                    rc = hardDisk->LockRead (&mediaState);
-                    CheckComRCReturnRC (rc);
-
-                    task->lockedMedia.push_back (VMPowerUpTask::LockedMedia::
-                                                 value_type (hardDisk, false));
-                }
-
-                if (mediaState == MediaState_Inaccessible)
-                    task->mediaToCheck.push_back (hardDisk);
-
-                ComPtr <IHardDisk> parent;
-                rc = hardDisk->COMGETTER(Parent) (parent.asOutParam());
+                ComPtr <IProgress> resetProgress;
+                rc = hardDisk->Reset (resetProgress.asOutParam());
                 CheckComRCReturnRC (rc);
-                hardDisk = parent;
+
+                /* save for later use on the powerup thread */
+                task->hardDiskProgresses.push_back (resetProgress);
             }
         }
     }
-    /* lock the DVD image for reading if mounted */
-    {
-        ComPtr <IDVDDrive> drive;
-        rc = mMachine->COMGETTER(DVDDrive) (drive.asOutParam());
-        CheckComRCReturnRC (rc);
-
-        DriveState_T driveState;
-        rc = drive->COMGETTER(State) (&driveState);
-        CheckComRCReturnRC (rc);
-
-        if (driveState == DriveState_ImageMounted)
-        {
-            ComPtr <IDVDImage> image;
-            rc = drive->GetImage (image.asOutParam());
-            CheckComRCReturnRC (rc);
-
-            rc = image->LockRead (&mediaState);
-            CheckComRCReturnRC (rc);
-
-            task->lockedMedia.push_back (VMPowerUpTask::LockedMedia::
-                                         value_type (image, false));
-
-            if (mediaState == MediaState_Inaccessible)
-                task->mediaToCheck.push_back (image);
-        }
-    }
-    /* lock the floppy image for reading if mounted */
-    {
-        ComPtr <IFloppyDrive> drive;
-        rc = mMachine->COMGETTER(FloppyDrive) (drive.asOutParam());
-        CheckComRCReturnRC (rc);
-
-        DriveState_T driveState;
-        rc = drive->COMGETTER(State) (&driveState);
-        CheckComRCReturnRC (rc);
-
-        if (driveState == DriveState_ImageMounted)
-        {
-            ComPtr<IFloppyImage> image;
-            rc = drive->GetImage (image.asOutParam());
-            CheckComRCReturnRC (rc);
-
-            rc = image->LockRead (&mediaState);
-            CheckComRCReturnRC (rc);
-
-            task->lockedMedia.push_back (VMPowerUpTask::LockedMedia::
-                                         value_type (image, false));
-
-            if (mediaState == MediaState_Inaccessible)
-                task->mediaToCheck.push_back (image);
-        }
-    }
-    /* SUCCEEDED locking all media */
 
     rc = consoleInitReleaseLog (mMachine);
     CheckComRCReturnRC (rc);
 
+    /* pass the progress object to the caller if requested */
+    if (aProgress)
+    {
+        if (task->hardDiskProgresses.size() == 0)
+        {
+            /* there are no other operations to track, return the powerup
+             * progress only */
+            powerupProgress.queryInterfaceTo (aProgress);
+        }
+        else
+        {
+            /* create a combined progress object */
+            ComObjPtr <CombinedProgress> progress;
+            progress.createObject();
+            VMPowerUpTask::ProgressList progresses (task->hardDiskProgresses);
+            progresses.push_back (ComPtr <IProgress> (powerupProgress));
+            rc = progress->init (static_cast <IConsole *> (this),
+                                 progressDesc, progresses.begin(),
+                                 progresses.end());
+            AssertComRCReturnRC (rc);
+            progress.queryInterfaceTo (aProgress);
+        }
+    }
+
     int vrc = RTThreadCreate (NULL, Console::powerUpThread, (void *) task.get(),
                               0, RTTHREADTYPE_MAIN_WORKER, 0, "VMPowerUp");
@@ -4529 +4448 @@
                        E_FAIL);
 
-    /* clear the locked media list to prevent unlocking on task destruction as
-     * we are not going to fail after this point */
-    task->lockedMedia.clear();
-
     /* task is now owned by powerUpThread(), so release it */
     task.release();
 
-    /* finally, set the state: no right to fail in this method afterwards! */
+    /* finally, set the state: no right to fail in this method afterwards
+     * since we've already started the thread and it is now responsible for
+     * any error reporting and appropriate state change! */
 
     if (mMachineState == MachineState_Saved)
@@ -6334 +6251 @@
     try
     {
-        {
-            ErrorInfoKeeper eik (true /* aIsNull */);
-            MultiResult mrc (S_OK);
-
-            /* perform a check of inaccessible media deferred in PowerUp() */
-            for (VMPowerUpTask::Media::const_iterator
-                 it = task->mediaToCheck.begin();
-                 it != task->mediaToCheck.end(); ++ it)
-            {
-                MediaState_T mediaState;
-                rc = (*it)->COMGETTER(State) (&mediaState);
-                CheckComRCThrowRC (rc);
-
-                Assert (mediaState == MediaState_LockedRead ||
-                        mediaState == MediaState_LockedWrite);
-
-                /* Note that we locked the medium already, so use the error
-                 * value to see if there was an accessibility failure */
-
-                Bstr error;
-                rc = (*it)->COMGETTER(LastAccessError) (error.asOutParam());
-                CheckComRCThrowRC (rc);
-
-                if (!error.isNull())
-                {
-                    Bstr loc;
-                    rc = (*it)->COMGETTER(Location) (loc.asOutParam());
-                    CheckComRCThrowRC (rc);
-
-                    /* collect multiple errors */
-                    eik.restore();
-
-                    /* be in sync with MediumBase::setStateError() */
-                    Assert (!error.isEmpty());
-                    mrc = setError (E_FAIL,
-                        tr ("Medium '%ls' is not accessible. %ls"),
-                        loc.raw(), error.raw());
-
-                    eik.fetch();
-                }
-            }
-
-            eik.restore();
-            CheckComRCThrowRC ((HRESULT) mrc);
-        }
+        /* wait for auto reset ops to complete so that we can successfully lock
+         * the attached hard disks by calling LockMedia() below */
+        for (VMPowerUpTask::ProgressList::const_iterator
+             it = task->hardDiskProgresses.begin();
+             it != task->hardDiskProgresses.end(); ++ it)
+        {
+            HRESULT rc2 = (*it)->WaitForCompletion (-1);
+            AssertComRC (rc2);
+        }
+
+        /* lock attached media. This method will also check their
+         * accessibility. Note that the media will be unlocked automatically
+         * by SessionMachine::setMachineState() when the VM is powered down. */
+        rc = console->mControl->LockMedia();
+        CheckComRCThrowRC (rc);
 
 #ifdef VBOX_WITH_VRDP

trunk/src/VBox/Main/MachineImpl.cpp

@@ -10368 +10368 @@
 
 /**
+ * Locks the attached media.
+ *
+ * All attached hard disks and DVD/floppy are locked for writing. Parents of
+ * attached hard disks (if any) are locked for reading.
+ *
+ * This method also performs accessibility check of all media it locks: if some
+ * media is inaccessible, the method will return a failure and a bunch of
+ * extended error info objects per each inaccessible medium.
+ *
+ * Note that this method is atomic: if it returns a success, all media are
+ * locked as described above; on failure no media is locked at all (all
+ * succeeded individual locks will be undone).
+ *
+ * This method is intended to be called when the machine is in Starting or
+ * Restoring state and asserts otherwise.
+ *
+ * The locks made by this method must be undone by calling #unlockMedia() when
+ * no more needed.
+ */
+HRESULT SessionMachine::lockMedia()
+{
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), autoCaller.rc());
+
+    AutoWriteLock alock (this);
+
+    AssertReturn (mData->mMachineState == MachineState_Starting ||
+                  mData->mMachineState == MachineState_Restoring, E_FAIL);
+
+    typedef std::list <ComPtr <IMedium> > MediaList;
+    MediaList mediaToCheck;
+    MediaState_T mediaState;
+
+    try
+    {
+        HRESULT rc = S_OK;
+
+        /* lock hard disks */
+        for (HDData::AttachmentList::const_iterator it =
+                 mHDData->mAttachments.begin();
+             it != mHDData->mAttachments.end(); ++ it)
+        {
+            ComObjPtr<HardDisk> hd = (*it)->hardDisk();
+
+            bool first = true;
+
+            while (!hd.isNull())
+            {
+                if (first)
+                {
+                    rc = hd->LockWrite (&mediaState);
+                    CheckComRCThrowRC (rc);
+
+                    mData->mSession.mLockedMedia.push_back (
+                        Data::Session::LockedMedia::value_type (
+                            ComPtr <IHardDisk> (hd), true));
+
+                    first = false;
+                }
+                else
+                {
+                    rc = hd->LockRead (&mediaState);
+                    CheckComRCThrowRC (rc);
+
+                    mData->mSession.mLockedMedia.push_back (
+                        Data::Session::LockedMedia::value_type (
+                            ComPtr <IHardDisk> (hd), false));
+                }
+
+                if (mediaState == MediaState_Inaccessible)
+                    mediaToCheck.push_back (ComPtr <IHardDisk> (hd));
+
+                /* no locks or callers here since there should be no way to
+                 * change the hard disk parent at this point (as it is still
+                 * attached to the machine) */
+                hd = hd->parent();
+            }
+        }
+
+        /* lock the DVD image for reading if mounted */
+        {
+            AutoReadLock driveLock (mDVDDrive);
+            if (mDVDDrive->data()->state == DriveState_ImageMounted)
+            {
+                ComObjPtr <DVDImage> image = mDVDDrive->data()->image;
+
+                rc = image->LockRead (&mediaState);
+                CheckComRCThrowRC (rc);
+
+                mData->mSession.mLockedMedia.push_back (
+                    Data::Session::LockedMedia::value_type (
+                        ComPtr <IDVDImage> (image), false));
+
+                if (mediaState == MediaState_Inaccessible)
+                    mediaToCheck.push_back (ComPtr <IDVDImage> (image));
+            }
+        }
+
+        /* lock the floppy image for reading if mounted */
+        {
+            AutoReadLock driveLock (mFloppyDrive);
+            if (mFloppyDrive->data()->state == DriveState_ImageMounted)
+            {
+                ComObjPtr <FloppyImage> image = mFloppyDrive->data()->image;
+
+                rc = image->LockRead (&mediaState);
+                CheckComRCThrowRC (rc);
+
+                mData->mSession.mLockedMedia.push_back (
+                    Data::Session::LockedMedia::value_type (
+                        ComPtr <IFloppyImage> (image), false));
+
+                if (mediaState == MediaState_Inaccessible)
+                    mediaToCheck.push_back (ComPtr <IFloppyImage> (image));
+            }
+        }
+
+        /* SUCCEEDED locking all media, now check accessibility */
+
+        ErrorInfoKeeper eik (true /* aIsNull */);
+        MultiResult mrc (S_OK);
+
+        /* perform a check of inaccessible media deferred above */
+        for (MediaList::const_iterator
+             it = mediaToCheck.begin();
+             it != mediaToCheck.end(); ++ it)
+        {
+            MediaState_T mediaState;
+            rc = (*it)->COMGETTER(State) (&mediaState);
+            CheckComRCThrowRC (rc);
+
+            Assert (mediaState == MediaState_LockedRead ||
+                    mediaState == MediaState_LockedWrite);
+
+            /* Note that we locked the medium already, so use the error
+             * value to see if there was an accessibility failure */
+
+            Bstr error;
+            rc = (*it)->COMGETTER(LastAccessError) (error.asOutParam());
+            CheckComRCThrowRC (rc);
+
+            if (!error.isNull())
+            {
+                Bstr loc;
+                rc = (*it)->COMGETTER(Location) (loc.asOutParam());
+                CheckComRCThrowRC (rc);
+
+                /* collect multiple errors */
+                eik.restore();
+
+                /* be in sync with MediumBase::setStateError() */
+                Assert (!error.isEmpty());
+                mrc = setError (E_FAIL,
+                    tr ("Medium '%ls' is not accessible. %ls"),
+                    loc.raw(), error.raw());
+
+                eik.fetch();
+            }
+        }
+
+        eik.restore();
+        CheckComRCThrowRC ((HRESULT) mrc);
+    }
+    catch (HRESULT aRC)
+    {
+        /* Unlock all locked media on failure */
+        unlockMedia();
+        return aRC;
+    }
+
+    return S_OK;
+}
+
+/**
+ * Undoes the locks made by by #lockMedia().
+ */
+void SessionMachine::unlockMedia()
+{
+    AutoCaller autoCaller (this);
+    AssertComRCReturnVoid (autoCaller.rc());
+
+    AutoWriteLock alock (this);
+
+    /* we may be holding important error info on the current thread;
+     * preserve it */
+    ErrorInfoKeeper eik;
+
+    HRESULT rc = S_OK;
+
+    for (Data::Session::LockedMedia::const_iterator
+         it = mData->mSession.mLockedMedia.begin();
+         it != mData->mSession.mLockedMedia.end(); ++ it)
+    {
+        if (it->second)
+            rc = it->first->UnlockWrite (NULL);
+        else
+            rc = it->first->UnlockRead (NULL);
+
+        AssertComRC (rc);
+    }
+
+    mData->mSession.mLockedMedia.clear();
+}
+
+/**
  * Helper to change the machine state (reimplementation).
  *
@@ -10417 +10622 @@
          mSnapshotData.mLastState >= MachineState_Running))
     {
-        /* The EMT thread has just stopped, unlock attached media. Note that
-         * opposed to locking, we do unlocking here because the VM process may
-         * have just aborted before properly unlocking all media it locked. */
-
-        for (HDData::AttachmentList::const_iterator it =
-                 mHDData->mAttachments.begin();
-             it != mHDData->mAttachments.end(); ++ it)
-        {
-            ComObjPtr<HardDisk> hd = (*it)->hardDisk();
-
-            bool first = true;
-
-            while (!hd.isNull())
-            {
-                if (first)
-                {
-                    rc = hd->UnlockWrite (NULL);
-                    AssertComRC (rc);
-
-                    first = false;
-                }
-                else
-                {
-                    rc = hd->UnlockRead (NULL);
-                    AssertComRC (rc);
-                }
-
-                /* no locks or callers here since there should be no way to
-                 * change the hard disk parent at this point (as it is still
-                 * attached to the machine) */
-                hd = hd->parent();
-            }
-        }
-        {
-            AutoReadLock driveLock (mDVDDrive);
-            if (mDVDDrive->data()->state == DriveState_ImageMounted)
-            {
-                rc = mDVDDrive->data()->image->UnlockRead (NULL);
-                AssertComRC (rc);
-            }
-        }
-        {
-            AutoReadLock driveLock (mFloppyDrive);
-            if (mFloppyDrive->data()->state == DriveState_ImageMounted)
-            {
-                rc = mFloppyDrive->data()->image->UnlockRead (NULL);
-                AssertComRC (rc);
-            }
-        }
+        /* The EMT thread has just stopped, unlock attached media. Note that as
+         * opposed to locking that is done from Console, we do unlocking here
+         * because the VM process may have aborted before having a chance to
+         * properly unlock all media it locked. */
+
+        unlockMedia();
     }
 

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

@@ -3261 +3261 @@
   <interface
      name="IInternalMachineControl" extends="$unknown"
-     uuid="4042ddf2-93d3-4749-8517-dde3f17ea630"
+     uuid="2c88b969-7a74-4ef3-b95f-8a209a1535f3"
      internal="yes"
      wsmap="suppress"
@@ -3612 +3612 @@
         </desc>
       </param>
+    </method>
+
+    <method name="lockMedia">
+      <desc>
+        Locks all media attached to the machine for writing and parents of
+        attahced different hard disks (if any) for reading. This operation is
+        atomic so that if it fails no media is actually locked.
+
+        This method is intended to be called when the machine is in Starting or
+        Restoring state. The locked media will be automatically unlocked when
+        the machine is powered off or crashed.
+      </desc>
     </method>
   </interface>

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

@@ -90 +90 @@
 
     /**
-     *  Internal machine data.
-     *
-     *  Only one instance of this data exists per every machine --
-     *  it is shared by the Machine, SessionMachine and all SnapshotMachine
-     *  instances associated with the given machine using the util::Shareable
-     *  template through the mData variable.
-     *
-     *  @note |const| members are persistent during lifetime so can be
-     *  accessed without locking.
-     *
-     *  @note There is no need to lock anything inside init() or uninit()
-     *  methods, because they are always serialized (see AutoCaller).
+     * Internal machine data.
+     *
+     * Only one instance of this data exists per every machine -- it is shared
+     * by the Machine, SessionMachine and all SnapshotMachine instances
+     * associated with the given machine using the util::Shareable template
+     * through the mData variable.
+     *
+     * @note |const| members are persistent during lifetime so can be
+     * accessed without locking.
+     *
+     * @note There is no need to lock anything inside init() or uninit()
+     * methods, because they are always serialized (see AutoCaller).
      */
     struct Data
     {
         /**
-         *  Data structure to hold information about sessions opened for the
-         *  given machine.
+         * Data structure to hold information about sessions opened for the
+         * given machine.
          */
         struct Session
@@ -123 +123 @@
 
             /**
-             *  PID of the session object that must be passed to openSession()
-             *  to finalize the openRemoteSession() request
-             *  (i.e., PID of the process created by openRemoteSession())
+             * PID of the session object that must be passed to openSession() to
+             * finalize the openRemoteSession() request (i.e., PID of the
+             * process created by openRemoteSession())
              */
             RTPROCESS mPid;
@@ -137 +137 @@
             /** Session machine object */
             ComObjPtr <SessionMachine> mMachine;
+
+            /**
+             * Successfully locked media list. The 2nd value in the pair is true
+             * if the medium is locked for writing and false if locked for
+             * reading.
+             */
+            typedef std::list <std::pair <ComPtr <IMedium>, bool > > LockedMedia;
+            LockedMedia mLockedMedia;
         };
 
@@ -904 +912 @@
     STDMETHOD(PushGuestProperty) (IN_BSTR aName, IN_BSTR aValue,
                                   ULONG64 aTimestamp, IN_BSTR aFlags);
+    STDMETHOD(LockMedia)() { return lockMedia(); }
 
     // public methods only for internal purposes
@@ -967 +976 @@
     void discardSnapshotHandler (DiscardSnapshotTask &aTask);
     void discardCurrentStateHandler (DiscardCurrentStateTask &aTask);
+
+    HRESULT lockMedia();
+    void unlockMedia();
 
     HRESULT setMachineState (MachineState_T aMachineState);