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

Timestamp:

Jun 7, 2018 1:49:48 PM (7 years ago)

Author:

vboxsync

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

122961

Message:

Main/Appliance: Teach importing new tricks: importing to specific location (by settings file name or base folder) and also importing straight into a group. Lots of cleanup and minor fixing (bad code quality due to lots of copy/paste, and what's worse is that the original code was broken already, using the variables inconsistently), plus some smallish coding style cleaup. Much more needed. Also fixed the incomplete use of the VM name on expert (the one in the VBox XML was not changed, and it's the preferred name on import).
VBoxManage: small updates to reflect the new features (and actually offer setting the VM name on export, which is something the GUI could do for a long time).

Location:

trunk/src/VBox/Main/src-server

Files:

• ApplianceImpl.cpp (modified) (1 diff)
• ApplianceImplExport.cpp (modified) (1 diff)
• ApplianceImplImport.cpp (modified) (21 diffs)
• MachineImpl.cpp (modified) (3 diffs)

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

@@ -954 +954 @@
 }
 
-HRESULT Appliance::i_searchUniqueDiskImageFilePath(Utf8Str& aName) const
+HRESULT Appliance::i_searchUniqueDiskImageFilePath(const Utf8Str &aMachineFolder, Utf8Str &aName) const
 {
     IMedium *harddisk = NULL;
     char *tmpName = RTStrDup(aName.c_str());
+    char *tmpAbsName = RTPathAbsExDup(aMachineFolder.c_str(), tmpName);
     int i = 1;
     /* Check if the file exists or if a file with this path is registered
      * already */
     /** @todo Maybe too cost-intensive; try to find a lighter way */
-    while (    RTPathExists(tmpName)
-            || mVirtualBox->OpenMedium(Bstr(tmpName).raw(), DeviceType_HardDisk, AccessMode_ReadWrite,
+    while (    RTPathExists(tmpAbsName)
+            || mVirtualBox->OpenMedium(Bstr(tmpAbsName).raw(), DeviceType_HardDisk, AccessMode_ReadWrite,
                                        FALSE /* fForceNewUuid */,  &harddisk) != VBOX_E_OBJECT_NOT_FOUND)
     {
-        RTStrFree(tmpName);
+        RTStrFree(tmpAbsName);
         char *tmpDir = RTStrDup(aName.c_str());
-        RTPathStripFilename(tmpDir);;
+        RTPathStripFilename(tmpDir);
         char *tmpFile = RTStrDup(RTPathFilename(aName.c_str()));
         RTPathStripSuffix(tmpFile);
         const char *pszTmpSuff = RTPathSuffix(aName.c_str());
         RTStrAPrintf(&tmpName, "%s%c%s_%d%s", tmpDir, RTPATH_DELIMITER, tmpFile, i, pszTmpSuff);
+        tmpAbsName = RTPathAbsExDup(aMachineFolder.c_str(), tmpName);
         RTStrFree(tmpFile);
         RTStrFree(tmpDir);

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

@@ -1918 +1918 @@
         // fill the machine config
         vsdescThis->m->pMachine->i_copyMachineDataToSettings(*pConfig);
+        pConfig->machineUserData.strName = strVMName;
 
         // Apply export tweaks to machine settings

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

@@ -200 +200 @@
 
             /* Based on the VM name, create a target machine path. */
-            Bstr bstrMachineFilename;
+            Bstr bstrSettingsFilename;
             rc = mVirtualBox->ComposeMachineFilename(Bstr(nameVBox).raw(),
                                                      NULL /* aGroup */,
                                                      NULL /* aCreateFlags */,
                                                      NULL /* aBaseFolder */,
-                                                     bstrMachineFilename.asOutParam());
+                                                     bstrSettingsFilename.asOutParam());
             if (FAILED(rc)) throw rc;
-            /* Determine the machine folder from that */
-            Utf8Str strMachineFolder = Utf8Str(bstrMachineFilename).stripFilename();
+            Utf8Str strMachineFolder(bstrSettingsFilename);
+            strMachineFolder.stripFilename();
+
+#if 1
+            /* The import logic should work exactly the same whether the
+             * following 3 items are present or not, but of course it may have
+             * an influence on the exact presentation of the import settings
+             * of an API client. */
+            Utf8Str strSettingsFilename(bstrSettingsFilename);
+            pNewDesc->i_addEntry(VirtualSystemDescriptionType_SettingsFile,
+                                 "",
+                                 "" /* no direct OVF correspondence */,
+                                 strSettingsFilename);
+            Utf8Str strBaseFolder;
+            mVirtualBox->i_getDefaultMachineFolder(strBaseFolder);
+            pNewDesc->i_addEntry(VirtualSystemDescriptionType_BaseFolder,
+                                 "",
+                                 "" /* no direct OVF correspondence */,
+                                 strBaseFolder);
+            pNewDesc->i_addEntry(VirtualSystemDescriptionType_PrimaryGroup,
+                                 "",
+                                 "" /* no direct OVF correspondence */,
+                                 "");
+#endif
 
             /* VM Product */
@@ -643 +665 @@
                     //  - we need a url specifier for the vdi format
 
+                    Utf8Str strFilename = di.strHref;
                     if (vdf.compare("VMDK", Utf8Str::CaseInsensitive) == 0)
                     {
                         /* If the href is empty use the VM name as filename */
-                        Utf8Str strFilename = di.strHref;
                         if (!strFilename.length())
                             strFilename = Utf8StrFmt("%s.vmdk", hd.strDiskId.c_str());
-
-                        Utf8Str strTargetPath = Utf8Str(strMachineFolder);
-                        strTargetPath.append(RTPATH_DELIMITER).append(di.strHref);
-                        /*
-                         * Remove last extension from the file name if the file is compressed
-                        */
-                        if (di.strCompression.compare("gzip", Utf8Str::CaseInsensitive)==0)
-                        {
-                            strTargetPath.stripSuffix();
-                        }
-
-                        i_searchUniqueDiskImageFilePath(strTargetPath);
-
-                        /* find the description for the hard disk controller
-                         * that has the same ID as hd.idController */
-                        const VirtualSystemDescriptionEntry *pController;
-                        if (!(pController = pNewDesc->i_findControllerFromID(hd.idController)))
-                            throw setError(E_FAIL,
-                                           tr("Cannot find hard disk controller with OVF instance ID %RI32 "
-                                              "to which disk \"%s\" should be attached"),
-                                           hd.idController,
-                                           di.strHref.c_str());
-
-                        /* controller to attach to, and the bus within that controller */
-                        Utf8StrFmt strExtraConfig("controller=%RI16;channel=%RI16",
-                                                  pController->ulIndex,
-                                                  hd.ulAddressOnParent);
-                        pNewDesc->i_addEntry(VirtualSystemDescriptionType_HardDiskImage,
-                                             hd.strDiskId,
-                                             di.strHref,
-                                             strTargetPath,
-                                             di.ulSuggestedSizeMB,
-                                             strExtraConfig);
                     }
                     else if (vdf.compare("RAW", Utf8Str::CaseInsensitive) == 0)
                     {
                         /* If the href is empty use the VM name as filename */
-                        Utf8Str strFilename = di.strHref;
                         if (!strFilename.length())
                             strFilename = Utf8StrFmt("%s.iso", hd.strDiskId.c_str());
-
-                        Utf8Str strTargetPath = Utf8Str(strMachineFolder)
-                            .append(RTPATH_DELIMITER)
-                            .append(di.strHref);
-                        /*
-                         * Remove last extension from the file name if the file is compressed
-                        */
-                        if (di.strCompression.compare("gzip", Utf8Str::CaseInsensitive)==0)
-                        {
-                            strTargetPath.stripSuffix();
-                        }
-
-                        i_searchUniqueDiskImageFilePath(strTargetPath);
-
-                        /* find the description for the hard disk controller
-                         * that has the same ID as hd.idController */
-                        const VirtualSystemDescriptionEntry *pController;
-                        if (!(pController = pNewDesc->i_findControllerFromID(hd.idController)))
-                            throw setError(E_FAIL,
-                                           tr("Cannot find disk controller with OVF instance ID %RI32 "
-                                              "to which disk \"%s\" should be attached"),
-                                           hd.idController,
-                                           di.strHref.c_str());
-
-                        /* controller to attach to, and the bus within that controller */
-                        Utf8StrFmt strExtraConfig("controller=%RI16;channel=%RI16",
-                                                  pController->ulIndex,
-                                                  hd.ulAddressOnParent);
-                        pNewDesc->i_addEntry(VirtualSystemDescriptionType_HardDiskImage,
-                                             hd.strDiskId,
-                                             di.strHref,
-                                             strTargetPath,
-                                             di.ulSuggestedSizeMB,
-                                             strExtraConfig);
                     }
                     else
@@ -729 +683 @@
                                           di.strHref.c_str(),
                                           di.strFormat.c_str());
+
+                    /*
+                     * Remove last extension from the file name if the file is compressed
+                     */
+                    if (di.strCompression.compare("gzip", Utf8Str::CaseInsensitive)==0)
+                        strFilename.stripSuffix();
+
+                    i_searchUniqueDiskImageFilePath(strMachineFolder, strFilename);
+
+                    /* find the description for the hard disk controller
+                     * that has the same ID as hd.idController */
+                    const VirtualSystemDescriptionEntry *pController;
+                    if (!(pController = pNewDesc->i_findControllerFromID(hd.idController)))
+                        throw setError(E_FAIL,
+                                       tr("Cannot find hard disk controller with OVF instance ID %RI32 "
+                                          "to which disk \"%s\" should be attached"),
+                                       hd.idController,
+                                       di.strHref.c_str());
+
+                    /* controller to attach to, and the bus within that controller */
+                    Utf8StrFmt strExtraConfig("controller=%RI16;channel=%RI16",
+                                              pController->ulIndex,
+                                              hd.ulAddressOnParent);
+                    pNewDesc->i_addEntry(VirtualSystemDescriptionType_HardDiskImage,
+                                         hd.strDiskId,
+                                         di.strHref,
+                                         strFilename,
+                                         di.ulSuggestedSizeMB,
+                                         strExtraConfig);
                 }
             }
