Changeset 47817 in vbox for trunk/src/VBox/Main/src-client

Timestamp:

Aug 16, 2013 3:30:15 PM (12 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

88078

Message:

GuestCtrl: Update for IGuestFile; some renaming.

Location:

trunk/src/VBox/Main/src-client

Files:

• GuestFileImpl.cpp (modified) (25 diffs)
• GuestProcessImpl.cpp (modified) (7 diffs)
• GuestSessionImpl.cpp (modified) (7 diffs)

trunk/src/VBox/Main/src-client/GuestFileImpl.cpp

@@ -153 +153 @@
     if (RT_SUCCESS(vrc))
     {
-        mData.mID = 0;
+        mSession = pSession;
+
+        mData.mID = uFileID;
         mData.mInitialSize = 0;
         mData.mStatus = FileStatus_Undefined;
+        mData.mOpenInfo = openInfo;
 
         unconst(mEventSource).createObject();
@@ -273 +276 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    *aDisposition = getDispositionFromString(mData.mOpenInfo.mDisposition);
+    uint32_t uDisposition = 0;
+    /** @todo Fix me! */
+    *aDisposition = uDisposition;
 
     return S_OK;
@@ -363 +368 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    *aOpenMode = getOpenModeFromString(mData.mOpenInfo.mOpenMode);
+    uint32_t uOpenMode = 0;
+    /** @todo Fix me! */
+    *aOpenMode = uOpenMode;
 
     return S_OK;
@@ -398 +405 @@
     AssertPtrReturn(pSvcCb, VERR_INVALID_POINTER);
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     int vrc;
     switch (pCbCtx->uFunction)
@@ -454 +459 @@
     if (RT_SUCCESS(vrc))
         vrc = waitForStatusChange(pEvent, 30 * 1000 /* Timeout in ms */,
-                                  NULL /* FileStatus */);
+                                  NULL /* FileStatus */, pGuestRc);
     unregisterWaitEvent(pEvent);
 
     LogFlowFuncLeaveRC(vrc);
     return vrc;
-}
-
-/* static */
-uint32_t GuestFile::getDispositionFromString(const Utf8Str &strDisposition)
-{
-    return 0; /** @todo Implement me! */
-}
-
-/* static */
-uint32_t GuestFile::getOpenModeFromString(const Utf8Str &strOpenMode)
-{
-    uint32_t uOpenMode = 0;
-
-    const char *pc = strOpenMode.c_str();
-    while (*pc != '\0')
-    {
-        switch (*pc++)
-        {
-            case 'r':
-                uOpenMode |= RTFILE_O_READ;
-                break;
-
-            case 'w':
-                uOpenMode |= RTFILE_O_WRITE;
-                break;
-
-            default:
-                /* Silently skip unknown values. */
-                break;
-        }
-    }
-
-    return uOpenMode;
 }
 
@@ -543 +515 @@
     AssertPtrReturn(pSvcCbData, VERR_INVALID_POINTER);
 
+    LogFlowThisFuncEnter();
+
     if (pSvcCbData->mParms < 3)
         return VERR_INVALID_PARAMETER;
@@ -548 +522 @@
     int vrc = VINF_SUCCESS;
 
-    int idx = 0; /* Current parameter index. */
+    int idx = 1; /* Current parameter index. */
     CALLBACKDATA_FILE_NOTIFY dataCb;
     /* pSvcCb->mpaParms[0] always contains the context ID. */
@@ -554 +528 @@
     pSvcCbData->mpaParms[idx++].getUInt32(&dataCb.rc);
 
+    FileStatus_T fileStatus = FileStatus_Undefined;
     int guestRc = (int)dataCb.rc; /* uint32_t vs. int. */
 
