[PATCH 5/8] vbox: Introduce vboxReportError()

[Martin Kletzander]

On Mon, Jan 23, 2023 at 10:35:52AM +0100, Michal Privoznik wrote:
>When a VirtualBox API fails it produced an exception. Until now,
>we did not have correct APIs wired up to get the exception and
>its error message. Thus, we were left with plain:
>
>  virReportError("virtualbox API failed, rc=%08x", rc);
>
>This is not very user friendly because those rc values are hard
>to parse (e.g. some values are defined as a sum of a base value
>and some other value) and also it expects users to know where to
>look.
>
>But now that we have all machinery needed for querying
>exceptions, vboxReportError() can be introduced. The aim is to
>query VirtualBox exceptions and append them after the error
>message we intent to report. If the exception can't be queried
>successfully, this behaves exactly like virReportError().
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/vbox/vbox_common.c | 113 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 113 insertions(+)
>
>diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
>index bd77641d39..2a913e5975 100644
>--- a/src/vbox/vbox_common.c
>+++ b/src/vbox/vbox_common.c
>@@ -63,6 +63,119 @@ static struct _vboxDriver *vbox_driver;
> static struct _vboxDriver *vboxDriverObjNew(void);
> static __thread bool vboxDriverDisposed;
>
>+#define vboxReportError(errcode, ...) \
>+    vboxReportErrorHelper(data, errcode, __FILE__, \
>+                          __FUNCTION__, __LINE__, __VA_ARGS__)
>+
>+static void G_GNUC_PRINTF(6, 7) G_GNUC_UNUSED
>+vboxReportErrorHelper(struct _vboxDriver *data,
>+                      int errcode,
>+                      const char *filename,
>+                      const char *funcname,
>+                      size_t linenr,
>+                      const char *fmt, ...)
>+{
>+    int save_errno = errno;
>+    g_auto(virBuffer) errBuf = VIR_BUFFER_INITIALIZER;
>+    nsIException *ex = NULL;
>+    IVirtualBoxErrorInfo *ei = NULL;
>+    const nsID *vboxErrorInfoIID = NULL;
>+    bool multipleLines = false;
>+    nsresult rc;
>+    g_autofree char *detail = NULL;
>+
>+    if (fmt) {
>+        va_list args;
>+
>+        va_start(args, fmt);
>+        detail = g_strdup_vprintf(fmt, args);
>+        va_end(args);
>+    }
>+
>+    rc = gVBoxAPI.UPFN.GetException(data->pFuncs, &ex);
>+    if (NS_FAILED(rc) || !ex) {
>+        VIR_WARN("failed to get exception object");
>+        goto report;
>+    }
>+
>+    vboxErrorInfoIID = gVBoxAPI.UIVirtualBoxErrorInfo.GetIID();
>+    rc = VBOX_QUERY_INTERFACE(ex, vboxErrorInfoIID, (void **)&ei);
>+    if (NS_FAILED(rc) || !ei) {
>+        VIR_WARN("unable to typecast exception object");
>+        goto report;
>+    }
>+
>+    while (ei) {
>+        IVirtualBoxErrorInfo *ei_next = NULL;
>+        PRUnichar *componentUtf16 = NULL;
>+        char *componentUtf8 = NULL;
>+        PRUnichar *textUtf16 = NULL;
>+        char *textUtf8 = NULL;
>+
>+        rc = gVBoxAPI.UIVirtualBoxErrorInfo.GetComponent(ei, &componentUtf16);
>+        if (NS_FAILED(rc)) {
>+            VIR_WARN("failed to get error text");

Maybe s/text/component/ so that we know where to look if more things go
wrong.  But wow on the complexity of getting a sensible error message
from vbox.  I guess it'd be easier in C++.

>+            goto report;
>+        }
>+
>+        rc = gVBoxAPI.UIVirtualBoxErrorInfo.GetText(ei, &textUtf16);
>+        if (NS_FAILED(rc)) {
>+            VBOX_UTF16_FREE(componentUtf16);
>+            VIR_WARN("failed to get error text");
>+            goto report;
>+        }
>+
>+        VBOX_UTF16_TO_UTF8(componentUtf16, &componentUtf8);
>+        VBOX_UTF16_FREE(componentUtf16);
>+
>+        VBOX_UTF16_TO_UTF8(textUtf16, &textUtf8);
>+        VBOX_UTF16_FREE(textUtf16);
>+
>+        virBufferAsprintf(&errBuf, "%s: %s", componentUtf8, textUtf8);
>+        VBOX_UTF8_FREE(componentUtf8);
>+        VBOX_UTF8_FREE(textUtf8);
>+
>+        if (multipleLines)
>+            virBufferAddChar(&errBuf, '\n');
>+        else
>+            multipleLines = true;
>+
>+        rc = gVBoxAPI.UIVirtualBoxErrorInfo.GetNext(ei, &ei_next);
>+        if (NS_FAILED(rc)) {
>+            break;
>+        }
>+
>+        VBOX_RELEASE(ei);
>+        ei = ei_next;
>+    }
>+
>+ report:
>+    if (virBufferUse(&errBuf)) {
>+        const char *vboxErr = virBufferCurrentContent(&errBuf);
>+        g_autofree char *newDetail = NULL;
>+
>+        if (!detail || STREQ(detail, "")) {
>+            newDetail = g_strdup(vboxErr);
>+        } else {
>+            newDetail = g_strdup_printf("%s: %s", detail, vboxErr);
>+        }
>+
>+        VIR_FREE(detail);
>+        detail = g_steal_pointer(&newDetail);
>+    }
>+
>+    virReportErrorHelper(VIR_FROM_THIS, errcode, filename, funcname, linenr, "%s", detail);
>+
>+    rc = gVBoxAPI.UPFN.ClearException(data->pFuncs);
>+    if (NS_FAILED(rc)) {
>+        VIR_WARN("failed to clear exception");
>+    }
>+
>+    VBOX_RELEASE(ei);
>+    VBOX_RELEASE(ex);
>+    errno = save_errno;
>+}
>+
> static int
> vboxDomainDevicesDefPostParse(virDomainDeviceDef *dev G_GNUC_UNUSED,
>                               const virDomainDef *def G_GNUC_UNUSED,
>-- 
>2.39.1
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230124/1cee18fe/attachment.sig>