Changeset 16662 in vbox for trunk

Timestamp:

Feb 11, 2009 1:01:48 PM (16 years ago)

Author:

vboxsync

Message:

OVF: create new IAppliance::read() method instead of reading in constructor; IVirtualBox::openAppliance -> createAppliance; IAppliance->importAppliance -> importMachines

Location:

trunk/src/VBox

Files:

• Frontends/VBoxManage/VBoxManageImport.cpp (modified) (2 diffs)
• Frontends/VirtualBox/src/VBoxImportAppliance.cpp (modified) (2 diffs)
• Main/ApplianceImpl.cpp (modified) (11 diffs)
• Main/idl/VirtualBox.xidl (modified) (7 diffs)
• Main/include/ApplianceImpl.h (modified) (2 diffs)
• Main/include/VirtualBoxImpl.h (modified) (1 diff)

trunk/src/VBox/Frontends/VBoxManage/VBoxManageImport.cpp

@@ -155 +155 @@
         Bstr bstrOvfFilename(strOvfFilename);
         ComPtr<IAppliance> appliance;
-        CHECK_ERROR_BREAK(a->virtualBox, OpenAppliance(bstrOvfFilename, appliance.asOutParam()));
+        CHECK_ERROR_BREAK(a->virtualBox, CreateAppliance(appliance.asOutParam()));
+
+        CHECK_ERROR_BREAK(appliance, Read(bstrOvfFilename));
 
         RTPrintf("Interpreting %s... ", strOvfFilename.c_str());
@@ -434 +436 @@
                 ComPtr<IProgress> progress;
                 CHECK_ERROR_BREAK(appliance,
-                                  ImportAppliance(progress.asOutParam()));
+                                  ImportMachines(progress.asOutParam()));
 
                 showProgress(progress);

trunk/src/VBox/Frontends/VirtualBox/src/VBoxImportAppliance.cpp

@@ -954 +954 @@
         CVirtualBox vbox = vboxGlobal().virtualBox();
         /* Open the appliance */
