[vbox-dev] Import/Export NVMe Disks (#19320) [PATCH]

Dave Cardwell hello at davecardwell.com
Wed Feb 19 00:51:30 GMT 2020


Hi there,

Re: https://www.virtualbox.org/ticket/19320 - Allow exporting & importing
of VMs with NVMe storage devices
<https://www.virtualbox.org/ticket/19320>

VirtualBox errors with VBOX_E_NOT_SUPPORTED when exporting a VM with an
NVMe controller. I am not a C++ programmer but have had a crack at the
patch below by taking cues from the import/export code for the other
controller types.

As far as I can tell there is no OVF specified ResourceSubType for NVMe so
I chose “nvme” like VirtualBox’s existing internal representation. VMWare
Fusion uses vmware.nvme.controller in its exports so I supported that as
well for importing. Let me know if you would rather use that for
compatibility on export, or a vendor prefixed version like
org.virtualbox.nvme instead.


Following code licensed under MIT.


Index: src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceEditorWidget.cpp
===================================================================
--- src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceEditorWidget.cpp
(revision 82781)
+++ src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceEditorWidget.cpp
(working copy)
@@ -482,6 +482,7 @@
                     case
KVirtualSystemDescriptionType_HardDiskControllerSATA: value =
UIApplianceEditorWidget::tr("Storage Controller (SATA)"); break;
                     case
KVirtualSystemDescriptionType_HardDiskControllerSCSI: value =
UIApplianceEditorWidget::tr("Storage Controller (SCSI)"); break;
                     case
KVirtualSystemDescriptionType_HardDiskControllerSAS:  value =
UIApplianceEditorWidget::tr("Storage Controller (SAS)"); break;
+                    case
KVirtualSystemDescriptionType_HardDiskControllerNVMe: value =
UIApplianceEditorWidget::tr("Storage Controller (NVMe)"); break;
                     case KVirtualSystemDescriptionType_CDROM:
     value = UIApplianceEditorWidget::tr("DVD"); break;
                     case KVirtualSystemDescriptionType_Floppy:
    value = UIApplianceEditorWidget::tr("Floppy"); break;
                     case KVirtualSystemDescriptionType_NetworkAdapter:
    value = UIApplianceEditorWidget::tr("Network Adapter"); break;
@@ -656,6 +657,7 @@
                     case
KVirtualSystemDescriptionType_HardDiskControllerSATA: value =
UIIconPool::iconSet(":/sata_16px.png"); break;
                     case
KVirtualSystemDescriptionType_HardDiskControllerSCSI: value =
UIIconPool::iconSet(":/scsi_16px.png"); break;
                     case
KVirtualSystemDescriptionType_HardDiskControllerSAS:  value =
UIIconPool::iconSet(":/sas_16px.png"); break;
+                    case
KVirtualSystemDescriptionType_HardDiskControllerNVMe: value =
UIIconPool::iconSet(":/pcie_16px.png"); break;
                     case KVirtualSystemDescriptionType_HardDiskImage:
     value = UIIconPool::iconSet(":/hd_16px.png"); break;
                     case KVirtualSystemDescriptionType_CDROM:
     value = UIIconPool::iconSet(":/cd_16px.png"); break;
                     case KVirtualSystemDescriptionType_Floppy:
    value = UIIconPool::iconSet(":/fd_16px.png"); break;
@@ -1340,7 +1342,8 @@
                 if (types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerIDE ||
                     types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerSATA ||
                     types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerSCSI ||
-                    types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerSAS)
+                    types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerSAS ||
+                    types[i] ==
KVirtualSystemDescriptionType_HardDiskControllerNVMe)
                     controllerMap[i] = pHardwareItem;
             }
         }
@@ -1681,6 +1684,7 @@
     KVirtualSystemDescriptionType_HardDiskControllerSATA,
     KVirtualSystemDescriptionType_HardDiskControllerSCSI,
     KVirtualSystemDescriptionType_HardDiskControllerSAS,