+    LogFlowFunc(("uType=%RU32, guestRc=%Rrc\n",
+                 dataCb.uType, guestRc));
+
+    if (RT_FAILURE(guestRc))
+    {
+        int rc2 = setFileStatus(FileStatus_Error, guestRc);
+        AssertRC(rc2);
+
+        return VINF_SUCCESS; /* Report to the guest. */
+    }
+
     switch (dataCb.uType)
     {
         case GUEST_FILE_NOTIFYTYPE_ERROR:
         {
-            AssertMsg(mData.mStatus != FileStatus_Error, ("File status already set to error\n"));
-
             int rc2 = setFileStatus(FileStatus_Error, guestRc);
             AssertRC(rc2);
@@ -573 +557 @@
                 pSvcCbData->mpaParms[idx++].getUInt32(&dataCb.u.open.uHandle);
 
-                AssertMsg(mData.mID == 0, ("File ID already set to %RU32\n", mData.mID));
-                mData.mID = dataCb.u.open.uHandle;
-                AssertMsg(mData.mID == VBOX_GUESTCTRL_CONTEXTID_GET_OBJECT(pCbCtx->uContextID),
-                          ("File ID %RU32 does not match context ID %RU32\n", mData.mID,
-                           VBOX_GUESTCTRL_CONTEXTID_GET_OBJECT(pCbCtx->uContextID)));
+                {
+                    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+                    AssertMsg(mData.mID == VBOX_GUESTCTRL_CONTEXTID_GET_OBJECT(pCbCtx->uContextID),
+                              ("File ID %RU32 does not match context ID %RU32\n", mData.mID,
+                               VBOX_GUESTCTRL_CONTEXTID_GET_OBJECT(pCbCtx->uContextID)));
+                }
 
                 /* Set the process status. */
@@ -599 +584 @@
 
         case GUEST_FILE_NOTIFYTYPE_READ:
+        {
             if (pSvcCbData->mParms == 4)
             {
@@ -606 +592 @@
                 if (cbRead)
                 {
+                    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
                     mData.mOffCurrent += cbRead;
+
+                    alock.release();
 
                     com::SafeArray<BYTE> data((size_t)cbRead);
                     data.initFrom((BYTE*)dataCb.u.read.pvData, cbRead);
+
                     fireGuestFileReadEvent(mEventSource, mSession, this, mData.mOffCurrent,
                                            cbRead, ComSafeArrayAsInParam(data));
@@ -617 +608 @@
                 vrc = VERR_NOT_SUPPORTED;
             break;
+        }
 
         case GUEST_FILE_NOTIFYTYPE_WRITE:
+        {
             if (pSvcCbData->mParms == 4)
             {
                 pSvcCbData->mpaParms[idx++].getUInt32(&dataCb.u.write.cbWritten);
 
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
                 mData.mOffCurrent += dataCb.u.write.cbWritten;
+                uint64_t uOffCurrent = mData.mOffCurrent;
+
+                alock.release();
 
                 if (dataCb.u.write.cbWritten)
-                    fireGuestFileWriteEvent(mEventSource, mSession, this, mData.mOffCurrent,
+                    fireGuestFileWriteEvent(mEventSource, mSession, this, uOffCurrent,
                                             dataCb.u.write.cbWritten);
             }
@@ -632 +630 @@
                 vrc = VERR_NOT_SUPPORTED;
             break;
+        }
 
         case GUEST_FILE_NOTIFYTYPE_SEEK:
+        {
             if (pSvcCbData->mParms == 4)
             {
                 pSvcCbData->mpaParms[idx++].getUInt64(&dataCb.u.seek.uOffActual);
 
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
                 mData.mOffCurrent = dataCb.u.seek.uOffActual;
+                uint64_t uOffCurrent = mData.mOffCurrent;
+
+                alock.release();
 
                 if (dataCb.u.seek.uOffActual)
                     fireGuestFileOffsetChangedEvent(mEventSource, mSession, this,
-                                                    mData.mOffCurrent, 0 /* Processed */);
+                                                    uOffCurrent, 0 /* Processed */);
             }
             else
                 vrc = VERR_NOT_SUPPORTED;
             break;
+        }
 
         case GUEST_FILE_NOTIFYTYPE_TELL:
+        {
             if (pSvcCbData->mParms == 4)
             {
                 pSvcCbData->mpaParms[idx++].getUInt64(&dataCb.u.tell.uOffActual);
 
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
                 if (mData.mOffCurrent != dataCb.u.tell.uOffActual)
                 {
                     mData.mOffCurrent = dataCb.u.tell.uOffActual;
+                    uint64_t uOffCurrent = mData.mOffCurrent;
+
+                    alock.release();
 
                     fireGuestFileOffsetChangedEvent(mEventSource, mSession, this,
-                                                    mData.mOffCurrent, 0 /* Processed */);
+                                                    uOffCurrent, 0 /* Processed */);
                 }
             }
@@ -664 +676 @@
                 vrc = VERR_NOT_SUPPORTED;
             break;
+        }
 
         default:
@@ -670 +683 @@
     }
 
-    LogFlowThisFunc(("strName=%s, uType=%RU32, guestRc=%Rrc\n",
-                     mData.mOpenInfo.mFileName.c_str(), dataCb.uType, dataCb.rc));
-
-    if (RT_SUCCESS(vrc))
-    {
-        /* Nothing to do here yet. */
-    }
-    else if (vrc == VERR_NOT_SUPPORTED)
-    {
-        /* Also let the callback know. */
-        guestRc = VERR_NOT_SUPPORTED;
-    }
+    LogFlowThisFunc(("uType=%RU32, guestRc=%Rrc\n",
+                     dataCb.uType, dataCb.rc));
 
     LogFlowFuncLeaveRC(vrc);
@@ -692 +695 @@
     AssertPtrReturn(pSvcCbData, VERR_INVALID_POINTER);
 
-    LogFlowThisFunc(("strFile=%s\n",
-                     mData.mOpenInfo.mFileName.c_str()));
-
     int vrc = setFileStatus(FileStatus_Down, VINF_SUCCESS);
 
@@ -701 +701 @@
 }
 
-int GuestFile::openFile(int *pGuestRc)
-{
-    LogFlowThisFunc(("strFile=%s, strOpenMode=%s, strDisposition=%s, uCreationMode=%RU32\n",
+int GuestFile::openFile(uint32_t uTimeoutMS, int *pGuestRc)
+{
+    LogFlowThisFuncEnter();
+
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    LogFlowThisFunc(("strFile=%s, strOpenMode=%s, strDisposition=%s, uCreationMode=%RU32, uOffset=%RU64\n",
                      mData.mOpenInfo.mFileName.c_str(), mData.mOpenInfo.mOpenMode.c_str(),
-                     mData.mOpenInfo.mDisposition.c_str(), mData.mOpenInfo.mCreationMode));
+                     mData.mOpenInfo.mDisposition.c_str(), mData.mOpenInfo.mCreationMode, mData.mOpenInfo.mInitialOffset));
     int vrc;
 
@@ -734 +738 @@
     paParms[i++].setPointer((void*)mData.mOpenInfo.mDisposition.c_str(),
                             (ULONG)mData.mOpenInfo.mDisposition.length() + 1);
+    paParms[i++].setPointer((void*)mData.mOpenInfo.mSharingMode.c_str(),
+                            (ULONG)mData.mOpenInfo.mSharingMode.length() + 1);
     paParms[i++].setUInt32(mData.mOpenInfo.mCreationMode);
     paParms[i++].setUInt64(mData.mOpenInfo.mInitialOffset);
 
+    alock.release(); /* Drop read lock before sending. */
+
     vrc = sendCommand(HOST_FILE_OPEN, i, paParms);
     if (RT_SUCCESS(vrc))
-        vrc = waitForStatusChange(pEvent, 30 * 1000 /* Timeout in ms */,
-                                  NULL /* FileStatus */);
+        vrc = waitForStatusChange(pEvent, uTimeoutMS,
+                                  NULL /* FileStatus */, pGuestRc);
 
     unregisterWaitEvent(pEvent);
@@ -903 +911 @@
 }
 
-/* Does not do locking; caller is responsible for that! */
 int GuestFile::setFileStatus(FileStatus_T fileStatus, int fileRc)
 {
+    LogFlowThisFuncEnter();
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     LogFlowThisFunc(("oldStatus=%ld, newStatus=%ld, fileRc=%Rrc\n",
                      mData.mStatus, fileStatus, fileRc));
@@ -920 +931 @@
     if (mData.mStatus != fileStatus)
     {
-        mData.mStatus = fileStatus;
+        mData.mStatus    = fileStatus;
+        mData.mLastError = fileRc;
 
         ComObjPtr<VirtualBoxErrorInfo> errorInfo;
@@ -927 +939 @@
         if (RT_FAILURE(fileRc))
         {
-            int rc2 = errorInfo->initEx(VBOX_E_IPRT_ERROR, fileRc,
-                                        COM_IIDOF(IGuestFile), getComponentName(),
-                                        guestErrorToString(fileRc));
-            AssertRC(rc2);
-        }
+            hr = errorInfo->initEx(VBOX_E_IPRT_ERROR, fileRc,
+                                   COM_IIDOF(IGuestFile), getComponentName(),
+                                   guestErrorToString(fileRc));
+            ComAssertComRC(hr);
+        }
+
+        /* Copy over necessary data before releasing lock again. */
+        FileStatus_T fileStatus = mData.mStatus;
+
+        alock.release(); /* Release lock before firing off event. */
 
         fireGuestFileStateChangedEvent(mEventSource, mSession,
-                                       this, mData.mStatus, errorInfo);
+                                       this, fileStatus, errorInfo);
     }
 
@@ -1013 +1030 @@
 }
 