@@ -2383 +2366 @@
  *
  * @param di ovfreader.cpp structure describing the disk image from the OVF that is to be imported
- * @param strTargetPath Where to create the target image.
+ * @param strDstPath Where to create the target image.
  * @param pTargetHD out: The newly created target disk. This also gets pushed on stack.llHardDisksCreated for cleanup.
  * @param stack
  */
 void Appliance::i_importOneDiskImage(const ovf::DiskImage &di,
-                                     Utf8Str *pStrDstPath,
+                                     const Utf8Str &strDstPath,
                                      ComObjPtr<Medium> &pTargetHD,
                                      ImportStack &stack)
 {
+    char *pszAbsDstPath = RTPathAbsExDup(stack.strMachineFolder.c_str(),
+                                         strDstPath.c_str());
+    Utf8Str strAbsDstPath(pszAbsDstPath);
+    RTStrFree(pszAbsDstPath);
+    pszAbsDstPath = NULL;
+
     ComObjPtr<Progress> pProgress;
     pProgress.createObject();
@@ -2397 +2386 @@
                                  static_cast<IAppliance*>(this),
                                  BstrFmt(tr("Creating medium '%s'"),
-                                 pStrDstPath->c_str()).raw(),
+                                         strAbsDstPath.c_str()).raw(),
                                  TRUE);
     if (FAILED(rc)) throw rc;