+    KVirtualSystemDescriptionType_HardDiskControllerNVMe,
     /* OCI */
     KVirtualSystemDescriptionType_CloudProfileName,
     KVirtualSystemDescriptionType_CloudBucket,
Index:
src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceExportEditorWidget.cpp
===================================================================
---
src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceExportEditorWidget.cpp
(revision 82781)
+++
src/VBox/Frontends/VirtualBox/src/widgets/UIApplianceExportEditorWidget.cpp
(working copy)
@@ -50,6 +50,7 @@
             << KVirtualSystemDescriptionType_HardDiskControllerSATA
             << KVirtualSystemDescriptionType_HardDiskControllerSCSI
             << KVirtualSystemDescriptionType_HardDiskControllerSAS
+            << KVirtualSystemDescriptionType_HardDiskControllerNVMe
             << KVirtualSystemDescriptionType_CloudProfileName;
     }
 };
Index: src/VBox/Main/idl/VirtualBox.xidl
===================================================================
--- src/VBox/Main/idl/VirtualBox.xidl (revision 82781)
+++ src/VBox/Main/idl/VirtualBox.xidl (working copy)
@@ -4024,6 +4024,7 @@
     <const name="CloudOCISubnetCompartment" value="47" />
     <const name="CloudPublicSSHKey" value="48" />
     <const name="BootingFirmware" value="49" />
+    <const name="HardDiskControllerNVMe" value="50" />
   </enum>

   <enum
Index: src/VBox/Main/include/ovfreader.h
===================================================================
--- src/VBox/Main/include/ovfreader.h (revision 82781)
+++ src/VBox/Main/include/ovfreader.h (working copy)
@@ -534,8 +534,8 @@
 {
     uint32_t                idController;       // instance ID
(Item/InstanceId); this gets referenced from VirtualDisk

-    enum ControllerSystemType { IDE, SATA, SCSI };
-    ControllerSystemType    system;             // one of IDE, SATA, SCSI
+    enum ControllerSystemType { IDE, SATA, SCSI, NVMe };
+    ControllerSystemType    system;             // one of IDE, SATA, SCSI,
NVMe

     RTCString        strControllerType;
             // controller subtype (Item/ResourceSubType); e.g. "LsiLogic";
can be empty (esp. for IDE)
Index: src/VBox/Main/src-server/ApplianceImpl.cpp
===================================================================
--- src/VBox/Main/src-server/ApplianceImpl.cpp (revision 82781)
+++ src/VBox/Main/src-server/ApplianceImpl.cpp (working copy)
@@ -1770,6 +1770,7 @@
             case VirtualSystemDescriptionType_HardDiskControllerSATA:
             case VirtualSystemDescriptionType_HardDiskControllerSCSI:
             case VirtualSystemDescriptionType_HardDiskControllerSAS:
+            case VirtualSystemDescriptionType_HardDiskControllerNVMe:
                 if (d.strRef == strRef)
                     return &d;
                 break;
Index: src/VBox/Main/src-server/ApplianceImplExport.cpp
===================================================================
--- src/VBox/Main/src-server/ApplianceImplExport.cpp (revision 82781)
+++ src/VBox/Main/src-server/ApplianceImplExport.cpp (working copy)
@@ -185,6 +185,7 @@
         int32_t lIDEControllerSecondaryIndex = 0;
         int32_t lSATAControllerIndex = 0;
         int32_t lSCSIControllerIndex = 0;
+        int32_t lNVMeControllerIndex = 0;

         /* Fetch all available storage controllers */
         com::SafeIfaceArray<IStorageController> nwControllers;
@@ -195,6 +196,7 @@
         ComPtr<IStorageController> pSATAController;
         ComPtr<IStorageController> pSCSIController;
         ComPtr<IStorageController> pSASController;
+        ComPtr<IStorageController> pNVMeController;
         for (size_t j = 0; j < nwControllers.size(); ++j)
         {
             StorageBus_T eType;
@@ -212,6 +214,8 @@
             else if (   eType == StorageBus_SAS
                      && pSASController.isNull())
                 pSASController = nwControllers[j];
+            else if (   eType == StorageBus_PCIe)
+                pNVMeController = nwControllers[j];
         }

 //     <const name="HardDiskControllerIDE" value="6" />
@@ -292,6 +296,16 @@
                                  strVBox);
         }

