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

Timestamp:

Feb 24, 2015 11:09:17 AM (10 years ago)

Author:

vboxsync

Message:

Main/VirtualBox: eliminate redundant parameter to i_registerMedium, avoiding bugs with registering e.g. floppy images as hard disks.
Main/Medium: switch over to CreateMedium, final API touch-ups, variable name cleanup, typo checking for backend support of fixed image creation, check for backends not supporting a particular device type
Frontends/VBoxManage: cleanup, adapt to new API, generalize all operations which worked on hard disks only to all device types
Frontends/VirtualBox: adapt to new API

Location:

trunk/src/VBox/Main

Files:

• idl/VirtualBox.xidl (modified) (8 diffs)
• include/VirtualBoxImpl.h (modified) (3 diffs)
• src-server/ApplianceImplImport.cpp (modified) (2 diffs)
• src-server/MachineImplCloneVM.cpp (modified) (2 diffs)
• src-server/MediumImpl.cpp (modified) (18 diffs)
• src-server/SnapshotImpl.cpp (modified) (2 diffs)
• src-server/VirtualBoxImpl.cpp (modified) (11 diffs)
• testcase/tstMediumLock.cpp (modified) (2 diffs)
• testcase/tstVBoxAPIXPCOM.cpp (modified) (2 diffs)

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

@@ -1820 +1820 @@
   <interface
     name="IVirtualBox" extends="$unknown"
-    uuid="5e67bc8a-5f8f-11e4-aa15-123b93f75cba"
+    uuid="3afa096e-2cad-4cb0-aba2-47fe3fc62bd8"
     wsmap="managed"
     >
@@ -2359 +2359 @@
         a "DVD" device type, this is ignored and read-only mode is always assumed.</desc>
       </param>
-      <param name="forceNewUuid" type="boolean" dir="in">
-         <desc>Allows the caller to request a completely new medium UUID for
-           the image which is to be opened. Useful if one intends to open an exact
-           copy of a previously opened image, as this would normally fail due to
-           the duplicate UUID.</desc>
-      </param>
       <param name="aDeviceTypeType" type="DeviceType" dir="in">
         <desc>
           Must be one of "HardDisk", "DVD" or "Floppy".
-        </desc>
-      </param>
-      <param name="medium" type="IMedium" dir="return">
-        <desc>Created medium object.</desc>
-      </param>
-    </method>
-
-    <method name="createHardDisk">
-      <desc>
-        Creates a new base medium object that will use the given storage
-        format and location for medium data.
-
-        The actual storage unit is not created by this method. In order to
-        do it, and before you are able to attach the created medium to
-        virtual machines, you must call one of the following methods to
-        allocate a format-specific storage unit at the specified location:
-        <ul>
-          <li><link to="IMedium::createBaseStorage"/></li>
-          <li><link to="IMedium::createDiffStorage"/></li>
-        </ul>
-
-        Some medium attributes, such as <link to="IMedium::id"/>, may
-        remain uninitialized until the medium storage unit is successfully
-        created by one of the above methods.
-
-        After the storage unit is successfully created, it will be
-        accessible through the <link to="#openMedium"/> method and can
-        be found in the <link to="#hardDisks"/> array.
-
-        The list of all storage formats supported by this VirtualBox
-        installation can be obtained using
-        <link to="ISystemProperties::mediumFormats"/>. If the @a format
-        attribute is empty or @c null then the default storage format
-        specified by <link to="ISystemProperties::defaultHardDiskFormat"/> will
-        be used for creating a storage unit of the medium.
-
-        Note that the format of the location string is storage format specific.
-        See <link to="IMedium::location"/> and IMedium for more details.
-
-        <result name="VBOX_E_OBJECT_NOT_FOUND">
-          @a format identifier is invalid. See
-          <link to="ISystemProperties::mediumFormats"/>.
-        </result>
-        <result name="VBOX_E_FILE_ERROR">
-          @a location is a not valid file name (for file-based formats only).
-        </result>
-      </desc>
-      <param name="format" type="wstring" dir="in">
-        <desc>
-          Identifier of the storage format to use for the new medium.
-        </desc>
-      </param>
-      <param name="location" type="wstring" dir="in">
-        <desc>
-          Location of the storage unit for the new medium.
         </desc>
       </param>
@@ -9202 +9141 @@
         Keep in mind that the medium format identifier
         (<link to="IMediumFormat::id"/>) used in other API calls like