-int GuestFile::waitForStatusChange(GuestWaitEvent *pEvent,
-                                   uint32_t uTimeoutMS, FileStatus_T *pFileStatus)
+int GuestFile::waitForStatusChange(GuestWaitEvent *pEvent, uint32_t uTimeoutMS,
+                                   FileStatus_T *pFileStatus, int *pGuestRc)
 {
     AssertPtrReturn(pEvent, VERR_INVALID_POINTER);
+    /* pFileStatus is optional. */
 
     VBoxEventType_T evtType;
@@ -1028 +1046 @@
         Assert(!pFileEvent.isNull());
 
-        HRESULT hr = pFileEvent->COMGETTER(Status)(pFileStatus);
+        HRESULT hr;
+        if (pFileStatus)
+        {
+            hr = pFileEvent->COMGETTER(Status)(pFileStatus);
+            ComAssertComRC(hr);
+        }
+
+        ComPtr<IVirtualBoxErrorInfo> errorInfo;
+        hr = pFileEvent->COMGETTER(Error)(errorInfo.asOutParam());
         ComAssertComRC(hr);
+
+        LONG lGuestRc;
+        hr = errorInfo->COMGETTER(ResultDetail)(&lGuestRc);
+        ComAssertComRC(hr);
+
+        LogFlowThisFunc(("resultDetail=%RI32 (rc=%Rrc)\n",
+                         lGuestRc, lGuestRc));
+
+        if (RT_FAILURE((int)lGuestRc))
+            vrc = VERR_GSTCTL_GUEST_ERROR;
+
+        if (pGuestRc)
+            *pGuestRc = (int)lGuestRc;
     }
 