+        if (!pNVMeController.isNull())
+        {
+            Utf8Str strVBox = "NVMe";
+            lNVMeControllerIndex =
(int32_t)pNewDesc->m->maDescriptions.size();
+
 pNewDesc->i_addEntry(VirtualSystemDescriptionType_HardDiskControllerNVMe,
+                                 Utf8StrFmt("%d", lNVMeControllerIndex),
+                                 strVBox,
+                                 strVBox);
+        }
+
 //     <const name="HardDiskImage" value="9" />
 //     <const name="Floppy" value="18" />
 //     <const name="CDROM" value="19" />
@@ -344,6 +358,8 @@
                 strStBus = "SCSI";
                 else if ( storageBus == StorageBus_SAS)
                 strStBus = "SAS";
+                else if ( storageBus == StorageBus_PCIe)
+                strStBus = "PCIe";
                 LogRel(("Warning: skip the medium (bus: %s, slot: %d,
port: %d). No storage device attached.\n",
                 strStBus.c_str(), lDevice, lChannel));
                 continue;
@@ -520,6 +536,11 @@
                     lControllerVsys = lSCSIControllerIndex;
                     break;

+                case StorageBus_PCIe:
+                    lChannelVsys = lChannel;        // should be between 0
and 255
+                    lControllerVsys = lNVMeControllerIndex;
+                    break;
+
                 case StorageBus_Floppy:
                     lChannelVsys = 0;
                     lControllerVsys = 0;
@@ -1477,6 +1498,8 @@
     int32_t lSATAControllerIndex = 0;
     uint32_t idSCSIController = 0;
     int32_t lSCSIControllerIndex = 0;
+    uint32_t idNVMeController = 0;
+    int32_t lNVMeControllerIndex = 0;

     uint32_t ulInstanceID = 1;

@@ -1499,6 +1522,7 @@
                           : desc.type ==
VirtualSystemDescriptionType_HardDiskControllerSATA ?
"HardDiskControllerSATA"
                           : desc.type ==
VirtualSystemDescriptionType_HardDiskControllerSCSI ?
"HardDiskControllerSCSI"
                           : desc.type ==
VirtualSystemDescriptionType_HardDiskControllerSAS ? "HardDiskControllerSAS"
+                          : desc.type ==
VirtualSystemDescriptionType_HardDiskControllerNVMe ?
"HardDiskControllerNVMe"
                           : desc.type ==
VirtualSystemDescriptionType_HardDiskImage ? "HardDiskImage"
                           : Utf8StrFmt("%d", desc.type).c_str()),
                          desc.strRef.c_str(),
@@ -1689,6 +1713,42 @@
                     }
                     break;

