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(a)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