[PATCH 5/6] virprocess: Passthru error from virProcessRunInForkHelper

[Ján Tomko]

On a Wednesday in 2020, Michal Privoznik wrote:
>When running a function in a forked child, so far the only thing
>we could report is exit status of the child and the error
>message. However, it may be beneficial to the caller to know the
>actual error that happened in the child.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> build-aux/syntax-check.mk |  2 +-
> src/util/virprocess.c     | 51 ++++++++++++++++++++++++++++++++++++---
> tests/commandtest.c       | 43 +++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 5 deletions(-)
>
>diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
>index 3020921be8..2a38c03ba9 100644
>--- a/build-aux/syntax-check.mk
>+++ b/build-aux/syntax-check.mk
>@@ -1987,7 +1987,7 @@ exclude_file_name_regexp--sc_flags_usage = \
> exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
>   ^(src/rpc/gendispatch\.pl$$|tests/)
>
>-exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$)
>+exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$)
>
> exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \
>   ^(build-aux/syntax-check\.mk|include/libvirt/virterror\.h|src/remote/remote_daemon_dispatch\.c|src/util/virerror\.c|docs/internals/oomtesting\.html\.in)$$
>diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>index 24135070b7..e8674402f9 100644
>--- a/src/util/virprocess.c
>+++ b/src/util/virprocess.c
>@@ -1126,6 +1126,23 @@ virProcessRunInMountNamespace(pid_t pid G_GNUC_UNUSED,
>
>
> #ifndef WIN32
>+typedef struct {
>+    int code;
>+    int domain;
>+    char message[VIR_ERROR_MAX_LENGTH];
>+    virErrorLevel level;
>+    char str1[VIR_ERROR_MAX_LENGTH];
>+    char str2[VIR_ERROR_MAX_LENGTH];
>+    /* str3 doesn't seem to be used. Ignore it. */
>+    int int1;
>+    int int2;
>+} errorData;
>+
>+typedef union {
>+    errorData data;
>+    char bindata[sizeof(errorData)];
>+} errorDataBin;
>+
> static int
> virProcessRunInForkHelper(int errfd,
>                           pid_t ppid,
>@@ -1134,9 +1151,19 @@ virProcessRunInForkHelper(int errfd,
> {
>     if (cb(ppid, opaque) < 0) {
>         virErrorPtr err = virGetLastError();
>+        errorDataBin bin = { 0 };
>+

Consider allocating this on the heap instead of a 3K+ stack variable.

Jano

>         if (err) {
>-            size_t len = strlen(err->message) + 1;
>-            ignore_value(safewrite(errfd, err->message, len));
>+            bin.data.code = err->code;
>+            bin.data.domain = err->domain;
>+            ignore_value(virStrcpy(bin.data.message, err->message, sizeof(bin.data.message)));
>+            bin.data.level = err->level;
>+            ignore_value(virStrcpy(bin.data.str1, err->str1, sizeof(bin.data.str1)));
>+            ignore_value(virStrcpy(bin.data.str2, err->str2, sizeof(bin.data.str2)));
>+            bin.data.int1 = err->int1;
>+            bin.data.int2 = err->int2;
>+
>+            ignore_value(safewrite(errfd, bin.bindata, sizeof(bin)));
>         }
>
>         return -1;
>@@ -1188,16 +1215,32 @@ virProcessRunInFork(virProcessForkCallback cb,
>     } else {
>         int status;
>         g_autofree char *buf = NULL;
>+        errorDataBin bin;

Same here.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200319/97a4d039/attachment-0001.sig>