Changeset 31308 in vbox for trunk

Timestamp:

Aug 2, 2010 2:55:22 PM (14 years ago)

Author:

vboxsync

Message:

Main: storage controller/attachment cleanup

Location:

trunk/src/VBox/Main

Files:

• MachineImpl.cpp (modified) (6 diffs)
• MediumImpl.cpp (modified) (2 diffs)
• StorageControllerImpl.cpp (modified) (5 diffs)
• VirtualBoxImpl.cpp (modified) (1 diff)
• include/MachineImpl.h (modified) (1 diff)
• include/StorageControllerImpl.h (modified) (1 diff)
• include/VirtualBoxImpl.h (modified) (1 diff)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3153 +3153 @@
     if (FAILED(rc)) return rc;
 
-    /* check that the port and device are not out of range. */
-    ULONG portCount;
-    ULONG devicesPerPort;
-    rc = ctl->COMGETTER(PortCount)(&portCount);
+    // check that the port and device are not out of range
+    rc = ctl->checkPortAndDeviceValid(aControllerPort, aDevice);
     if (FAILED(rc)) return rc;
-    rc = ctl->COMGETTER(MaxDevicesPerPortCount)(&devicesPerPort);
-    if (FAILED(rc)) return rc;
-
-    if (   (aControllerPort < 0)
-        || (aControllerPort >= (LONG)portCount)
-        || (aDevice < 0)
-        || (aDevice >= (LONG)devicesPerPort)
-       )
-        return setError(E_INVALIDARG,
-                        tr("The port and/or count parameter are out of range [%lu:%lu]"),
-                        portCount,
-                        devicesPerPort);
 
     /* check if the device slot is already busy */
@@ -3230 +3216 @@
                         medium->getLocationFull().raw());
 
-    bool indirect = false;
+    bool fImplicit = false;
     if (!medium.isNull())
-        indirect = medium->isReadOnly();
+        fImplicit = medium->isReadOnly();
     bool associate = true;
 
