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

Timestamp:

Jan 26, 2012 12:54:50 AM (13 years ago)

Author:

vboxsync

Message:

Config.kmk,VMMDev,Main,QtGui,VBoxManage: Refactored IGuest::additionsVersion and associated acts, splitting it up into additionsVersion and additionsRevision like IVirtualBox and IExtPack handles versioning. Fixed missing saved state in VMMDev where the VMMDevReq_ReportGuestInfo2 info was not saved and Main+Frontends led to believe we were running guest additions older than 3.2. The changes have be subjected to limited testing. Added TODOs for another missing save in VMMDev.

Location:

trunk/src/VBox/Main

Files:

• idl/VirtualBox.xidl (modified) (4 diffs)
• include/GuestImpl.h (modified) (3 diffs)
• src-client/GuestImpl.cpp (modified) (5 diffs)
• src-client/VMMDevInterface.cpp (modified) (4 diffs)

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

@@ -1415 +1415 @@
         format is 3 integer numbers divided by dots (e.g. 1.0.1). The
         last number represents the build number and will frequently change.
+
+        This may be followed by a _ALPHA[0-9]*, _BETA[0-9]* or _RC[0-9]* tag
+        in prerelease builds.  Non-Oracle builds may (/shall) also have a
+        publisher tag, at the end.  The publisher tag starts with an underscore
+        just like the prerelease build type tag.
       </desc>
     </attribute>
@@ -8668 +8673 @@
   <interface
     name="IGuest" extends="$unknown"
-    uuid="1E404590-608E-492C-B00A-90C9EEB6C33F"
+    uuid="88696240-7411-4fe6-bb5e-ef56fb8a61f3"
     wsmap="managed"
     >
@@ -8703 +8708 @@
     <attribute name="additionsVersion" type="wstring" readonly="yes">
       <desc>
-        Version of the Guest Additions including the revision (3 decimal numbers
-        separated by dots + revision number) installed on the guest or empty
-        when the Additions are not installed.
+        Version of the Guest Additions in the same format as
+        <link to="IVirtualBox::version"/>.
+      </desc>
+    </attribute>
+
+    <attribute name="additionsRevision" type="unsigned long" readonly="yes">
+      <desc>
+        The internal build revision number of the additions.
+
+        See also <link to="IVirtualBox::revision"/>.
       </desc>
     </attribute>
@@ -16031 +16043 @@
       <desc>
         The extension pack version string.  This is restricted to the dotted
-        version number and a build indicator.  No tree revision or tag will be
-        included in the string as those things are available as separate
-        properties.  Examples: "1.2.3", "1.2.3_BETA1" and "1.2.3_RC2".
+        version number and optionally a build indicator.  No tree revision or
+        tag will be included in the string as those things are available as
+        separate properties.  An optional publisher tag may be present like for
+        <link to="IVirtualBox::version"/>.
+
+        Examples: "1.2.3", "1.2.3_BETA1" and "1.2.3_RC2".
       </desc>
     </attribute>

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

@@ -82 +82 @@
     STDMETHOD(COMGETTER(OSTypeId)) (BSTR *aOSTypeId);
     STDMETHOD(COMGETTER(AdditionsRunLevel)) (AdditionsRunLevelType_T *aRunLevel);
-    STDMETHOD(COMGETTER(AdditionsVersion)) (BSTR *aAdditionsVersion);
+    STDMETHOD(COMGETTER(AdditionsVersion))(BSTR *a_pbstrAdditionsVersion);
+    STDMETHOD(COMGETTER(AdditionsRevision))(ULONG *a_puAdditionsRevision);
     STDMETHOD(COMGETTER(Facilities)) (ComSafeArrayOut(IAdditionsFacility*, aFacilities));
     STDMETHOD(COMGETTER(MemoryBalloonSize)) (ULONG *aMemoryBalloonSize);
@@ -133 +134 @@
     // Public methods that are not in IDL (only called internally).
     void setAdditionsInfo(Bstr aInterfaceVersion, VBOXOSTYPE aOsType);
