Changeset 88746 in vbox for trunk/src/VBox/Frontends

Timestamp:

Apr 28, 2021 1:08:53 PM (4 years ago)

Author:

vboxsync

Message:

FE/Qt: bugref:8400: Rework r130366; More careful checks and coding-style; Return early on errors; Also, no need to search for medium while it's there already.

Location:

trunk/src/VBox/Frontends/VirtualBox/src/medium

Files:

• UIMediumItem.cpp (modified) (3 diffs)
• UIMediumItem.h (modified) (1 diff)

trunk/src/VBox/Frontends/VirtualBox/src/medium/UIMediumItem.cpp

@@ -201 +201 @@
 bool UIMediumItem::changeMediumType(KMediumType enmOldType, KMediumType enmNewType)
 {
+    /* Cache the list of VMs the medium is attached to. We will need this for the re-attachment: */
     QList<AttachmentCache> attachmentCacheList;
-    /* Cache the list of vms the medium is attached to. We will need this for the re-attachment: */
     foreach (const QUuid &uMachineId, medium().curStateMachineIds())
     {
@@ -208 +208 @@
         if (comMachine.isNull())
             continue;
-        CMediumAttachmentVector attachments = comMachine.GetMediumAttachments();
-        foreach (const CMediumAttachment &attachment, attachments)
-        {
-            const CMedium& comMedium = attachment.GetMedium();
-            if (comMedium.isNull() || comMedium.GetId() != id())
+        foreach (const CStorageController &comController, comMachine.GetStorageControllers())
+        {
+            if (comController.isNull())
                 continue;
-            AttachmentCache attachmentCache;
-            attachmentCache.m_uMachineId = uMachineId;
-            attachmentCache.m_strControllerName = attachment.GetController();
-            attachmentCache.m_port = attachment.GetPort();
-            attachmentCache.m_device = attachment.GetDevice();
-            attachmentCacheList << attachmentCache;
-        }
-    }
-
-    /* Detach the medium from all the vms it is already attached to: */
+            const QString strControllerName = comController.GetName();
+            foreach (const CMediumAttachment &comAttachment, comMachine.GetMediumAttachmentsOfController(strControllerName))
+            {
+                if (comAttachment.isNull())
+                    continue;
+                const CMedium &comMedium = comAttachment.GetMedium();
+                if (comMedium.isNull() || comMedium.GetId() != id())
+                    continue;
+                AttachmentCache attachmentCache;
+                attachmentCache.m_uMachineId = uMachineId;
+                attachmentCache.m_strControllerName = strControllerName;
+                attachmentCache.m_enmControllerBus = comController.GetBus();
+                attachmentCache.m_iAttachmentPort = comAttachment.GetPort();
+                attachmentCache.m_iAttachmentDevice = comAttachment.GetDevice();
+                attachmentCacheList << attachmentCache;
+            }
+        }
+    }
+
+    /* Detach the medium from all the VMs it's attached to: */
     if (!release(true))
         return false;
 
-    /* Search for corresponding medium: */
-    CMedium comMedium = uiCommon().medium(id()).medium();
-
     /* Attempt to change medium type: */
+    CMedium comMedium = medium().medium();
     comMedium.SetType(enmNewType);
-    bool fSuccess = true;
-    /* Show error message if necessary: */
-    if (!comMedium.isOk() && parentTree())
-    {
-        msgCenter().cannotChangeMediumType(comMedium, enmOldType, enmNewType, treeWidget());
-        fSuccess = false;
-    }
-    /* Reattach the medium to all the vms it was previously attached: */
+    if (!comMedium.isOk())
+    {
+        msgCenter().cannotChangeMediumType(comMedium, enmOldType, enmNewType, parentTree());
+        return false;
+    }
+
+    /* Reattach the medium to all the VMs it was previously attached: */
     foreach (const AttachmentCache &attachmentCache, attachmentCacheList)
-        attachTo(attachmentCache);
-    return fSuccess;
+        if (!attachTo(attachmentCache))
+            return false;
+
+    /* True finally: */
+    return true;
 }
 
@@ -350 +358 @@
 bool UIMediumItem::attachTo(const AttachmentCache &attachmentCache)
 {
+    /* Open session: */
+    CSession comSession = uiCommon().openSession(attachmentCache.m_uMachineId);
+    if (comSession.isNull())
+        return false;
+
+    /* Attach device to machine: */
     CMedium comMedium = medium().medium();
-
-    if (comMedium.isNull())
-        return false;
-
-    /* Open session: */
-    CSession session = uiCommon().openSession(attachmentCache.m_uMachineId);
-    if (session.isNull())
-        return false;
-
-    /* Get machine: */
-    CMachine machine = session.GetMachine();
-
-    bool fSuccess = false;
-    machine.AttachDevice(attachmentCache.m_strControllerName, attachmentCache.m_port,
-                         attachmentCache.m_device, comMedium.GetDeviceType(), comMedium);
-
-    if (machine.isOk())
-    {
-        machine.SaveSettings();
-        if (!machine.isOk())
-            msgCenter().cannotSaveMachineSettings(machine, treeWidget());
-        else
-            fSuccess = true;
+    KDeviceType enmDeviceType = comMedium.GetDeviceType();
+    CMachine comMachine = comSession.GetMachine();
+    comMachine.AttachDevice(attachmentCache.m_strControllerName,
+                            attachmentCache.m_iAttachmentPort,
+                            attachmentCache.m_iAttachmentDevice,
+                            enmDeviceType,
+                            comMedium);
+    if (!comMachine.isOk())
+        msgCenter().cannotAttachDevice(comMachine,
+                                       mediumTypeToLocal(enmDeviceType),
+                                       comMedium.GetLocation(),
+                                       StorageSlot(attachmentCache.m_enmControllerBus,
+                                                   attachmentCache.m_iAttachmentPort,
+                                                   attachmentCache.m_iAttachmentDevice),
+                                       parentTree());
+    else
+    {
+        /* Save machine settings: */
+        comMachine.SaveSettings();
+        if (!comMachine.isOk())
+            msgCenter().cannotSaveMachineSettings(comMachine, parentTree());
     }
 
     /* Close session: */
-    session.UnlockMachine();
-
-    /* Return result: */
-    return fSuccess;
+    comSession.UnlockMachine();
+
+    /* True finally: */
+    return true;
 }
 

trunk/src/VBox/Frontends/VirtualBox/src/medium/UIMediumItem.h

@@ -121 +121 @@
 
 private:
+
     /** A simple struct used to save some parameters of machine device attachment.
-      * Used for re-attaching the medium to vms after a medium type change. */
+      * Used for re-attaching the medium to VMs after a medium type change. */
     struct AttachmentCache
     {
-        QString m_strControllerName;
-        QUuid   m_uMachineId;
-        LONG    m_port;
-        LONG    m_device;
+        /** Holds the machine ID. */
+        QUuid        m_uMachineId;
+        /** Holds the controller name. */
+        QString      m_strControllerName;
+        /** Holds the controller bus. */
+        KStorageBus  m_enmControllerBus;
+        /** Holds the attachment port. */
+        LONG         m_iAttachmentPort;
+        /** Holds the attachment device. */
+        LONG         m_iAttachmentDevice;
     };