Re: [libvirt] [PATCH v2 06/15] vbox: Errors in vboxAttachDrives are now critical

3 Nov 2017

On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
...
Previously, if one tried to define a VBOX VM and the API failed to
perform the requested actions for some reason, it would just log the
error and move on to process remaining disk definitions. This is not
desired as it could result in incorrectly defined VM without the caller
even knowing about it. So now all the code paths that call
virReportError are now treated as hard failures as they should have
been.
---
 src/vbox/vbox_common.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index b949c4db7..9f4bf18a3 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -955,17 +955,17 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxDriverPtr data,
     }
 }
-static void
+static int
 vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
 {
     size_t i;
-    int type, format;
+    int type, format, ret = 0;
     const char *src = NULL;
     nsresult rc = 0;
     virDomainDiskDefPtr disk = NULL;
     PRUnichar *storageCtlName = NULL;
     IMedium *medium = NULL;
-    PRUnichar *mediumFileUtf16 = NULL, *mediumEmpty = NULL;
+    PRUnichar *mediumFileUtf16 = NULL;
     PRUint32 devicePort, deviceSlot, deviceType, accessMode;
     vboxIID mediumUUID;
@@ -1049,6 +1049,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
                 deviceType = DeviceType_Floppy;
                 accessMode = AccessMode_ReadWrite;
             } else {
+                ret = -1;
                 goto cleanup;
             }
@@ -1056,19 +1057,17 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
                                                deviceType, accessMode, &medium);
if (!medium) {
-                VBOX_UTF8_TO_UTF16("", &mediumEmpty);
-
                 rc = gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj,
                                                       mediumFileUtf16,
                                                       deviceType, accessMode,
                                                       &medium);
-                VBOX_UTF16_FREE(mediumEmpty);
             }
The mediumEmpty changes could have gone into the previous patch or in a
separate patch.  It's unused as of commit 34364df3 which should have
never copied it in from the "old" code which ended up getting removed as
part of commit c7c286c6.

I can extract it into a separate patch before pushing...

Reviewed-by: John Ferlan <jferlan@redhat.com>


John


[...]

    