@@ -3246 +3232 @@
             if ((pAttachTemp = findAttachment(oldAtts, medium)))
             {
-                AssertReturn(!indirect, E_FAIL);
+                AssertReturn(!fImplicit, E_FAIL);
 
                 /* see if it's the same bus/channel/device */
@@ -3265 +3251 @@
 
         /* go further only if the attachment is to be indirect */
-        if (!indirect)
+        if (!fImplicit)
             break;
 
@@ -3333 +3319 @@
                     mediumLock.attach(medium);
                     /* not implicit, doesn't require association with this VM */
-                    indirect = false;
+                    fImplicit = false;
                     associate = false;
                     /* go right to the MediumAttachment creation */
@@ -3477 +3463 @@
     ComObjPtr<MediumAttachment> attachment;
     attachment.createObject();
-    rc = attachment->init(this, medium, aControllerName, aControllerPort, aDevice, aType, indirect, 0 /* No bandwidth limit */);
+    rc = attachment->init(this,
+                          medium,
+                          aControllerName,
+                          aControllerPort,
+                          aDevice,
+                          aType,
+                          fImplicit,
+                          0 /* No bandwidth limit */);
     if (FAILED(rc)) return rc;
 

trunk/src/VBox/Main/MediumImpl.cpp

@@ -3236 +3236 @@
     {
         /* use the default format if not */
-        AutoReadLock propsLock(m->pVirtualBox->systemProperties() COMMA_LOCKVAL_SRC_POS);
+        AutoReadLock propsLock(m->pVirtualBox->getSystemProperties() COMMA_LOCKVAL_SRC_POS);
         strFormat = m->pVirtualBox->getDefaultHardDiskFormat();
     }
@@ -4843 +4843 @@
     /* get the format object first */
     {
-        AutoReadLock propsLock(m->pVirtualBox->systemProperties() COMMA_LOCKVAL_SRC_POS);
-
-        unconst(m->formatObj)
-            = m->pVirtualBox->systemProperties()->mediumFormat(aFormat);
+        SystemProperties *pSysProps = m->pVirtualBox->getSystemProperties();
+        AutoReadLock propsLock(pSysProps COMMA_LOCKVAL_SRC_POS);
+
+        unconst(m->formatObj) = pSysProps->mediumFormat(aFormat);
         if (m->formatObj.isNull())
             return setError(E_INVALIDARG,

trunk/src/VBox/Main/StorageControllerImpl.cpp

@@ -21 +21 @@
 #include "MachineImpl.h"
 #include "VirtualBoxImpl.h"
+#include "SystemPropertiesImpl.h"
 
 #include <iprt/string.h>
@@ -79 +80 @@
 {
     Data()
-        : pParent(NULL)
+        : pVirtualBox(NULL),
+          pSystemProperties(NULL),
+          pParent(NULL)
     { }
+
+    VirtualBox * const                  pVirtualBox;
+    SystemProperties * const            pSystemProperties;
 
     Machine * const                     pParent;
@@ -131 +137 @@
 
     m = new Data();
+
+    unconst(m->pVirtualBox) = aParent->getVirtualBox();
+    unconst(m->pSystemProperties) = m->pVirtualBox->getSystemProperties();
 
     unconst(m->pParent) = aParent;
@@ -412 +421 @@
 
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    ComPtr<IVirtualBox> VBox;
-    HRESULT rc = m->pParent->COMGETTER(Parent)(VBox.asOutParam());
-    if (FAILED(rc))
-        return rc;
-
-    ComPtr<ISystemProperties> sysProps;
-    rc = VBox->COMGETTER(SystemProperties)(sysProps.asOutParam());
-    if (FAILED(rc))
-        return rc;
-
-    rc = sysProps->GetMaxDevicesPerPortForStorageBus(m->bd->mStorageBus, aMaxDevices);
+    HRESULT rc = m->pSystemProperties->GetMaxDevicesPerPortForStorageBus(m->bd->mStorageBus, aMaxDevices);
+
     return rc;
 }
@@ -751 +750 @@
 }
 
+/**
+ * Returns S_OK if the given port and device numbers are within the range supported
+ * by this controller. If not, it sets an error and returns E_INVALIDARG.
+ * @param ulPort
+ * @param ulDevice
+ * @return
+ */
+HRESULT StorageController::checkPortAndDeviceValid(LONG aControllerPort,
+                                                   LONG aDevice)
+{
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    ULONG portCount = m->bd->mPortCount;
+    ULONG devicesPerPort;
+    HRESULT rc = m->pSystemProperties->GetMaxDevicesPerPortForStorageBus(m->bd->mStorageBus, &devicesPerPort);
+    if (FAILED(rc)) return rc;
+
+    if (   (aControllerPort < 0)
+        || (aControllerPort >= (LONG)portCount)
+        || (aDevice < 0)
+        || (aDevice >= (LONG)devicesPerPort)
+       )
+        return setError(E_INVALIDARG,
+                        tr("The port and/or count parameter are out of range [%lu:%lu]"),
+                        portCount,
+                        devicesPerPort);
+
+    return S_OK;
+}
+
 /** @note Locks objects for writing! */
 void StorageController::rollback()

trunk/src/VBox/Main/VirtualBoxImpl.cpp

@@ -2939 +2939 @@
 }
 
-const ComObjPtr<SystemProperties>& VirtualBox::systemProperties() const
+SystemProperties* VirtualBox::getSystemProperties() const
 {
     return m->pSystemProperties;

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

@@ -821 +821 @@
     Machine* const          mPeer;
 
-    VirtualBox* const       mParent;
+    VirtualBox * const      mParent;
 
     Shareable<Data>         mData;

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

@@ -85 +85 @@
     ULONG getInstance() const;
 
+    HRESULT checkPortAndDeviceValid(LONG aControllerPort,
+                                    LONG aDevice);
+
     void rollback();
     void commit();

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

@@ -237 +237 @@
 
     const ComObjPtr<Host>& host() const;
-    const ComObjPtr<SystemProperties>& systemProperties() const;
+    SystemProperties* getSystemProperties() const;
 #ifdef VBOX_WITH_RESOURCE_USAGE_API
     const ComObjPtr<PerformanceCollector>& performanceCollector() const;