-        <link to="IVirtualBox::createHardDisk"/> to refer to a particular
+        <link to="IVirtualBox::createMedium"/> to refer to a particular
         medium format is a case-insensitive string. This means that, for
         example, all of the following strings:
@@ -9225 +9164 @@
         The medium format set by this attribute is used by VirtualBox
         when the medium format was not specified explicitly. One example is
-        <link to="IVirtualBox::createHardDisk"/> with the empty
+        <link to="IVirtualBox::createMedium"/> with the empty
         format argument. A more complex example is implicit creation of
         differencing media when taking a snapshot of a virtual machine:
@@ -9249 +9188 @@
           <link to="#mediumFormats"/>,
           <link to="IMediumFormat::id"/>,
-          <link to="IVirtualBox::createHardDisk"/>
+          <link to="IVirtualBox::createMedium"/>
         </see>
       </desc>
@@ -13518 +13457 @@
       Existing media are opened using <link to="IVirtualBox::openMedium"/>;
       new hard disk media can be created with the VirtualBox API using the
-      <link to="IVirtualBox::createHardDisk"/> method. Differencing hard
+      <link to="IVirtualBox::createMedium"/> method. Differencing hard
       disks (see below) are usually implicitly created by VirtualBox as
       needed, but may also be created explicitly using <link to="#createDiffStorage"/>.
@@ -14846 +14785 @@
         Each medium format is identified by a string represented by the
         <link to="#id"/> attribute. This string is used in calls like
-        <link to="IVirtualBox::createHardDisk"/> to specify the desired
+        <link to="IVirtualBox::createMedium"/> to specify the desired
         format.
 
@@ -14870 +14809 @@
         This string is used in methods of other interfaces where it is necessary
         to specify a medium format, such as
-        <link to="IVirtualBox::createHardDisk"/>.
+        <link to="IVirtualBox::createMedium"/>.
       </desc>
     </attribute>

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

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2006-2014 Oracle Corporation
+ * Copyright (C) 2006-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -217 +217 @@
     int i_calculateFullPath(const Utf8Str &strPath, Utf8Str &aResult);
     void i_copyPathRelativeToConfig(const Utf8Str &strSource, Utf8Str &strTarget);
-    HRESULT i_registerMedium(const ComObjPtr<Medium> &pMedium, ComObjPtr<Medium> *ppMedium, DeviceType_T argType,
+    HRESULT i_registerMedium(const ComObjPtr<Medium> &pMedium, ComObjPtr<Medium> *ppMedium,
                              AutoWriteLock &mediaTreeLock);
     HRESULT i_unregisterMedium(Medium *pMedium);
@@ -295 +295 @@
                              std::vector<MachineState_T> &aStates);
     HRESULT createAppliance(ComPtr<IAppliance> &aAppliance);
-    HRESULT createHardDisk(const com::Utf8Str &aFormat,
-                           const com::Utf8Str &aLocation,
-                           ComPtr<IMedium> &aMedium);
     HRESULT createMedium(const com::Utf8Str &aFormat,
                          const com::Utf8Str &aLocation,
                          AccessMode_T aAccessMode,
-                         BOOL aForceNewUuid,
                          DeviceType_T aDeviceType,
                          ComPtr<IMedium> &aMedium);

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

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2008-2014 Oracle Corporation
+ * Copyright (C) 2008-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -2521 +2521 @@
 
                 /* Now create an empty hard disk. */
-                rc = mVirtualBox->CreateHardDisk(Bstr(strTrgFormat).raw(),
-                                                 Bstr(*strTargetPath).raw(),
-                                                 ComPtr<IMedium>(pTargetHD).asOutParam());
+                rc = mVirtualBox->CreateMedium(Bstr(strTrgFormat).raw(),
+                                               Bstr(*strTargetPath).raw(),
+                                               AccessMode_ReadWrite, DeviceType_HardDisk,
+                                               ComPtr<IMedium>(pTargetHD).asOutParam());
                 if (FAILED(rc)) throw rc;
 

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

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2011-2014 Oracle Corporation
+ * Copyright (C) 2011-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -1252 +1252 @@
                             AutoWriteLock tlock(p->mParent->i_getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
                             rc = p->mParent->i_registerMedium(pTarget, &pTarget,
-                                                            DeviceType_HardDisk,
                                                             tlock);
                             if (FAILED(rc)) throw rc;

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

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2008-2014 Oracle Corporation
+ * Copyright (C) 2008-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -883 +883 @@
  * storage unit.
  *
- * This gets called by VirtualBox::CreateHardDisk() in which case uuidMachineRegistry
+ * This gets called by VirtualBox::CreateMedium() in which case uuidMachineRegistry
  * is empty since starting with VirtualBox 4.0, we no longer add opened media to a
  * registry automatically (this is deferred until the medium is attached to a machine).
@@ -975 +975 @@
         }
 
