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

[John Ferlan]

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 at redhat.com>


John


[...]