@@ -1197 +1236 @@
         rc = rc2;
 
-    /*
-     * Release autocaller before calling uninit.
-     */
-    autoCaller.release();
-
-    uninit();
-
-    LogFlowFuncLeaveRC(rc);
     if (RT_FAILURE(rc))
     {
@@ -1214 +1245 @@
     }
 
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */

trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp

@@ -239 +239 @@
         mData.mExitCode = 0;
         mData.mPID = 0;
-        mData.mRC = VINF_SUCCESS;
+        mData.mLastError = VINF_SUCCESS;
         mData.mStatus = ProcessStatus_Undefined;
         /* Everything else will be set by the actual starting routine. */
@@ -967 +967 @@
         /* Do not allow overwriting an already set error. If this happens
          * this means we forgot some error checking/locking somewhere. */
-        AssertMsg(RT_SUCCESS(mData.mRC), ("Guest rc already set (to %Rrc)\n", mData.mRC));
+        AssertMsg(RT_SUCCESS(mData.mLastError), ("Guest rc already set (to %Rrc)\n", mData.mLastError));
     }
     else
@@ -976 +976 @@
     if (mData.mStatus != procStatus) /* Was there a process status change? */
     {
-        mData.mStatus = procStatus;
-        mData.mRC     = procRc;
+        mData.mStatus    = procStatus;
+        mData.mLastError = procRc;
 
         ComObjPtr<VirtualBoxErrorInfo> errorInfo;
         HRESULT hr = errorInfo.createObject();
         ComAssertComRC(hr);
-        if (RT_FAILURE(mData.mRC))
-        {
-            int rc2 = errorInfo->initEx(VBOX_E_IPRT_ERROR, mData.mRC,
-                                        COM_IIDOF(IGuestProcess), getComponentName(),
-                                        guestErrorToString(mData.mRC));
-            AssertRC(rc2);
+        if (RT_FAILURE(mData.mLastError))
+        {
+            hr = errorInfo->initEx(VBOX_E_IPRT_ERROR, mData.mLastError,
+                                   COM_IIDOF(IGuestProcess), getComponentName(),
+                                   guestErrorToString(mData.mLastError));
+            ComAssertComRC(hr);
         }
 
         /* Copy over necessary data before releasing lock again. */
         uint32_t uPID =  mData.mPID;
-        ProcessStatus_T uStatus = mData.mStatus;
+        ProcessStatus_T procStatus = mData.mStatus;
         /** @todo Also handle mSession? */
 
@@ -998 +998 @@
 
         fireGuestProcessStateChangedEvent(mEventSource, mSession, this,
-                                          uPID, uStatus, errorInfo);
+                                          uPID, procStatus, errorInfo);
 #if 0
         /*
@@ -1408 +1408 @@
 
     LogFlowThisFunc(("fWaitFlags=0x%x, uTimeoutMS=%RU32, procStatus=%RU32, procRc=%Rrc, pGuestRc=%p\n",
-                     fWaitFlags, uTimeoutMS, mData.mStatus, mData.mRC, pGuestRc));
+                     fWaitFlags, uTimeoutMS, mData.mStatus, mData.mLastError, pGuestRc));
 
     /* Did some error occur before? Then skip waiting and return. */
@@ -1414 +1414 @@
     {
         waitResult = ProcessWaitResult_Error;
-        AssertMsg(RT_FAILURE(mData.mRC), ("No error rc (%Rrc) set when guest process indicated an error\n", mData.mRC));
+        AssertMsg(RT_FAILURE(mData.mLastError), ("No error rc (%Rrc) set when guest process indicated an error\n", mData.mLastError));
         if (pGuestRc)
-            *pGuestRc = mData.mRC; /* Return last set error. */
+            *pGuestRc = mData.mLastError; /* Return last set error. */
         return VERR_GSTCTL_GUEST_ERROR;
     }
@@ -1427 +1427 @@
     {
         if (pGuestRc)
-            *pGuestRc = mData.mRC; /* Return last set error (if any). */
-        return RT_SUCCESS(mData.mRC) ? VINF_SUCCESS : VERR_GSTCTL_GUEST_ERROR;
+            *pGuestRc = mData.mLastError; /* Return last set error (if any). */
+        return RT_SUCCESS(mData.mLastError) ? VINF_SUCCESS : VERR_GSTCTL_GUEST_ERROR;
     }
 

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -1005 +1005 @@
                              Utf8Str(strName).c_str(), mData.mSession.mID, mData.mFiles.size() - 1, mData.mNumObjects - 1));
 