-        rc = m->pVirtualBox->i_registerMedium(this, &pMedium, aDeviceType, treeLock);
+        rc = m->pVirtualBox->i_registerMedium(this, &pMedium, treeLock);
         Assert(this == pMedium || FAILED(rc));
     }
@@ -1280 +1280 @@
         const settings::Medium &med = *it;
 
-        ComObjPtr<Medium> pHD;
-        pHD.createObject();
-        rc = pHD->init(aVirtualBox,
-                       this,            // parent
-                       aDeviceType,
-                       uuidMachineRegistry,
-                       med,               // child data
-                       strMachineFolder);
+        ComObjPtr<Medium> pMedium;
+        pMedium.createObject();
+        rc = pMedium->init(aVirtualBox,
+                           this,            // parent
+                           aDeviceType,
+                           uuidMachineRegistry,
+                           med,               // child data
+                           strMachineFolder);
         if (FAILED(rc)) break;
 
         AutoWriteLock treeLock(aVirtualBox->i_getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
 
-        rc = m->pVirtualBox->i_registerMedium(pHD, &pHD, DeviceType_HardDisk, treeLock);
+        rc = m->pVirtualBox->i_registerMedium(pMedium, &pMedium, treeLock);
         if (FAILED(rc)) break;
     }
@@ -2380 +2380 @@
 
         if (    (mediumVariantFlags & MediumVariant_Fixed)
-            &&  !(m->formatObj->i_getCapabilities() & MediumFormatCapabilities_CreateDynamic))
+            &&  !(m->formatObj->i_getCapabilities() & MediumFormatCapabilities_CreateFixed))
             throw setError(VBOX_E_NOT_SUPPORTED,
                            tr("Medium format '%s' does not support fixed storage creation"),
@@ -5942 +5942 @@
 {
     /* Note that we need to de-associate ourselves from the parent to let
-     * unregisterMedium() properly save the registry */
+     * VirtualBox::i_unregisterMedium() properly save the registry */
 
     /* we modify mParent and access children */
@@ -6695 +6695 @@
         {
             id.create();
-            /* VirtualBox::registerMedium() will need UUID */
+            /* VirtualBox::i_registerMedium() will need UUID */
             unconst(m->id) = id;
         }
@@ -6738 +6738 @@
                                task.mVDOperationIfaces);
             if (RT_FAILURE(vrc))
-                throw setError(VBOX_E_FILE_ERROR,
-                               tr("Could not create the medium storage unit '%s'%s"),
-                               location.c_str(), i_vdError(vrc).c_str());
+            {
+                if (vrc == VERR_VD_INVALID_TYPE)
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Parameters for creating the medium storage unit '%s' are invalid%s"),
+                                   location.c_str(), i_vdError(vrc).c_str());
+                else
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Could not create the medium storage unit '%s'%s"),
+                                   location.c_str(), i_vdError(vrc).c_str());
+            }
 
             size = VDGetFileSize(hdd, 0);
@@ -6762 +6769 @@
         AutoWriteLock treeLock(m->pVirtualBox->i_getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
         ComObjPtr<Medium> pMedium;
-        rc = m->pVirtualBox->i_registerMedium(this, &pMedium, DeviceType_HardDisk, treeLock);
+        rc = m->pVirtualBox->i_registerMedium(this, &pMedium, treeLock);
         Assert(this == pMedium);
     }
@@ -6833 +6840 @@
         {
             targetId.create();
-            /* VirtualBox::registerMedium() will need UUID */
+            /* VirtualBox::i_registerMedium() will need UUID */
             unconst(pTarget->m->id) = targetId;
         }
@@ -6911 +6918 @@
                                task.mVDOperationIfaces);
             if (RT_FAILURE(vrc))
-                throw setError(VBOX_E_FILE_ERROR,
-                                tr("Could not create the differencing medium storage unit '%s'%s"),
-                                targetLocation.c_str(), i_vdError(vrc).c_str());
+            {
+                if (vrc == VERR_VD_INVALID_TYPE)
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Parameters for creating the differencing medium storage unit '%s' are invalid%s"),
+                                   targetLocation.c_str(), i_vdError(vrc).c_str());
+                else
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Could not create the differencing medium storage unit '%s'%s"),
+                                   targetLocation.c_str(), i_vdError(vrc).c_str());
+            }
 
             size = VDGetFileSize(hdd, VD_LAST_IMAGE);