+                case VirtualSystemDescriptionType_HardDiskControllerNVMe:
+                    /*  <Item>
+                            <rasd:Caption>nvmeController0</rasd:Caption>
+                            <rasd:Description>NVMe
Controller</rasd:Description>
+                            <rasd:InstanceId>4</rasd:InstanceId>
+                            <rasd:ResourceType>20</rasd:ResourceType>
+
 <rasd:ResourceSubType>nvme</rasd:ResourceSubType>
+                            <rasd:Address>0</rasd:Address>
+                            <rasd:BusNumber>0</rasd:BusNumber>
+                        </Item>
+                    */
+                    if (uLoop == 1)
+                    {
+                        strDescription = "NVMe Controller";
+                        strCaption = "nvmeController0";
+                        type = ovf::ResourceType_OtherStorageDevice; // 20
+                        // it seems that OVFTool always writes these two,
and since we can only
+                        // have one NVMe controller, we'll use this as well
+                        lAddress = 0;
+                        lBusNumber = 0;
+
+                        if (    desc.strVBoxCurrent.isEmpty()      // NVMe
is the default in VirtualBox
+                             || (!desc.strVBoxCurrent.compare("nvme",
Utf8Str::CaseInsensitive))
+                            )
+                            strResourceSubType = "nvme";
+                        else
+                            throw setError(VBOX_E_NOT_SUPPORTED,
+                                           tr("Invalid config string
\"%s\" in NVMe controller"),
+                                           desc.strVBoxCurrent.c_str());
+
+                        // remember this ID
+                        idNVMeController = ulInstanceID;
+                        lNVMeControllerIndex = lIndexThis;
+                    }
+                    break;
+
                 case VirtualSystemDescriptionType_HardDiskImage:
                     /*  <Item>
                             <rasd:Caption>disk1</rasd:Caption>
@@ -1725,6 +1785,8 @@
                                 ulParent = idSCSIController;
                             else if (lControllerIndex ==
lSATAControllerIndex)
                                 ulParent = idSATAController;
+                            else if (lControllerIndex ==
lNVMeControllerIndex)
+                                ulParent = idNVMeController;
                         }
                         if (pos2 != Utf8Str::npos)

 RTStrToInt32Ex(desc.strExtraConfigCurrent.c_str() + pos2 + 8, NULL, 0,
&lAddressOnParent);
@@ -1802,6 +1864,8 @@
                                 ulParent = idSCSIController;
                             else if (lControllerIndex ==
lSATAControllerIndex)
                                 ulParent = idSATAController;
+                            else if (lControllerIndex ==
lNVMeControllerIndex)
+                                ulParent = idNVMeController;
                         }
                         if (pos2 != Utf8Str::npos)

 RTStrToInt32Ex(desc.strExtraConfigCurrent.c_str() + pos2 + 8, NULL, 0,
&lAddressOnParent);
Index: src/VBox/Main/src-server/ApplianceImplImport.cpp
===================================================================
--- src/VBox/Main/src-server/ApplianceImplImport.cpp (revision 82781)
+++ src/VBox/Main/src-server/ApplianceImplImport.cpp (working copy)
@@ -561,6 +561,7 @@
             uint16_t cIDEused = 0;
             uint16_t cSATAused = 0; NOREF(cSATAused);
             uint16_t cSCSIused = 0; NOREF(cSCSIused);
+            uint16_t cNVMeused = 0; NOREF(cNVMeused);
             ovf::ControllersMap::const_iterator hdcIt;
             /* Iterate through all storage controllers */
             for (hdcIt = vsysThis.mapControllers.begin();
@@ -649,6 +650,28 @@
                                             strControllerID.c_str());
                         ++cSCSIused;
                     break;
+
+                    case ovf::HardDiskController::NVMe:
+                        /* Check for the constrains */
+                        if (cNVMeused < 1)
+                        {
+                            /* We only support a plain NVMe controller, so
use them always */
+
 pNewDesc->i_addEntry(VirtualSystemDescriptionType_HardDiskControllerNVMe,
+                                                 strControllerID,
+                                                 hdc.strControllerType,
+                                                 "NVMe");
+                        }
+                        else
+                        {
+                            /* Warn only once */
+                            if (cNVMeused == 1)
+                                i_addWarning(tr("The virtual system \"%s\"
requests support for more than one "
+                                                "NVMe controller, but
VirtualBox has support for only one"),
+                                                vsysThis.strName.c_str());
+
+                        }
+                        ++cNVMeused;
+                    break;
                 }
             }

@@ -3516,6 +3539,14 @@
             break;
         }

+        case ovf::HardDiskController::NVMe:
+        {
+            controllerName = "NVMe";
+            lControllerPort = (long)ulAddressOnParent;
+            lDevice = (long)0;
+            break;
+        }
+
         default: break;
     }