-    void setAdditionsInfo2(const char *a_pszVersion, const char *a_pszVersionName, uint32_t a_uRevision);
+    void setAdditionsInfo2(uint32_t a_uFullVersion, const char *a_pszName, uint32_t a_uRevision, uint32_t a_fFeatures);
     bool facilityIsActive(VBoxGuestFacilityType enmFacility);
     HRESULT facilityUpdate(VBoxGuestFacilityType enmFacility, VBoxGuestFacilityStatus enmStatus);
@@ -287 +288 @@
     struct Data
     {
-        Data() : mAdditionsRunLevel (AdditionsRunLevelType_None) {}
+        Data() : mAdditionsRunLevel(AdditionsRunLevelType_None)
+            , mAdditionsVersionFull(0), mAdditionsRevision(0), mAdditionsFeatures(0)
+        { }
 
         Bstr                    mOSTypeId;
         FacilityMap             mFacilityMap;
         AdditionsRunLevelType_T mAdditionsRunLevel;
-        Bstr                    mAdditionsVersion;
-        //ULONG                   mAdditionsRevision;
+        uint32_t                mAdditionsVersionFull;
+        Bstr                    mAdditionsVersionNew;
+        uint32_t                mAdditionsRevision;
+        uint32_t                mAdditionsFeatures;
         Bstr                    mInterfaceVersion;
     };

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

@@ -36 +36 @@
 #include <iprt/cpp/utils.h>
 #include <VBox/vmm/pgm.h>
+#include <VBox/version.h>
 
 // defines
@@ -164 +165 @@
 /////////////////////////////////////////////////////////////////////////////
 