@@ -6949 +6963 @@
          * better than breaking media registry consistency) */
         ComObjPtr<Medium> pMedium;
-        mrc = m->pVirtualBox->i_registerMedium(pTarget, &pMedium, DeviceType_HardDisk, treeLock);
+        mrc = m->pVirtualBox->i_registerMedium(pTarget, &pMedium, treeLock);
         Assert(pTarget == pMedium);
 
@@ -7184 +7198 @@
             ComObjPtr<Medium> pMedium;
             rc2 = m->pVirtualBox->i_registerMedium(pTarget, &pMedium,
-                                                   DeviceType_HardDisk,
                                                    treeLock);
             AssertComRC(rc2);
@@ -7534 +7547 @@
             ComObjPtr<Medium> pMedium;
             mrc = pParent->m->pVirtualBox->i_registerMedium(pTarget, &pMedium,
-                                                            DeviceType_HardDisk,
                                                             treeLock);
             Assert(   FAILED(mrc)
@@ -7550 +7562 @@
             ComObjPtr<Medium> pMedium;
             mrc = m->pVirtualBox->i_registerMedium(pTarget, &pMedium,
-                                                   DeviceType_HardDisk,
                                                    treeLock);
             Assert(   FAILED(mrc)
@@ -7781 +7792 @@
                                task.mVDOperationIfaces);
             if (RT_FAILURE(vrc))
-                throw setError(VBOX_E_FILE_ERROR,
-                               tr("Could not create the differencing medium storage unit '%s'%s"),
-                               location.c_str(), i_vdError(vrc).c_str());
+            {
+                if (vrc == VERR_VD_INVALID_TYPE)
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Parameters for creating the differencing medium storage unit '%s' are invalid%s"),
+                                   location.c_str(), i_vdError(vrc).c_str());
+                else
+                    throw setError(VBOX_E_FILE_ERROR,
+                                   tr("Could not create the differencing medium storage unit '%s'%s"),
+                                   location.c_str(), i_vdError(vrc).c_str());
+            }
 
             size = VDGetFileSize(hdd, VD_LAST_IMAGE);
@@ -8324 +8342 @@
             ComObjPtr<Medium> pMedium;
             mrc = pParent->m->pVirtualBox->i_registerMedium(this, &pMedium,
-                                                            DeviceType_HardDisk,
                                                             treeLock);
             Assert(this == pMedium);
@@ -8338 +8355 @@
             eik.restore();
             ComObjPtr<Medium> pMedium;
-            mrc = m->pVirtualBox->i_registerMedium(this, &pMedium, DeviceType_HardDisk, treeLock);
+            mrc = m->pVirtualBox->i_registerMedium(this, &pMedium, treeLock);
             Assert(this == pMedium);
             eik.fetch();

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