-        CAppliance appliance = vbox.OpenAppliance (file);
+        CAppliance appliance = vbox.CreateAppliance();
         if (appliance.isOk())
         {
+            appliance.Read(file); // @todo error handling
             /* Now we have to interpret that stuff */
             appliance.Interpret();
@@ -967 +968 @@
                     /* Start the import asynchronously */
                     CProgress progress;
-                    progress = appliance.ImportAppliance();
+                    progress = appliance.ImportMachines();
                     if (!appliance.isOk())
                     {

trunk/src/VBox/Main/ApplianceImpl.cpp

@@ -239 +239 @@
 
 /**
- * Implementation for IVirtualBox::openAppliance. Loads the given appliance (see API reference).
+ * Implementation for IVirtualBox::createAppliance.
  *
- * @param bstrPath Appliance to open (either .ovf or .ova file, see API reference)
  * @param anAppliance IAppliance object created if S_OK is returned.
  * @return S_OK or error.
  */
-STDMETHODIMP VirtualBox::OpenAppliance(IN_BSTR bstrPath, IAppliance** anAppliance)
+STDMETHODIMP VirtualBox::CreateAppliance(IAppliance** anAppliance)
 {
     HRESULT rc;
@@ -251 +250 @@
     ComObjPtr<Appliance> appliance;
     appliance.createObject();
-    /* @todo r=poetzsch: We should consider to split the creation & opening of
-       the appliance into two calls. Now the returned appliance is null in the
-       case of an error. But at the same time the error message is in the
-       appliance object which isn't returned. So I propose: createAppliance in
-       IVirtualBox & OpenAppliance in IAppliance. Such an error could easily
-       produced if you set an invalid path to the open call. */
-    rc = appliance->init(this, bstrPath);
+    rc = appliance->init(this);
 //     ComAssertComRCThrowRC(rc);
 
@@ -861 +854 @@
  */
 
-HRESULT Appliance::init(VirtualBox *aVirtualBox, IN_BSTR &path)
+HRESULT Appliance::init(VirtualBox *aVirtualBox)
 {
     HRESULT rc;
@@ -869 +862 @@
     AssertReturn(autoInitSpan.isOk(), E_FAIL);
 
-    /* Weakly reference to a VirtualBox object */
+    /* Weak reference to a VirtualBox object */
     unconst(mVirtualBox) = aVirtualBox;
 
     // initialize data
     m = new Data;
-    m->bstrPath = path;
-
-    // see if we can handle this file; for now we insist it has an ".ovf" extension
-    Utf8Str utf8Path(path);
-    const char *pcszLastDot = strrchr(utf8Path, '.');
-    if (    (!pcszLastDot)
-         || (    strcmp(pcszLastDot, ".ovf")
-              && strcmp(pcszLastDot, ".OVF")
-            )
-       )
-        return setError(VBOX_E_FILE_ERROR,
-                        tr("Appliance file must have .ovf extension"));
-
-    try
-    {
-        xml::XmlFileParser parser;
-        xml::Document doc;
-        parser.read(utf8Path.raw(),
-                    doc);
-
-        const xml::Node *pRootElem = doc.getRootElement();
-        if (strcmp(pRootElem->getName(), "Envelope"))
-            return setError(VBOX_E_FILE_ERROR,
-                            tr("Root element in OVF file must be \"Envelope\"."));
-
-        // OVF has the following rough layout:
-        /*
-            -- <References> ....  files referenced from other parts of the file, such as VMDK images
-            -- Metadata, comprised of several section commands
-            -- virtual machines, either a single <VirtualSystem>, or a <VirtualSystemCollection>
-            -- optionally <Strings> for localization
-        */
-
-        // get all "File" child elements of "References" section so we can look up files easily;
-        // first find the "References" sections so we can look up files
-        xml::NodesList listFileElements;      // receives all /Envelope/References/File nodes
-        const xml::Node *pReferencesElem;
-        if ((pReferencesElem = pRootElem->findChildElement("References")))
-            pReferencesElem->getChildElements(listFileElements, "File");
-
-        // now go though the sections
-        if (!(SUCCEEDED(rc = LoopThruSections(utf8Path.raw(), pReferencesElem, pRootElem))))
-            return rc;
-    }
-    catch(xml::Error &x)
-    {
-        return setError(VBOX_E_FILE_ERROR,
-                        x.what());
-    }
 
     /* Confirm a successful initialization */
@@ -1202 +1146 @@
 }
 
+STDMETHODIMP Appliance::Read(IN_BSTR path)
+{
+    HRESULT rc = S_OK;
+
+    if (!path)
+        return E_POINTER;
+
+    AutoCaller autoCaller(this);
+    CheckComRCReturnRC(autoCaller.rc());
+
+    AutoWriteLock alock(this);
+    m->bstrPath = path;
+
+    // see if we can handle this file; for now we insist it has an ".ovf" extension
+    Utf8Str utf8Path(path);
+    const char *pcszLastDot = strrchr(utf8Path, '.');
+    if (    (!pcszLastDot)
+         || (    strcmp(pcszLastDot, ".ovf")
+              && strcmp(pcszLastDot, ".OVF")
+            )
+       )
+        return setError(VBOX_E_FILE_ERROR,
+                        tr("Appliance file must have .ovf extension"));
+
+    try
+    {
+        xml::XmlFileParser parser;
+        xml::Document doc;
+        parser.read(utf8Path.raw(),
+                    doc);
+
+        const xml::Node *pRootElem = doc.getRootElement();
+        if (strcmp(pRootElem->getName(), "Envelope"))
+            return setError(VBOX_E_FILE_ERROR,
+                            tr("Root element in OVF file must be \"Envelope\"."));
+
+        // OVF has the following rough layout:
+        /*
+            -- <References> ....  files referenced from other parts of the file, such as VMDK images
+            -- Metadata, comprised of several section commands
+            -- virtual machines, either a single <VirtualSystem>, or a <VirtualSystemCollection>
+            -- optionally <Strings> for localization
+        */
+
+        // get all "File" child elements of "References" section so we can look up files easily;
+        // first find the "References" sections so we can look up files
+        xml::NodesList listFileElements;      // receives all /Envelope/References/File nodes
+        const xml::Node *pReferencesElem;
+        if ((pReferencesElem = pRootElem->findChildElement("References")))
+            pReferencesElem->getChildElements(listFileElements, "File");
+
+        // now go though the sections
+        if (!(SUCCEEDED(rc = LoopThruSections(utf8Path.raw(), pReferencesElem, pRootElem))))
+            return rc;
+    }
+    catch(xml::Error &x)
+    {
+        return setError(VBOX_E_FILE_ERROR,
+                        x.what());
+    }
+
+    return S_OK;
+}
+
 STDMETHODIMP Appliance::Interpret()
 {
@@ -1537 +1545 @@
 }
 
-STDMETHODIMP Appliance::ImportAppliance(IProgress **aProgress)
+STDMETHODIMP Appliance::ImportMachines(IProgress **aProgress)
 {
     CheckComArgOutPointerValid(aProgress);
@@ -1710 +1718 @@
             if (!task->progress.isNull())
             {
-                rc = task->progress->notifyProgress(opCountMax * opCount++);
+                rc = task->progress->notifyProgress((uint32_t)(opCountMax * opCount++));
                 CheckComRCThrowRC(rc);
             }
@@ -1736 +1744 @@
             if (!task->progress.isNull())
             {
-                rc = task->progress->notifyProgress(opCountMax * opCount++);
+                rc = task->progress->notifyProgress((uint32_t)(opCountMax * opCount++));
                 CheckComRCThrowRC(rc);
             }
@@ -1773 +1781 @@
             if (!task->progress.isNull())
             {
-                rc = task->progress->notifyProgress(opCountMax * opCount++);
+                rc = task->progress->notifyProgress((uint32_t)(opCountMax * opCount++));
                 CheckComRCThrowRC(rc);
             }
@@ -1825 +1833 @@
             if (!task->progress.isNull())
             {
-                rc = task->progress->notifyProgress(opCountMax * opCount++);
+                rc = task->progress->notifyProgress((uint32_t)(opCountMax * opCount++));
                 CheckComRCThrowRC(rc);
             }
@@ -1892 +1900 @@
             if (!task->progress.isNull())
             {
-                rc = task->progress->notifyProgress(opCountMax * opCount++);
+                rc = task->progress->notifyProgress((uint32_t)(opCountMax * opCount++));
                 CheckComRCThrowRC(rc);
             }

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

@@ -1727 +1727 @@
     </method>
 
-    <method name="openAppliance">
-      <desc>
-        Attempts to open the given appliance, which must be in Open Virtual Machine Format (OVF).
-
-        As prescribed by the OVF standard, VirtualBox supports OVF in two formats:
-
-        <ol>
-          <li>If the OVF is distributed as a set of files, then @a file must be a fully qualified
-          path name to an existing OVF descriptor file with an <tt>.ovf</tt> file extension. If
-          this descriptor file references other files, as OVF appliances distributed as a set of
-          files most likely do, those files must be in the same directory as the descriptor file.</li>
-
-          <li>If the OVF is distributed as a single file, it must be in TAR format and have the
-          <tt>.ova</tt> file extension. This TAR file must then contain at least the OVF descriptor
-          files and optionally other files.</li>
-        </ol>
-
-        In both cases, VirtualBox will open the OVF descriptor file, parse its contents and create a
-        new instance of IAppliance representing the OVF file.
-
-        This method succeeds if the OVF is syntactically valid and, by itself, without errors. The
-        returned IAppliance object can then be used to retrieve information about the appliance and
-        import it into VirtualBox. The mere fact that this method returns successfully does not mean
-        that VirtualBox supports all features requested by the appliance; see the documentation for
-        <link to="IAppliance" /> for details.
-
-      </desc>
-      <param name="file" type="wstring" dir="in">
-        <desc>
-          Name of appliance file to open (either with an <tt>.ovf</tt> or <tt>.ova</tt> extension, depending
-          on whether the appliance is distributed as a set of files or as a single file, respectively).
-        </desc>
-      </param>
-      <param name="machine" type="IAppliance" dir="return">
+    <method name="createAppliance">
+      <desc>
+        Creates a new appliance object, which represents an appliance in the Open Virtual Machine
+        Format (OVF). This can then be used to import an OVF appliance into VirtualBox; see the
+        documentation for <link to="IAppliance" /> for details.
+      </desc>
+      <param name="appliance" type="IAppliance" dir="return">
         <desc>New appliance.</desc>
       </param>
@@ -2882 +2855 @@
     <desc>
         Represents an appliance in OVF format. An instance of this is returned by
-        <link to="IVirtualBox::openAppliance" />.
+        <link to="IVirtualBox::createAppliance" />.
 
         Importing an OVF appliance into VirtualBox is a three-step process:
@@ -2888 +2861 @@
         <ol>
           <li>
-          Call <link to="IVirtualBox::openAppliance" /> with the full path of the OVF
-          file. So long as the appliance file is syntactically valid, this will succeed
-          and return an instance of IAppliance that contains the parsed data from the
-          OVF file.
+          Call <link to="IVirtualBox::createAppliance" />. This will create an empty IAppliance object.
+          </li>
+
+          <li>
+          Call <link to="#open" /> with the full path of the OVF file you would like to
+          import. So long as this file is syntactically valid, this will succeed and return
+          an instance of IAppliance that contains the parsed data from the OVF file.
           </li>
 
@@ -2903 +2879 @@
           </li>
 
-          <li>Finally, the caller should invoke <link to="#importAppliance" />, which will
+          <li>Finally, the caller should invoke <link to="#importMachines" />, which will
           create virtual machines in VirtualBox as instances of <link to="IMachine" /> that
           match the information in the virtual system descriptions.
@@ -2913 +2889 @@
     <attribute name="path" type="wstring" readonly="yes">
       <desc>Path to the main file of the OVF appliance, which is either the <tt>.ovf</tt> or
-      the <tt>.ova</tt> file passed to <link to="IVirtualBox::openAppliance" />.</desc>
+      the <tt>.ova</tt> file passed to <link to="IVirtualBox::read" />.</desc>
     </attribute>
 
@@ -2956 +2932 @@
     </attribute>
 
+    <method name="read">
+      <desc>
+        The OVF standard suggests two different file formats:
+
+        <ol>
+          <li>If the OVF is distributed as a set of files, then @a file must be a fully qualified
+          path name to an existing OVF descriptor file with an <tt>.ovf</tt> file extension. If
+          this descriptor file references other files, as OVF appliances distributed as a set of
+          files most likely do, those files must be in the same directory as the descriptor file.</li>
+
+          <li>If the OVF is distributed as a single file, it must be in TAR format and have the
+          <tt>.ova</tt> file extension. This TAR file must then contain at least the OVF descriptor
+          files and optionally other files. This variant is not yet supported by VirtualBox;
+          support will be added in a later version.</li>
+        </ol>
+
+        This method succeeds if the OVF is syntactically valid and, by itself, without errors. The
+        mere fact that this method returns successfully does not mean that VirtualBox supports all
+        features requested by the appliance; this can be examined after a call to <link to="#interpret" />.
+      </desc>
+      <param name="file" type="wstring" dir="in">
+        <desc>
+          Name of appliance file to open (either with an <tt>.ovf</tt> or <tt>.ova</tt> extension, depending
+          on whether the appliance is distributed as a set of files or as a single file, respectively).
+        </desc>
+      </param>
+    </method>
+
     <method name="interpret">
       <desc>
@@ -2969 +2973 @@
     </method>
 
-    <method name="importAppliance">
+    <method name="importMachines">
       <desc>
         Imports the appliance into VirtualBox by creating instances of <link to="IMachine" />

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

@@ -62 +62 @@
     void FinalRelease() { uninit(); }
 
-    HRESULT init(VirtualBox *aVirtualBox, IN_BSTR &path);
+    HRESULT init(VirtualBox *aVirtualBox);
     void uninit();
 
@@ -74 +74 @@
 
     /* IAppliance methods */
-    /* void interpret (); */
+    STDMETHOD(Read)(IN_BSTR path);
     STDMETHOD(Interpret)(void);
-    STDMETHOD(ImportAppliance)(IProgress **aProgress);
+    STDMETHOD(ImportMachines)(IProgress **aProgress);
 
     /* public methods only for internal purposes */

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

@@ -145 +145 @@
     STDMETHOD(FindMachine) (IN_BSTR aName, IMachine **aMachine);
     STDMETHOD(UnregisterMachine) (IN_GUID aId, IMachine **aMachine);
-    STDMETHOD(OpenAppliance) (IN_BSTR aName, IAppliance **anAppliance);
+    STDMETHOD(CreateAppliance) (IAppliance **anAppliance);
 
     STDMETHOD(CreateHardDisk2) (IN_BSTR aFormat, IN_BSTR aLocation,