Changeset 75663 in vbox for trunk/src/VBox/Main/src-server

Timestamp:

Nov 22, 2018 2:00:59 PM (6 years ago)

Author:

vboxsync

Message:

Main/ExtPackManager+CloudProviderManager+VirtualBox: Handle runtime install and uninstall of extension packs containing cloud providers. This is a safeguard against uninstalling active cloud providers (which would cause crashes), and makes the install more convcenient. At the same time it eliminated the need to have a special "detect all cloud providers" logic when VBoxSVC is started. It achieves perfect singleton property of the per-extpack CloudProviderManager and the related CloudProvider objects. Nothing is ever created twice (which would cause trouble with coordinating e.g. the profile file updates).

Location:

trunk/src/VBox/Main/src-server

Files:

• CloudProviderManagerImpl.cpp (modified) (4 diffs)
• VirtualBoxImpl.cpp (modified) (2 diffs)

trunk/src/VBox/Main/src-server/CloudProviderManagerImpl.cpp

@@ -25 +25 @@
 #include "Logging.h"
 
-using namespace std;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -53 +52 @@
 }
 
-HRESULT CloudProviderManager::init(VirtualBox *aParent)
+HRESULT CloudProviderManager::init()
 {
     // Enclose the state transition NotReady->InInit->Ready.
@@ -61 +60 @@
     m_apCloudProviders.clear();
 
-#ifdef VBOX_WITH_EXTPACK
-    // Engage the extension pack manager and get all the implementations of
-    // this class and all implemented cloud providers.
-    ExtPackManager *pExtPackMgr = aParent->i_getExtPackManager();
-    mpExtPackMgr = pExtPackMgr;
-    if (pExtPackMgr)
-    {
-        mcExtPackMgrUpdate = pExtPackMgr->i_getUpdateCounter();
-        // Make sure that the current value doesn't match, forcing a refresh.
-        mcExtPackMgrUpdate--;
-        i_refreshProviders();
-    }
-#else
-    RT_NOREF(aParent);
-#endif
-
     autoInitSpan.setSucceeded();
     return S_OK;
@@ -90 +73 @@
 
 #ifdef VBOX_WITH_EXTPACK
-void CloudProviderManager::i_refreshProviders()
-{
-    uint64_t cExtPackMgrUpdate;
-    {
-        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-        if (mpExtPackMgr.isNull())
-            return;
-        cExtPackMgrUpdate = mpExtPackMgr->i_getUpdateCounter();
-        if (cExtPackMgrUpdate == mcExtPackMgrUpdate)
-            return;
-    }
+bool CloudProviderManager::i_canRemoveExtPack(IExtPack *aExtPack)
+{
+    AssertReturn(aExtPack, false);
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-    // Reread the current value to figure out if some other thead did the work
-    // already before this thread got hold of the write lock.
-    cExtPackMgrUpdate = mpExtPackMgr->i_getUpdateCounter();
-    if (cExtPackMgrUpdate == mcExtPackMgrUpdate)
-        return;
-    mcExtPackMgrUpdate = cExtPackMgrUpdate;
-
-    if (!m_mapCloudProviderManagers.empty())
-    {
-        /// @todo The code below will need to be extended to handle extpack
-        // install and uninstall safely (and the latter needs non-trivial
-        // checks if any extpack related cloud activity is pending.
-        return;
-    }
-
-    std::vector<ComPtr<IUnknown> > apObjects;
+
+    // If any cloud provider in this extension pack fails to prepare the
+    // uninstall it and the cloud provider will be kept, so that the user
+    // can retry safely later. All other cloud providers in this extpack
+    // will be done as usual. No attempt is made to bring back the other
+    // cloud providers into working shape.
+
+    bool fRes = true;
+    Bstr bstrName;
+    aExtPack->GetName(bstrName.asOutParam());
+    Utf8Str strName(bstrName);
+    ExtPackNameCloudProviderManagerMap::iterator it = m_mapCloudProviderManagers.find(strName);
+    if (it != m_mapCloudProviderManagers.end())
+    {
+        ComPtr<ICloudProviderManager> pTmp(it->second);
+
+        Assert(m_astrExtPackNames.size() == m_apCloudProviders.size());
+        for (size_t i = 0; i < m_astrExtPackNames.size(); )
+        {
+            if (m_astrExtPackNames[i] != strName)
+            {
+                i++;
+                continue;
+            }
+
+            // pTmpProvider will point to an object with refcount > 0 until
+            // the ComPtr is removed from m_apCloudProviders.
+            HRESULT hrc = S_OK;
+            ULONG uRefCnt = 1;
+            ICloudProvider *pTmpProvider(m_apCloudProviders[i]);
+            if (pTmpProvider)
+            {
+                hrc = pTmpProvider->PrepareUninstall();
+                // Sanity check the refcount, it should be 1 at this point.
+                pTmpProvider->AddRef();
+                uRefCnt = pTmpProvider->Release();
+                Assert(uRefCnt == 1);
+            }
+            if (SUCCEEDED(hrc) && uRefCnt == 1)
+            {
+                m_astrExtPackNames.erase(m_astrExtPackNames.begin() + i);
+                m_apCloudProviders.erase(m_apCloudProviders.begin() + i);
+            }
+            else
+            {
+                LogRel(("CloudProviderManager: provider '%s' blocks extpack uninstall, result=%Rhrc, refcount=%u\n", strName.c_str(), hrc, uRefCnt));
+                fRes = false;
+                i++;
+            }
+        }
+
+        if (fRes)
+            m_mapCloudProviderManagers.erase(it);
+    }
+
+    return fRes;
+}
+
+void CloudProviderManager::i_addExtPack(IExtPack *aExtPack)
+{
+    AssertReturnVoid(aExtPack);
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    Bstr bstrName;
+    aExtPack->GetName(bstrName.asOutParam());
+    Utf8Str strName(bstrName);
+    ComPtr<IUnknown> pObj;
     std::vector<com::Utf8Str> astrExtPackNames;
     com::Guid idObj(COM_IIDOF(ICloudProviderManager));
-    mpExtPackMgr->i_queryObjects(idObj.toString(), apObjects, &astrExtPackNames);
-    for (unsigned i = 0; i < apObjects.size(); i++)
-    {
-        ComPtr<ICloudProviderManager> pTmp;
-        HRESULT hrc = apObjects[i].queryInterfaceTo(pTmp.asOutParam());
-        if (SUCCEEDED(hrc))
-            m_mapCloudProviderManagers[astrExtPackNames[i]] = pTmp;
-        SafeIfaceArray<ICloudProvider> apProvidersFromCurrExtPack;
-        hrc = pTmp->COMGETTER(Providers)(ComSafeArrayAsOutParam(apProvidersFromCurrExtPack));
-        if (SUCCEEDED(hrc))
-        {
-            for (unsigned j = 0; j < apProvidersFromCurrExtPack.size(); j++)
-                m_apCloudProviders.push_back(apProvidersFromCurrExtPack[i]);
-        }
+    HRESULT hrc = aExtPack->QueryObject(Bstr(idObj.toString()).raw(), pObj.asOutParam());
+    if (FAILED(hrc))
+        return;
+
+    ComPtr<ICloudProviderManager> pTmp(pObj);
+    if (pTmp.isNull())
+        return;
+
+    SafeIfaceArray<ICloudProvider> apProvidersFromCurrExtPack;
+    hrc = pTmp->COMGETTER(Providers)(ComSafeArrayAsOutParam(apProvidersFromCurrExtPack));
+    if (FAILED(hrc))
+        return;
+
+    m_mapCloudProviderManagers[strName] = pTmp;
+    for (unsigned i = 0; i < apProvidersFromCurrExtPack.size(); i++)
+    {
+        Assert(m_astrExtPackNames.size() == m_apCloudProviders.size());
+        m_astrExtPackNames.push_back(strName);
+        m_apCloudProviders.push_back(apProvidersFromCurrExtPack[i]);
     }
 }

trunk/src/VBox/Main/src-server/VirtualBoxImpl.cpp

@@ -555 +555 @@
         rc = unconst(m->pCloudProviderManager).createObject();
         if (SUCCEEDED(rc))
-            rc = m->pCloudProviderManager->init(this);
+            rc = m->pCloudProviderManager->init();
         ComAssertComRCThrowRC(rc);
         if (FAILED(rc)) throw rc;
@@ -3768 +3768 @@
 }
 
+CloudProviderManager *VirtualBox::i_getCloudProviderManager() const
+{
+    return m->pCloudProviderManager;
+}
+
 #ifdef VBOX_WITH_EXTPACK
 /**