@@ -2418 +2407 @@
     }
 
-    /* First of all check if the path is an UUID. If so, the user like to
-     * import the disk into an existing path. This is useful for iSCSI for
-     * example. */
+    /* First of all check if the original (non-absolute) destination path is
+     * a valid hard disk UUID. If so, the user wants to import the disk into
+     * an existing path. This is useful for iSCSI for example. */
     RTUUID uuid;
-    int vrc = RTUuidFromStr(&uuid, pStrDstPath->c_str());
+    int vrc = RTUuidFromStr(&uuid, strDstPath.c_str());
     if (vrc == VINF_SUCCESS)
     {
@@ -2442 +2431 @@
             ULONG lCabs = 0;
 
-            char *pszSuff = RTPathSuffix(pStrDstPath->c_str());
+            char *pszSuff = RTPathSuffix(strAbsDstPath.c_str());
             if (pszSuff != NULL)
             {
@@ -2472 +2461 @@
                     strTrgFormat = "vdi";
                     trgFormat = pSysProps->i_mediumFormatFromExtension(strTrgFormat);
-                    *pStrDstPath = pStrDstPath->stripSuffix();
-                    *pStrDstPath = pStrDstPath->append(".");
-                    *pStrDstPath = pStrDstPath->append(strTrgFormat.c_str());
+                    strAbsDstPath.stripSuffix();
+                    strAbsDstPath.append(".");
+                    strAbsDstPath.append(strTrgFormat.c_str());
                 }
 
@@ -2492 +2481 @@
                     throw setError(VBOX_E_NOT_SUPPORTED,
                                    tr("Could not find a valid medium format for the target disk '%s'"),
-                                   pStrDstPath->c_str());
+                                   strAbsDstPath.c_str());
             }
             else
