Changeset 24691 in vbox for trunk/src/VBox/Main

Timestamp:

Nov 16, 2009 12:46:23 PM (15 years ago)

Author:

vboxsync

Message:

Main/MediumImpl: fix medium state refresh/locking order, as the refresh cannot be performed if the image is locked for writing. Consistent with SessionMachine::lockMedia again.

File:

• trunk/src/VBox/Main/MediumImpl.cpp (modified) (5 diffs)

trunk/src/VBox/Main/MediumImpl.cpp

@@ -614 +614 @@
  */
 class Medium::ImageChain : public Medium::List,
-                             public com::SupportErrorInfoBase
+                           public com::SupportErrorInfoBase
 {
 public:
@@ -671 +671 @@
         {
             HRESULT rc = S_OK;
-            MediumState_T mediaState;
-            rc = (*it)->LockRead(&mediaState);
-            CheckComRCReturnRC(rc);
-
-            if (mediaState == MediumState_Inaccessible)
+            MediumState_T mediumState = (*it)->state();
+
+            /* accessibility check must be first, otherwise locking
+             * interferes with getting the medium state. */
+            if (mediumState == MediumState_Inaccessible)
             {
-                rc = (*it)->RefreshState(&mediaState);
+                rc = (*it)->RefreshState(&mediumState);
                 CheckComRCReturnRC(rc);
-                Assert(mediaState == MediumState_LockedRead);
-
-                /* 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());
-                CheckComRCReturnRC(rc);
-
-                if (!error.isEmpty())
+
+                if (mediumState == MediumState_Inaccessible)
                 {
+                    Bstr error;
+                    rc = (*it)->COMGETTER(LastAccessError)(error.asOutParam());
+                    CheckComRCReturnRC(rc);
+
                     Bstr loc;
                     rc = (*it)->COMGETTER(Location)(loc.asOutParam());
-                    CheckComRCThrowRC(rc);
+                    CheckComRCReturnRC(rc);
 
                     /* collect multiple errors */
@@ -705 +702 @@
                 }
             }
+
+            rc = (*it)->LockRead(&mediumState);
+            CheckComRCReturnRC(rc);
         }
 
@@ -725 +725 @@
         {
             HRESULT rc = S_OK;
-            MediumState_T mediaState;
-            if (it == last)
-                rc = (*it)->LockWrite(&mediaState);
-            else
-                rc = (*it)->LockRead(&mediaState);
-            CheckComRCReturnRC(rc);
-
-            if (mediaState == MediumState_Inaccessible)
+            MediumState_T mediumState = (*it)->state();
+
+            /* accessibility check must be first, otherwise locking
+             * interferes with getting the medium state. */
+            if (mediumState == MediumState_Inaccessible)
             {
-                rc = (*it)->RefreshState(&mediaState);
+                rc = (*it)->RefreshState(&mediumState);
                 CheckComRCReturnRC(rc);
-                if (it == last)
-                    Assert(mediaState == MediumState_LockedWrite);
-                else
-                    Assert(mediaState == MediumState_LockedRead);
-
-                /* 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());
-                CheckComRCReturnRC(rc);
-
-                if (!error.isEmpty())
+
+                if (mediumState == MediumState_Inaccessible)
                 {
+                    Bstr error;
+                    rc = (*it)->COMGETTER(LastAccessError)(error.asOutParam());
+                    CheckComRCReturnRC(rc);
+
                     Bstr loc;
                     rc = (*it)->COMGETTER(Location)(loc.asOutParam());
-                    CheckComRCThrowRC(rc);
+                    CheckComRCReturnRC(rc);
 
                     /* collect multiple errors */
@@ -765 +756 @@
                 }
             }
+
+            if (it == last)
+                rc = (*it)->LockWrite(&mediumState);
+            else
+                rc = (*it)->LockRead(&mediumState);
         }