-STDMETHODIMP Guest::COMGETTER(OSTypeId)(BSTR *aOSTypeId)
-{
-    CheckComArgOutPointerValid(aOSTypeId);
-
-    AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    /* Redirect the call to IMachine if no additions are installed. */
-    if (mData.mAdditionsVersion.isEmpty())
-        return mParent->machine()->COMGETTER(OSTypeId)(aOSTypeId);
-
-    mData.mOSTypeId.cloneTo(aOSTypeId);
-
-    return S_OK;
-}
-
-STDMETHODIMP Guest::COMGETTER(AdditionsRunLevel) (AdditionsRunLevelType_T *aRunLevel)
-{
-    AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    *aRunLevel = mData.mAdditionsRunLevel;
-
-    return S_OK;
-}
-
-STDMETHODIMP Guest::COMGETTER(AdditionsVersion) (BSTR *aAdditionsVersion)
-{
-    CheckComArgOutPointerValid(aAdditionsVersion);
-
-    AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    HRESULT hr = S_OK;
-    if (   !mData.mAdditionsVersion.isEmpty()
-        || !mData.mAdditionsRunLevel > AdditionsRunLevelType_None)
-        mData.mAdditionsVersion.cloneTo(aAdditionsVersion);
-    else
-    {
-        /*
-         * If we got back an empty string from GetAdditionsVersion() we either
-         * really don't have the Guest Additions version yet or the guest is running
-         * older Guest Additions (< 3.2.0) which don't provide VMMDevReq_ReportGuestInfo2,
-         * so get the version + revision from the (hopefully) provided guest properties
-         * instead.
-         */
-        Bstr addVersion;
-        LONG64 u64Timestamp;
-        Bstr flags;
-        hr = mParent->machine()->GetGuestProperty(Bstr("/VirtualBox/GuestAdd/Version").raw(),
-                                                  addVersion.asOutParam(), &u64Timestamp, flags.asOutParam());
-        if (hr == S_OK)
-        {
-            Bstr addRevision;
-            hr = mParent->machine()->GetGuestProperty(Bstr("/VirtualBox/GuestAdd/Revision").raw(),
-                                                      addRevision.asOutParam(), &u64Timestamp, flags.asOutParam());
-            if (   hr == S_OK
-                && !addVersion.isEmpty()
-                && !addRevision.isEmpty())
-            {
-                /* Some Guest Additions versions had interchanged version + revision values,
-                 * so check if the version value at least has a dot to identify it and change
-                 * both values to reflect the right content. */
-                if (!Utf8Str(addVersion).contains("."))
-                {
-                    Bstr addTemp = addVersion;
-                    addVersion = addRevision;
-                    addRevision = addTemp;
-                }
-
-                /** @todo r=bird: See comment about the space before 'r' in
-                 *        setAdditionsInfo2. */
-                Bstr additionsVersion = BstrFmt("%ls r%ls",
-                                                addVersion.raw(), addRevision.raw());
-                additionsVersion.cloneTo(aAdditionsVersion);
-            }
-            /** @todo r=bird: else: Should not return failure! */
-        }
+STDMETHODIMP Guest::COMGETTER(OSTypeId)(BSTR *a_pbstrOSTypeId)
+{
+    CheckComArgOutPointerValid(a_pbstrOSTypeId);
+
+    AutoCaller autoCaller(this);
+    HRESULT hrc = autoCaller.rc();
+    if (SUCCEEDED(hrc))
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+        if (!mData.mInterfaceVersion.isEmpty())
+            mData.mOSTypeId.cloneTo(a_pbstrOSTypeId);
         else
         {
-            /* If getting the version + revision above fails or they simply aren't there
-             * because of *really* old Guest Additions we only can report the interface
-             * version to at least have something. */
-            mData.mInterfaceVersion.cloneTo(aAdditionsVersion);
-            /** @todo r=bird: hr is still indicating failure! */
+            /* Redirect the call to IMachine if no additions are installed. */
+            ComPtr<IMachine> ptrMachine(mParent->machine());
+            alock.release();
+            hrc = ptrMachine->COMGETTER(OSTypeId)(a_pbstrOSTypeId);
         }
     }
-
-    return hr;
+    return hrc;
+}
+
+STDMETHODIMP Guest::COMGETTER(AdditionsRunLevel) (AdditionsRunLevelType_T *aRunLevel)
+{
+    AutoCaller autoCaller(this);
+    if (FAILED(autoCaller.rc())) return autoCaller.rc();
+
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    *aRunLevel = mData.mAdditionsRunLevel;
+
+    return S_OK;
+}
+
+STDMETHODIMP Guest::COMGETTER(AdditionsVersion)(BSTR *a_pbstrAdditionsVersion)
+{
+    CheckComArgOutPointerValid(a_pbstrAdditionsVersion);
+
+    AutoCaller autoCaller(this);
+    HRESULT hrc = autoCaller.rc();
+    if (SUCCEEDED(hrc))
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+        /*
+         * Return the ReportGuestInfo2 version info if available.
+         */
+        if (   !mData.mAdditionsVersionNew.isEmpty()
+            || mData.mAdditionsRunLevel <= AdditionsRunLevelType_None)
+            mData.mAdditionsVersionNew.cloneTo(a_pbstrAdditionsVersion);
+        else
+        {
+            /*
+             * If we're running older guest additions (< 3.2.0) try get it from
+             * the guest properties.
+             */
+            ComPtr<IMachine> ptrMachine = mParent->machine();
+            alock.release(); /* No need to hold this during the IPC fun. */
+
+            Bstr bstr;
+            hrc = ptrMachine->GetGuestPropertyValue(Bstr("/VirtualBox/GuestAdd/Version").raw(), bstr.asOutParam());
+            if (SUCCEEDED(hrc))
+                bstr.detachTo(a_pbstrAdditionsVersion);
+            else
+            {
+                /* Returning 1.4 is better than nothing. */
+                alock.acquire();
+                mData.mInterfaceVersion.cloneTo(a_pbstrAdditionsVersion);
+                hrc = S_OK;
+            }
+        }
+    }
+    return hrc;
+}
+
+STDMETHODIMP Guest::COMGETTER(AdditionsRevision)(ULONG *a_puAdditionsRevision)
+{
+    CheckComArgOutPointerValid(a_puAdditionsRevision);
+
+    AutoCaller autoCaller(this);
+    HRESULT hrc = autoCaller.rc();
+    if (SUCCEEDED(hrc))
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+        /*
+         * Return the ReportGuestInfo2 version info if available.
+         */
+        if (   !mData.mAdditionsVersionNew.isEmpty()
+            || mData.mAdditionsRunLevel <= AdditionsRunLevelType_None)
+            *a_puAdditionsRevision = mData.mAdditionsRevision;
+        else
+        {
+            /*
+             * If we're running older guest additions (< 3.2.0) try get it from
+             * the guest properties.
+             */
+            ComPtr<IMachine> ptrMachine = mParent->machine();
+            alock.release(); /* No need to hold this during the IPC fun. */
+
+            Bstr bstr;
+            hrc = ptrMachine->GetGuestPropertyValue(Bstr("/VirtualBox/GuestAdd/Revision").raw(), bstr.asOutParam());
+            if (SUCCEEDED(hrc))
+            {
+                Utf8Str str(bstr);
+                uint32_t uRevision;
+                int vrc = RTStrToUInt32Full(str.c_str(), 0, &uRevision);
+                if (vrc == VINF_SUCCESS)
+                    *a_puAdditionsRevision = uRevision;
+                else
+                    hrc = VBOX_E_IPRT_ERROR;
+            }
+            if (FAILED(hrc))
+            {
+                /* Return 0 if we don't know. */
+                *a_puAdditionsRevision = 0;
+                hrc = 0;
+            }
+        }
+    }
+    return hrc;
 }
 