@@ -4229,6 +4260,29 @@
         if (FAILED(rc)) throw rc;
     }

+    /* Storage controller NVMe */
+    std::list<VirtualSystemDescriptionEntry*> vsdeHDCNVMe =
+
 vsdescThis->i_findByType(VirtualSystemDescriptionType_HardDiskControllerNVMe);
+    if (vsdeHDCNVMe.size() > 1)
+        throw setError(VBOX_E_FILE_ERROR,
+                       tr("Too many NVMe controllers in OVF; import
facility only supports one"));
+    if (!vsdeHDCNVMe.empty())
+    {
+        ComPtr<IStorageController> pController;
+        const Utf8Str &hdcVBox = vsdeHDCNVMe.front()->strVBoxCurrent;
+        if (hdcVBox == "NVMe")
+        {
+            rc = pNewMachine->AddStorageController(Bstr("NVMe").raw(),
+                                                   StorageBus_PCIe,
+
pController.asOutParam());
+            if (FAILED(rc)) throw rc;
+        }
+        else
+            throw setError(VBOX_E_FILE_ERROR,
+                           tr("Invalid NVMe controller type \"%s\""),
+                           hdcVBox.c_str());
+    }
+
     /* Now its time to register the machine before we add any storage
devices */
     rc = mVirtualBox->RegisterMachine(pNewMachine);
     if (FAILED(rc)) throw rc;
Index: src/VBox/Main/testcase/tstOVF.cpp
===================================================================
--- src/VBox/Main/testcase/tstOVF.cpp (revision 82781)
+++ src/VBox/Main/testcase/tstOVF.cpp (working copy)
@@ -194,6 +194,10 @@
                     pcszType = "scsi";
                 break;

+                case VirtualSystemDescriptionType_HardDiskControllerNVMe:
+                    pcszType = "nvme";
+                break;
+
                 case VirtualSystemDescriptionType_HardDiskImage:
                     pcszType = "hd";
                 break;
Index: src/VBox/Main/xml/ovfreader.cpp
===================================================================
--- src/VBox/Main/xml/ovfreader.cpp (revision 82781)
+++ src/VBox/Main/xml/ovfreader.cpp (working copy)
@@ -640,7 +640,7 @@
                         // handled separately in second loop below
                         break;

-                    case ResourceType_OtherStorageDevice:        // 20
  SATA controller
+                    case ResourceType_OtherStorageDevice:        // 20
  SATA/NVMe controller
                     {
                         /* <Item>
                             <rasd:Description>SATA
Controller</rasd:Description>
@@ -661,8 +661,18 @@

                             vsys.mapControllers[i.ulInstanceID] = hdc;
                         }
+                        else if (   i.strResourceSubType.compare("nvme",
RTCString::CaseInsensitive) == 0
+                            ||
i.strResourceSubType.compare("vmware.nvme.controller",
RTCString::CaseInsensitive) == 0)
+                        {
+                            HardDiskController hdc;
+                            hdc.system = HardDiskController::NVMe;
+                            hdc.idController = i.ulInstanceID;
+                            hdc.strControllerType = i.strResourceSubType;
+
+                            vsys.mapControllers[i.ulInstanceID] = hdc;
+                        }
                         else
-                            throw OVFLogicError(N_("Error reading \"%s\":
Host resource of type \"Other Storage Device (%d)\" is supported with SATA
AHCI controllers only, line %d (subtype:%s)"),
+                            throw OVFLogicError(N_("Error reading \"%s\":
Host resource of type \"Other Storage Device (%d)\" is supported with SATA
AHCI and NVMe controllers only, line %d (subtype:%s)"),
                                                 m_strPath.c_str(),

 ResourceType_OtherStorageDevice,
                                                 i.ulLineNumber,
i.strResourceSubType.c_str() );


-- 
Best wishes,
Dave Cardwell.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20200218/e8a2aaa0/attachment.html>


More information about the vbox-dev mailing list