-            itFiles->second->Release();
+            pFile->cancelWaitEvents();
+            pFile->Release();
 
             mData.mFiles.erase(itFiles);
@@ -1054 +1055 @@
 int GuestSession::fileOpenInternal(const GuestFileOpenInfo &openInfo, ComObjPtr<GuestFile> &pFile, int *pGuestRc)
 {
-    LogFlowThisFunc(("strPath=%s, strOpenMode=%s, strDisposition=%s, uCreationMode=%x, iOffset=%RI64\n",
+    LogFlowThisFunc(("strPath=%s, strOpenMode=%s, strDisposition=%s, uCreationMode=%x, uOffset=%RU64\n",
                      openInfo.mFileName.c_str(), openInfo.mOpenMode.c_str(), openInfo.mDisposition.c_str(),
                      openInfo.mCreationMode, openInfo.mInitialOffset));
@@ -1111 +1112 @@
         return rc;
 
-    int guestRc;
-    rc = pFile->openFile(&guestRc);
+    /*
+     * Since this is a synchronous guest call we have to
+     * register the file object first, releasing the session's
+     * lock and then proceed with the actual opening command
+     * -- otherwise the file's opening callback would hang
+     * because the session's lock still is in place.
+     */
+    try
+    {
+        /* Add the created file to our vector. */
+        mData.mFiles[uNewFileID] = pFile;
+        mData.mNumObjects++;
+        Assert(mData.mNumObjects <= VBOX_GUESTCTRL_MAX_OBJECTS);
+
+        LogFlowFunc(("Added new guest file \"%s\" (Session: %RU32) (now total %ld files, %ld objects)\n",
+                     openInfo.mFileName.c_str(), mData.mSession.mID, mData.mFiles.size(), mData.mNumObjects));
+
+        alock.release(); /* Release lock before firing off event. */
+
+        fireGuestFileRegisteredEvent(mEventSource, this, pFile,
+                                     true /* Registered */);
+    }
+    catch (std::bad_alloc &)
+    {
+        rc = VERR_NO_MEMORY;
+    }
+
     if (RT_SUCCESS(rc))
     {
-        try
-        {
-            /* Add the created file to our vector. */
-            mData.mFiles[uNewFileID] = pFile;
-            mData.mNumObjects++;
-            Assert(mData.mNumObjects <= VBOX_GUESTCTRL_MAX_OBJECTS);
-
-            LogFlowFunc(("Added new guest file \"%s\" (Session: %RU32) (now total %ld files, %ld objects)\n",
-                         openInfo.mFileName.c_str(), mData.mSession.mID, mData.mFiles.size(), mData.mNumObjects));
-
-            alock.release(); /* Release lock before firing off event. */
-
-            fireGuestFileRegisteredEvent(mEventSource, this, pFile,
-                                         true /* Registered */);
-            if (pGuestRc)
-                *pGuestRc = guestRc;
-        }
-        catch (std::bad_alloc &)
-        {
-            rc = VERR_NO_MEMORY;
+        int guestRc;
+        rc = pFile->openFile(30 * 1000 /* 30s timeout */, &guestRc);
+        if (   rc == VERR_GSTCTL_GUEST_ERROR
+            && pGuestRc)
+        {
+            *pGuestRc = guestRc;
         }
     }
@@ -2728 +2740 @@
 }
 
-STDMETHODIMP GuestSession::FileOpen(IN_BSTR aPath, IN_BSTR aOpenMode, IN_BSTR aDisposition, ULONG aCreationMode, LONG64 aOffset, IGuestFile **aFile)
+STDMETHODIMP GuestSession::FileOpen(IN_BSTR aPath, IN_BSTR aOpenMode, IN_BSTR aDisposition, ULONG aCreationMode, IGuestFile **aFile)
+{
+#ifndef VBOX_WITH_GUEST_CONTROL
+    ReturnComNotImplemented();
+#else
+    LogFlowThisFuncEnter();
+
+    Bstr strSharingMode = ""; /* Sharing mode is ignored. */
+
+    return FileOpenEx(aPath, aOpenMode, aDisposition, strSharingMode.raw(), aCreationMode,
+                      0 /* aOffset */, aFile);
+#endif /* VBOX_WITH_GUEST_CONTROL */
+}
+
+STDMETHODIMP GuestSession::FileOpenEx(IN_BSTR aPath, IN_BSTR aOpenMode, IN_BSTR aDisposition, IN_BSTR aSharingMode,
+                                      ULONG aCreationMode, LONG64 aOffset, IGuestFile **aFile)
 {
 #ifndef VBOX_WITH_GUEST_CONTROL
@@ -2741 +2768 @@
     if (RT_UNLIKELY((aDisposition) == NULL || *(aDisposition) == '\0'))
         return setError(E_INVALIDARG, tr("No disposition mode specified"));
+    /* aSharingMode is optional. */
 
     CheckComArgOutPointerValid(aFile);
@@ -2750 +2778 @@
     if (FAILED(hr))
         return hr;
-
-    /** @todo Validate open mode. */
-    /** @todo Validate disposition mode. */
 
     /** @todo Validate creation mode. */
@@ -2761 +2786 @@
     openInfo.mOpenMode = Utf8Str(aOpenMode);
     openInfo.mDisposition = Utf8Str(aDisposition);
+    openInfo.mSharingMode = Utf8Str(aSharingMode);
     openInfo.mCreationMode = aCreationMode;
     openInfo.mInitialOffset = aOffset;
 
+    uint64_t uFlagsIgnored;
+    int vrc = RTFileModeToFlagsEx(openInfo.mOpenMode.c_str(),
+                                  openInfo.mDisposition.c_str(),
+                                  openInfo.mSharingMode.c_str(),
+                                  &uFlagsIgnored);
+    if (RT_FAILURE(vrc))
+        return setError(E_INVALIDARG, tr("Invalid open mode / disposition / sharing mode specified"));
+
     ComObjPtr <GuestFile> pFile; int guestRc;
-    int vrc = fileOpenInternal(openInfo, pFile, &guestRc);
+    vrc = fileOpenInternal(openInfo, pFile, &guestRc);
     if (RT_SUCCESS(vrc))
     {