@@ -6 +6 @@
 
 /*
- * Copyright (C) 2006-2014 Oracle Corporation
+ * Copyright (C) 2006-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -3445 +3445 @@
 
         // then, register again
-        rc = mParent->i_registerMedium(pDeleteRec->mpTarget, &pDeleteRec->mpTarget, DeviceType_HardDisk, treeLock);
+        rc = mParent->i_registerMedium(pDeleteRec->mpTarget, &pDeleteRec->mpTarget, treeLock);
         AssertComRC(rc);
     }

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

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2006-2014 Oracle Corporation
+ * Copyright (C) 2006-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -677 +677 @@
         if (FAILED(rc)) return rc;
 
-        rc = i_registerMedium(pHardDisk, &pHardDisk, DeviceType_HardDisk, treeLock);
+        rc = i_registerMedium(pHardDisk, &pHardDisk, treeLock);
         if (FAILED(rc)) return rc;
     }
@@ -697 +697 @@
         if (FAILED(rc)) return rc;
 
-        rc = i_registerMedium(pImage, &pImage, DeviceType_DVD, treeLock);
+        rc = i_registerMedium(pImage, &pImage, treeLock);
         if (FAILED(rc)) return rc;
     }
@@ -717 +717 @@
         if (FAILED(rc)) return rc;
 
-        rc = i_registerMedium(pImage, &pImage, DeviceType_Floppy, treeLock);
+        rc = i_registerMedium(pImage, &pImage, treeLock);
         if (FAILED(rc)) return rc;
     }
@@ -1735 +1735 @@
 }
 
-HRESULT VirtualBox::createHardDisk(const com::Utf8Str &aFormat,
-                                   const com::Utf8Str &aLocation,
-                                   ComPtr<IMedium> &aMedium)
-{
-    /* we don't access non-const data members so no need to lock */
-    HRESULT rc = createMedium(aFormat, aLocation, AccessMode_ReadWrite, TRUE, DeviceType_HardDisk, aMedium);
-
-    return rc;
-}
-
 HRESULT VirtualBox::createMedium(const com::Utf8Str &aFormat,
                                  const com::Utf8Str &aLocation,
                                  AccessMode_T aAccessMode,
-                                 BOOL aForceNewUuid,
                                  DeviceType_T aDeviceType,
                                  ComPtr<IMedium> &aMedium)
 {
-    NOREF(aForceNewUuid);
-    NOREF(aAccessMode);
+    NOREF(aAccessMode); /**< @todo r=klaus make use of access mode */
 
     HRESULT rc = S_OK;
@@ -1867 +1855 @@
         if (SUCCEEDED(rc))
         {
-            rc = i_registerMedium(pMedium, &pMedium, aDeviceType, treeLock);
+            rc = i_registerMedium(pMedium, &pMedium, treeLock);
 
             treeLock.release();
@@ -4206 +4194 @@
  *                  to an unavoidable race there was a duplicate Medium object
  *                  created.
- * @param argType   Either DeviceType_HardDisk, DeviceType_DVD or DeviceType_Floppy.
  * @param mediaTreeLock Reference to the AutoWriteLock holding the media tree
  *                  lock, necessary to release it in the right spot.
@@ -4213 +4200 @@
 HRESULT VirtualBox::i_registerMedium(const ComObjPtr<Medium> &pMedium,
                                      ComObjPtr<Medium> *ppMedium,
-                                     DeviceType_T argType,
                                      AutoWriteLock &mediaTreeLock)
 {
@@ -4230 +4216 @@
     const char *pszDevType = NULL;
     ObjectsList<Medium> *pall = NULL;
-    switch (argType)
+    DeviceType_T devType;
+    {
+        AutoReadLock mediumLock(pMedium COMMA_LOCKVAL_SRC_POS);
+        devType = pMedium->i_getDeviceType();
+    }
+    switch (devType)
     {
         case DeviceType_HardDisk:
@@ -4245 +4236 @@
             break;
         default:
-            AssertMsgFailedReturn(("invalid device type %d", argType), E_INVALIDARG);
+            AssertMsgFailedReturn(("invalid device type %d", devType), E_INVALIDARG);
     }
 
@@ -4284 +4275 @@
 
         // store all hard disks (even differencing images) in the map
-        if (argType == DeviceType_HardDisk)
+        if (devType == DeviceType_HardDisk)
             m->mapHardDisks[id] = pMedium;
 

trunk/src/VBox/Main/testcase/tstMediumLock.cpp

@@ -7 +7 @@
 
 /*
- * Copyright (C) 2013 Oracle Corporation
+ * Copyright (C) 2013-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -108 +108 @@
     {
         RTTestSub(hTest, "Creating temp hard disk medium");
-        TEST_COM_SUCCESS(hTest, pVirtualBox->CreateHardDisk(Bstr("VDI").raw(), Bstr(szPathTemp).raw(), pMedium.asOutParam()), "create medium");
+        TEST_COM_SUCCESS(hTest, pVirtualBox->CreateMedium(Bstr("VDI").raw(), Bstr(szPathTemp).raw(), AccessMode_ReadWrite, DeviceType_HardDisk, pMedium.asOutParam()), "create medium");
         if (!pMedium.isNull())
         {

trunk/src/VBox/Main/testcase/tstVBoxAPIXPCOM.cpp

@@ -8 +8 @@
 
 /*
- * Copyright (C) 2006-2014 Oracle Corporation
+ * Copyright (C) 2006-2015 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -291 +291 @@
      */
     nsCOMPtr<IMedium> hardDisk = 0;
-    rc = virtualBox->CreateHardDisk(NS_LITERAL_STRING("VDI").get(),
-                                    NS_LITERAL_STRING("/tmp/TestHardDisk.vdi").get(),
-                                    getter_AddRefs(hardDisk));
+    rc = virtualBox->CreateMedium(NS_LITERAL_STRING("VDI").get(),
+                                  NS_LITERAL_STRING("/tmp/TestHardDisk.vdi").get(),
+                                  AccessMode_ReadWrite, DeviceType_HardDisk,
+                                  getter_AddRefs(hardDisk));
     if (NS_FAILED(rc))
     {