@@ -2498 +2487 @@
                 throw setError(VBOX_E_FILE_ERROR,
                                tr("The target disk '%s' has no extension "),
-                               pStrDstPath->c_str(), VERR_INVALID_NAME);
+                               strAbsDstPath.c_str(), VERR_INVALID_NAME);
             }
 
@@ -2510 +2499 @@
                 {
                     if (fGzipped)
-                        i_importDecompressFile(stack, strSrcFilePath, *pStrDstPath, strSourceOVF.c_str());
+                        i_importDecompressFile(stack, strSrcFilePath, strAbsDstPath, strSourceOVF.c_str());
                     else
-                        i_importCopyFile(stack, strSrcFilePath, *pStrDstPath, strSourceOVF.c_str());
+                        i_importCopyFile(stack, strSrcFilePath, strAbsDstPath, strSourceOVF.c_str());
                 }
                 catch (HRESULT /*arc*/)
@@ -2529 +2518 @@
                 rc = pTargetHD->init(mVirtualBox,
                                      strTrgFormat,
-                                     *pStrDstPath,
+                                     strAbsDstPath,
                                      Guid::Empty /* media registry: none yet */,
                                      DeviceType_HardDisk);
@@ -2536 +2525 @@
                 /* Now create an empty hard disk. */
                 rc = mVirtualBox->CreateMedium(Bstr(strTrgFormat).raw(),
-                                               Bstr(*pStrDstPath).raw(),
+                                               Bstr(strAbsDstPath).raw(),
                                                AccessMode_ReadWrite, DeviceType_HardDisk,
                                                ComPtr<IMedium>(pTargetHD).asOutParam());
@@ -2556 +2545 @@
                     /* operation's weight, as set up with the IProgress originally */
                     stack.pProgress->SetNextOperation(BstrFmt(tr("Creating disk image '%s'"),
-                                                      pStrDstPath->c_str()).raw(),
+                                                      strAbsDstPath.c_str()).raw(),
                                                       di.ulSuggestedSizeMB);
                 }