@@ -725 +752 @@
      * don't have the Additions version set.
      */
-    if (mData.mAdditionsVersion.isEmpty())
+    if (mData.mAdditionsVersionNew.isEmpty())
     {
         if (aInterfaceVersion.isEmpty())
@@ -760 +787 @@
 /**
  * Sets the Guest Additions version information details.
- * Gets called by vmmdevUpdateGuestInfo2.
  *
- * @param   a_pszVersion            The GuestInfo2 numbers turned into
- * @param   a_pszVersionName        This turns out to be the version string +
- *                                  beta/alpha/whatever suffix, duplicating info
- *                                  passed in @a a_pszVersion.
- * @param   a_uRevision             The SVN revision number.
+ * Gets called by vmmdevUpdateGuestInfo2 and vmmdevUpdateGuestInfo (to clear the
+ * state).
+ *
+ * @param   a_uFullVersion          VBoxGuestInfo2::additionsMajor,
+ *                                  VBoxGuestInfo2::additionsMinor and
+ *                                  VBoxGuestInfo2::additionsBuild combined into
+ *                                  one value by VBOX_FULL_VERSION_MAKE.
+ *
+ *                                  When this is 0, it's vmmdevUpdateGuestInfo
+ *                                  calling to reset the state.
+ *
+ * @param   a_pszName               Build type tag and/or publisher tag, empty
+ *                                  string if neiter of those are present.
+ * @param   a_uRevision             See VBoxGuestInfo2::additionsRevision.
+ * @param   a_fFeatures             See VBoxGuestInfo2::additionsFeatures.
  */
-void Guest::setAdditionsInfo2(const char *a_pszVersion, const char *a_pszVersionName, uint32_t a_uRevision)
+void Guest::setAdditionsInfo2(uint32_t a_uFullVersion, const char *a_pszName, uint32_t a_uRevision, uint32_t a_fFeatures)
 {
     AutoCaller autoCaller(this);
@@ -775 +811 @@
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    /** @todo r=bird: WHY is this code returning "1.2.3 r45678" in one case and
-     *        "1.2.3r45678" in the else?  Why aren't we doing it the same way as
-     *        IVirtualBox? One version attribute and one revision attribute, no
-     *        abigiuos spaces! */
-    if (*a_pszVersionName != '\0')
-        /*
-         * aVersionName could be "x.y.z_BETA1_FOOBAR", so append revision manually to
-         * become "x.y.z_BETA1_FOOBAR r12345".
-         */
-        mData.mAdditionsVersion = BstrFmt("%s r%u", a_pszVersionName, a_uRevision);
-    else /* aAdditionsVersion is in x.y.zr12345 format. */
-        mData.mAdditionsVersion = Bstr(a_pszVersion);
-    //mData.mAdditionsRevision = a_uRevision;
+    if (a_uFullVersion)
+    {
+        mData.mAdditionsVersionNew  = BstrFmt(*a_pszName ? "%u.%u.%u_%s" : "%u.%u.%u",
+                                              VBOX_FULL_VERSION_GET_MAJOR(a_uFullVersion),
+                                              VBOX_FULL_VERSION_GET_MINOR(a_uFullVersion),
+                                              VBOX_FULL_VERSION_GET_BUILD(a_uFullVersion),
+                                              a_pszName);
+        mData.mAdditionsVersionFull = a_uFullVersion;
+        mData.mAdditionsRevision    = a_uRevision;
+        mData.mAdditionsFeatures    = a_fFeatures;
+    }
+    else
+    {
+        Assert(!a_fFeatures && !a_uRevision && !*a_pszName);
+        mData.mAdditionsVersionNew.setNull();
+        mData.mAdditionsVersionFull = 0;
+        mData.mAdditionsRevision    = 0;
+        mData.mAdditionsFeatures    = 0;
+    }
 }
 

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

@@ -188 +188 @@
 /**
  * Reports Guest Additions API and OS version.
- * Called whenever the Additions issue a guest version report request or the VM is reset.
+ *
+ * Called whenever the Additions issue a guest version report request or the VM
+ * is reset.
  *
  * @param   pInterface          Pointer to this interface.
@@ -232 +234 @@
          */
         guest->setAdditionsInfo(Bstr(), guestInfo->osType); /* Clear interface version + OS type. */
-        guest->setAdditionsInfo2("", "", 0); /* Clear Guest Additions version. */
+        /** @todo Would be better if GuestImpl.cpp did all this in the above method call
+         *        while holding down the. */
+        guest->setAdditionsInfo2(0, "", 0,  0); /* Clear Guest Additions version. */
         guest->setAdditionsStatus(VBoxGuestFacilityType_All,
                                   VBoxGuestFacilityStatus_Inactive,
@@ -241 +245 @@
 
 /**
- * Reports the detailed Guest Additions version.
- * Called whenever the Additions issue a guest version report request or the VM is reset.
- *
- * @param   pInterface          Pointer to this interface.
- * @param   pGuestInfo          Pointer to Guest Additions information
- *                              structure.
- * @thread  The emulation thread.
- */
-DECLCALLBACK(void) vmmdevUpdateGuestInfo2(PPDMIVMMDEVCONNECTOR pInterface, const VBoxGuestInfo2 *pGuestInfo)
-{
-    PDRVMAINVMMDEV pDrv = PDMIVMMDEVCONNECTOR_2_MAINVMMDEV(pInterface);
-    AssertPtr(pGuestInfo);
+ * @interface_method_impl{PDMIVMMDEVCONNECTOR,pfnUpdateGuestInfo2}
+ */
+DECLCALLBACK(void) vmmdevUpdateGuestInfo2(PPDMIVMMDEVCONNECTOR pInterface, uint32_t uFullVersion,
+                                          const char *pszName, uint32_t uRevision, uint32_t fFeatures)
+{
+    PDRVMAINVMMDEV pDrv = PDMIVMMDEVCONNECTOR_2_MAINVMMDEV(pInterface);
+    AssertPtr(pszName);
+    Assert(uFullVersion);
 
     /* Store that information in IGuest. */
@@ -260 +260 @@
         return;
 
-    if (   pGuestInfo->additionsMajor    != 0
-        && pGuestInfo->additionsRevision != 0)
-    {
-        /** @todo r=bird: See comments on space before 'r' in setAdditionsInfo2! */
-        char szVersion[32];
-        RTStrPrintf(szVersion, sizeof(szVersion), "%d.%d.%dr%ld",
-                    pGuestInfo->additionsMajor,
-                    pGuestInfo->additionsMinor,
-                    pGuestInfo->additionsBuild,
-                    pGuestInfo->additionsRevision);
-
-        pGuest->setAdditionsInfo2(szVersion, pGuestInfo->szName, pGuestInfo->additionsRevision);
-
-        /*
-         * No need to tell the console interface about the update;
-         * vmmdevUpdateGuestInfo takes care of that when called as the
-         * last event in the chain.
-         */
-    }
+    /* Just pass it on... */
+    pGuest->setAdditionsInfo2(uFullVersion, pszName, uRevision, fFeatures);
+
+    /*
+     * No need to tell the console interface about the update;
+     * vmmdevUpdateGuestInfo takes care of that when called as the
+     * last event in the chain.
+     */
 }