@@ -2575 +2564 @@
                     if (fGzipped)
                     {
-                        Utf8Str strTargetFilePath(*pStrDstPath);
+                        Utf8Str strTargetFilePath(strAbsDstPath);
                         strTargetFilePath.stripFilename();
                         strTargetFilePath.append(RTPATH_SLASH_STR);
@@ -2685 +2674 @@
 
     /* Create the machine */
-    SafeArray<BSTR> groups; /* no groups */
-    rc = mVirtualBox->CreateMachine(NULL, /* machine name: use default */
+    SafeArray<BSTR> groups; /* no groups, or maybe one group... */
+    if (!stack.strPrimaryGroup.isEmpty())
+        Bstr(stack.strPrimaryGroup).detachTo(groups.appendedRaw());
+    rc = mVirtualBox->CreateMachine(Bstr(stack.strSettingsFilename).raw(),
                                     Bstr(stack.strNameVBox).raw(),
                                     ComSafeArrayAsInParam(groups),
@@ -3290 +3281 @@
 
                 i_importOneDiskImage(diCurrent,
-                                     &vsdeTargetHD->strVBoxCurrent,
+                                     vsdeTargetHD->strVBoxCurrent,
                                      pTargetHD,
                                      stack);
@@ -3451 +3442 @@
     /* OS Type */
     config.machineUserData.strOsType = stack.strOsTypeVBox;
+    /* Groups */
+    if (!stack.strPrimaryGroup.isEmpty())
+    {
+        config.machineUserData.llGroups.clear();
+        config.machineUserData.llGroups.push_back(stack.strPrimaryGroup);
+    }
     /* Description */
     config.machineUserData.strDescription = stack.strDescription;
@@ -3849 +3846 @@
 
                 i_importOneDiskImage(diCurrent,
-                                     &vsdeTargetHD->strVBoxCurrent,
+                                     vsdeTargetHD->strVBoxCurrent,
                                      pTargetHD,
                                      stack);
@@ -3942 +3939 @@
     rc = pNewMachine->init(mVirtualBox,
                            stack.strNameVBox,// name from OVF preparations; can be suffixed to avoid duplicates
+                           stack.strSettingsFilename,
                            config);          // the whole machine config
     if (FAILED(rc)) throw rc;
@@ -4002 +4000 @@
         stack.strNameVBox = vsdeName.front()->strVBoxCurrent;
 
-        // have VirtualBox suggest where the filename would be placed so we can
-        // put the disk images in the same directory
-        Bstr bstrMachineFilename;
-        rc = mVirtualBox->ComposeMachineFilename(Bstr(stack.strNameVBox).raw(),
-                                                 NULL /* aGroup */,
-                                                 NULL /* aCreateFlags */,
-                                                 NULL /* aBaseFolder */,
-                                                 bstrMachineFilename.asOutParam());
-        if (FAILED(rc)) throw rc;
-        // and determine the machine folder from that
-        stack.strMachineFolder = bstrMachineFilename;
+        // Primary group, which is entirely optional.
+        std::list<VirtualSystemDescriptionEntry*> vsdePrimaryGroup = vsdescThis->i_findByType(VirtualSystemDescriptionType_PrimaryGroup);
+        if (vsdePrimaryGroup.size() >= 1)
+            stack.strPrimaryGroup = vsdePrimaryGroup.front()->strVBoxCurrent;
+
+        // Draw the right conclusions from the (possibly modified) VM settings
+        // file name and base folder. If the VM settings file name is modified,
+        // it takes precedence, otherwise it is recreated from the base folder
+        // and the primary group.
+        std::list<VirtualSystemDescriptionEntry*> vsdeSettingsFile = vsdescThis->i_findByType(VirtualSystemDescriptionType_SettingsFile);
+        if (vsdeSettingsFile.size() >= 1)
+        {
+            VirtualSystemDescriptionEntry *vsdeSF1 = vsdeSettingsFile.front();
+            if (vsdeSF1->strVBoxCurrent != vsdeSF1->strVBoxSuggested)
+            stack.strSettingsFilename = vsdeSF1->strVBoxCurrent;
+        }
+        if (stack.strSettingsFilename.isEmpty())
+        {
+            Utf8Str strBaseFolder;
+            std::list<VirtualSystemDescriptionEntry*> vsdeBaseFolder = vsdescThis->i_findByType(VirtualSystemDescriptionType_BaseFolder);
+            if (vsdeBaseFolder.size() >= 1)
+                strBaseFolder = vsdeBaseFolder.front()->strVBoxCurrent;
+            Bstr bstrSettingsFilename;
+            rc = mVirtualBox->ComposeMachineFilename(Bstr(stack.strNameVBox).raw(),
+                                                     Bstr(stack.strPrimaryGroup).raw(),
+                                                     NULL /* aCreateFlags */,
+                                                     Bstr(strBaseFolder).raw(),
+                                                     bstrSettingsFilename.asOutParam());
+            if (FAILED(rc)) throw rc;
+            stack.strSettingsFilename = bstrSettingsFilename;
+        }
+
+        // Determine the machine folder from the settings file.
+        LogFunc(("i=%zu strName=%s strSettingsFilename=%s\n", i, stack.strNameVBox.c_str(), stack.strSettingsFilename.c_str()));
+        stack.strMachineFolder = stack.strSettingsFilename;
         stack.strMachineFolder.stripFilename();
-        LogFunc(("i=%zu strName=%s bstrMachineFilename=%ls\n", i, stack.strNameVBox.c_str(), bstrMachineFilename.raw()));
 
         // guest OS type

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

@@ -516 +516 @@
  *
  *  @param aParent  Associated parent object.
- *  @param strName  Name for the new machine; this overrides what is specified in config and is used
- *                  for the settings file as well.
+ *  @param strName  Name for the new machine; this overrides what is specified in config.
+ *  @param strSettingsFilename File name of .vbox file.
  *  @param config   Machine configuration loaded and parsed from XML.
  *
@@ -524 +524 @@
 HRESULT Machine::init(VirtualBox *aParent,
                       const Utf8Str &strName,
+                      const Utf8Str &strSettingsFilename,
                       const settings::MachineConfigFile &config)
 {
@@ -532 +533 @@
     AssertReturn(autoInitSpan.isOk(), E_FAIL);
 
-    Utf8Str strConfigFile;
-    aParent->i_getDefaultMachineFolder(strConfigFile);
-    strConfigFile.append(RTPATH_DELIMITER);
-    strConfigFile.append(strName);
-    strConfigFile.append(RTPATH_DELIMITER);
-    strConfigFile.append(strName);
-    strConfigFile.append(".vbox");
-
-    HRESULT rc = initImpl(aParent, strConfigFile);
+    HRESULT rc = initImpl(aParent, strSettingsFilename);
     if (FAILED(rc)) return rc;