Currently, when qemu_signal_init() fails it only returns a non-zero
value but without propagating any Error. But its callers need a
non-null err when runs error_report_err(err), or else 0->msg occurs.
To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.
This patch also adds the omitted error handling when creating signalfd
pipe fails in qemu_signalfd_compat().
Signed-off-by: Fei Li <fli@suse.com>
---
include/qemu/osdep.h | 2 +-
util/compatfd.c | 9 ++++++---
util/main-loop.c | 11 +++++------
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index a91068df0e..09ed85fcb8 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
additional fields in the future) */
};
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
void sigaction_invoke(struct sigaction *action,
struct qemu_signalfd_siginfo *info);
#endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 980bd33e52..d3ed890405 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -16,6 +16,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/thread.h"
+#include "qapi/error.h"
#include <sys/syscall.h>
@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
}
}
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
{
struct sigfd_compat_info *info;
QemuThread thread;
@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
info = malloc(sizeof(*info));
if (info == NULL) {
+ error_setg(errp, "Failed to allocate signalfd memory");
errno = ENOMEM;
return -1;
}
if (pipe(fds) == -1) {
+ error_setg(errp, "Failed to create signalfd pipe");
free(info);
return -1;
}
@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
return fds[0];
}
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
{
#if defined(CONFIG_SIGNALFD)
int ret;
@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
}
#endif
- return qemu_signalfd_compat(mask);
+ return qemu_signalfd_compat(mask, errp);
}
diff --git a/util/main-loop.c b/util/main-loop.c
index affe0403c5..b783f3c806 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
}
}
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
int sigfd;
sigset_t set;
@@ -94,10 +94,9 @@ static int qemu_signal_init(void)
pthread_sigmask(SIG_BLOCK, &set, NULL);
sigdelset(&set, SIG_IPI);
- sigfd = qemu_signalfd(&set);
+ sigfd = qemu_signalfd(&set, errp);
if (sigfd == -1) {
- fprintf(stderr, "failed to create signalfd\n");
- return -errno;
+ return -1;
}
fcntl_setfl(sigfd, O_NONBLOCK);
@@ -109,7 +108,7 @@ static int qemu_signal_init(void)
#else /* _WIN32 */
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
return 0;
}
@@ -148,7 +147,7 @@ int qemu_init_main_loop(Error **errp)
init_clocks(qemu_timer_notify_cb);
- ret = qemu_signal_init();
+ ret = qemu_signal_init(errp);
if (ret) {
return ret;
}
--
2.13.7
On Wed, 09/19 21:35, Fei Li wrote:
> Currently, when qemu_signal_init() fails it only returns a non-zero
> value but without propagating any Error. But its callers need a
> non-null err when runs error_report_err(err), or else 0->msg occurs.
>
> To avoid such segmentation fault, add a new Error parameter to make
> the call trace to propagate the err to the final caller.
>
> This patch also adds the omitted error handling when creating signalfd
> pipe fails in qemu_signalfd_compat().
>
> Signed-off-by: Fei Li <fli@suse.com>
> ---
> include/qemu/osdep.h | 2 +-
> util/compatfd.c | 9 ++++++---
> util/main-loop.c | 11 +++++------
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index a91068df0e..09ed85fcb8 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
> additional fields in the future) */
> };
>
> -int qemu_signalfd(const sigset_t *mask);
> +int qemu_signalfd(const sigset_t *mask, Error **errp);
> void sigaction_invoke(struct sigaction *action,
> struct qemu_signalfd_siginfo *info);
> #endif
> diff --git a/util/compatfd.c b/util/compatfd.c
> index 980bd33e52..d3ed890405 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -16,6 +16,7 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> #include "qemu/thread.h"
> +#include "qapi/error.h"
>
> #include <sys/syscall.h>
>
> @@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
> }
> }
>
> -static int qemu_signalfd_compat(const sigset_t *mask)
> +static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
> {
> struct sigfd_compat_info *info;
> QemuThread thread;
> @@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>
> info = malloc(sizeof(*info));
> if (info == NULL) {
> + error_setg(errp, "Failed to allocate signalfd memory");
> errno = ENOMEM;
> return -1;
> }
>
> if (pipe(fds) == -1) {
> + error_setg(errp, "Failed to create signalfd pipe");
> free(info);
> return -1;
> }
> @@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
> return fds[0];
> }
>
> -int qemu_signalfd(const sigset_t *mask)
> +int qemu_signalfd(const sigset_t *mask, Error **errp)
> {
> #if defined(CONFIG_SIGNALFD)
> int ret;
> @@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
> }
> #endif
>
> - return qemu_signalfd_compat(mask);
> + return qemu_signalfd_compat(mask, errp);
> }
> diff --git a/util/main-loop.c b/util/main-loop.c
> index affe0403c5..b783f3c806 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
> }
> }
>
> -static int qemu_signal_init(void)
> +static int qemu_signal_init(Error **errp)
> {
> int sigfd;
> sigset_t set;
> @@ -94,10 +94,9 @@ static int qemu_signal_init(void)
> pthread_sigmask(SIG_BLOCK, &set, NULL);
>
> sigdelset(&set, SIG_IPI);
> - sigfd = qemu_signalfd(&set);
> + sigfd = qemu_signalfd(&set, errp);
> if (sigfd == -1) {
> - fprintf(stderr, "failed to create signalfd\n");
> - return -errno;
> + return -1;
Why change from -errno to -1?
> }
>
> fcntl_setfl(sigfd, O_NONBLOCK);
> @@ -109,7 +108,7 @@ static int qemu_signal_init(void)
>
> #else /* _WIN32 */
>
> -static int qemu_signal_init(void)
> +static int qemu_signal_init(Error **errp)
> {
> return 0;
> }
> @@ -148,7 +147,7 @@ int qemu_init_main_loop(Error **errp)
>
> init_clocks(qemu_timer_notify_cb);
>
> - ret = qemu_signal_init();
> + ret = qemu_signal_init(errp);
> if (ret) {
> return ret;
> }
> --
> 2.13.7
>
On 09/19/2018 10:49 PM, Fam Zheng wrote:
> On Wed, 09/19 21:35, Fei Li wrote:
>> Currently, when qemu_signal_init() fails it only returns a non-zero
>> value but without propagating any Error. But its callers need a
>> non-null err when runs error_report_err(err), or else 0->msg occurs.
>>
>> To avoid such segmentation fault, add a new Error parameter to make
>> the call trace to propagate the err to the final caller.
>>
>> This patch also adds the omitted error handling when creating signalfd
>> pipe fails in qemu_signalfd_compat().
>>
>> Signed-off-by: Fei Li <fli@suse.com>
>> ---
>> include/qemu/osdep.h | 2 +-
>> util/compatfd.c | 9 ++++++---
>> util/main-loop.c | 11 +++++------
>> 3 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index a91068df0e..09ed85fcb8 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
>> additional fields in the future) */
>> };
>>
>> -int qemu_signalfd(const sigset_t *mask);
>> +int qemu_signalfd(const sigset_t *mask, Error **errp);
>> void sigaction_invoke(struct sigaction *action,
>> struct qemu_signalfd_siginfo *info);
>> #endif
>> diff --git a/util/compatfd.c b/util/compatfd.c
>> index 980bd33e52..d3ed890405 100644
>> --- a/util/compatfd.c
>> +++ b/util/compatfd.c
>> @@ -16,6 +16,7 @@
>> #include "qemu/osdep.h"
>> #include "qemu-common.h"
>> #include "qemu/thread.h"
>> +#include "qapi/error.h"
>>
>> #include <sys/syscall.h>
>>
>> @@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
>> }
>> }
>>
>> -static int qemu_signalfd_compat(const sigset_t *mask)
>> +static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
>> {
>> struct sigfd_compat_info *info;
>> QemuThread thread;
>> @@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>>
>> info = malloc(sizeof(*info));
>> if (info == NULL) {
>> + error_setg(errp, "Failed to allocate signalfd memory");
>> errno = ENOMEM;
>> return -1;
>> }
>>
>> if (pipe(fds) == -1) {
>> + error_setg(errp, "Failed to create signalfd pipe");
>> free(info);
>> return -1;
>> }
>> @@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>> return fds[0];
>> }
>>
>> -int qemu_signalfd(const sigset_t *mask)
>> +int qemu_signalfd(const sigset_t *mask, Error **errp)
>> {
>> #if defined(CONFIG_SIGNALFD)
>> int ret;
>> @@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
>> }
>> #endif
>>
>> - return qemu_signalfd_compat(mask);
>> + return qemu_signalfd_compat(mask, errp);
>> }
>> diff --git a/util/main-loop.c b/util/main-loop.c
>> index affe0403c5..b783f3c806 100644
>> --- a/util/main-loop.c
>> +++ b/util/main-loop.c
>> @@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
>> }
>> }
>>
>> -static int qemu_signal_init(void)
>> +static int qemu_signal_init(Error **errp)
>> {
>> int sigfd;
>> sigset_t set;
>> @@ -94,10 +94,9 @@ static int qemu_signal_init(void)
>> pthread_sigmask(SIG_BLOCK, &set, NULL);
>>
>> sigdelset(&set, SIG_IPI);
>> - sigfd = qemu_signalfd(&set);
>> + sigfd = qemu_signalfd(&set, errp);
>> if (sigfd == -1) {
>> - fprintf(stderr, "failed to create signalfd\n");
>> - return -errno;
>> + return -1;
> Why change from -errno to -1?
I changed this for two reasons.
One is I misunderstood that if pipe() fails the errno is not set
automatically.
And that's why I wrote the third paragraph in the commit message. Obviously
what I thought was wrong, and I will delete that msg in next version. :)
The second is for the following qemu_thread_create(), the current code
returns
a boolean true/false instead of the int err, considering propagating the
error
message and returning a int err may be duplicated.
But considering "return -errno" explains the failure reason and my
misunderstood
about pipe(), I'd prefer using "return -errno;" and adding the
"errno=err" in the
qemu_thread_create() function in patch 7/7. What do you think?
Have a nice day, thanks
Fei
>
>> }
>>
>> fcntl_setfl(sigfd, O_NONBLOCK);
>> @@ -109,7 +108,7 @@ static int qemu_signal_init(void)
>>
>> #else /* _WIN32 */
>>
>> -static int qemu_signal_init(void)
>> +static int qemu_signal_init(Error **errp)
>> {
>> return 0;
>> }
>> @@ -148,7 +147,7 @@ int qemu_init_main_loop(Error **errp)
>>
>> init_clocks(qemu_timer_notify_cb);
>>
>> - ret = qemu_signal_init();
>> + ret = qemu_signal_init(errp);
>> if (ret) {
>> return ret;
>> }
>> --
>> 2.13.7
>>
>
>
On Wed, Sep 19, 2018 at 09:35:17PM +0800, Fei Li wrote:
> Currently, when qemu_signal_init() fails it only returns a non-zero
> value but without propagating any Error. But its callers need a
> non-null err when runs error_report_err(err), or else 0->msg occurs.
>
> To avoid such segmentation fault, add a new Error parameter to make
> the call trace to propagate the err to the final caller.
>
> This patch also adds the omitted error handling when creating signalfd
> pipe fails in qemu_signalfd_compat().
>
> Signed-off-by: Fei Li <fli@suse.com>
Hi, Fei,
Please do s/comc/com/ in the CC list, then I can receive your
emails. :)
Could you explain a bit how the segfault is triggered? malloc() and
pipe() aren't something that will fault easily to me, so I would think
we just assert (especially it's in a very early phase of the process).
Thanks,
> ---
> include/qemu/osdep.h | 2 +-
> util/compatfd.c | 9 ++++++---
> util/main-loop.c | 11 +++++------
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index a91068df0e..09ed85fcb8 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
> additional fields in the future) */
> };
>
> -int qemu_signalfd(const sigset_t *mask);
> +int qemu_signalfd(const sigset_t *mask, Error **errp);
> void sigaction_invoke(struct sigaction *action,
> struct qemu_signalfd_siginfo *info);
> #endif
> diff --git a/util/compatfd.c b/util/compatfd.c
> index 980bd33e52..d3ed890405 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -16,6 +16,7 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> #include "qemu/thread.h"
> +#include "qapi/error.h"
>
> #include <sys/syscall.h>
>
> @@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
> }
> }
>
> -static int qemu_signalfd_compat(const sigset_t *mask)
> +static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
> {
> struct sigfd_compat_info *info;
> QemuThread thread;
> @@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>
> info = malloc(sizeof(*info));
> if (info == NULL) {
> + error_setg(errp, "Failed to allocate signalfd memory");
> errno = ENOMEM;
> return -1;
> }
>
> if (pipe(fds) == -1) {
> + error_setg(errp, "Failed to create signalfd pipe");
> free(info);
> return -1;
> }
> @@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
> return fds[0];
> }
>
> -int qemu_signalfd(const sigset_t *mask)
> +int qemu_signalfd(const sigset_t *mask, Error **errp)
> {
> #if defined(CONFIG_SIGNALFD)
> int ret;
> @@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
> }
> #endif
>
> - return qemu_signalfd_compat(mask);
> + return qemu_signalfd_compat(mask, errp);
> }
> diff --git a/util/main-loop.c b/util/main-loop.c
> index affe0403c5..b783f3c806 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
> }
> }
>
> -static int qemu_signal_init(void)
> +static int qemu_signal_init(Error **errp)
> {
> int sigfd;
> sigset_t set;
> @@ -94,10 +94,9 @@ static int qemu_signal_init(void)
> pthread_sigmask(SIG_BLOCK, &set, NULL);
>
> sigdelset(&set, SIG_IPI);
> - sigfd = qemu_signalfd(&set);
> + sigfd = qemu_signalfd(&set, errp);
> if (sigfd == -1) {
> - fprintf(stderr, "failed to create signalfd\n");
> - return -errno;
> + return -1;
> }
>
> fcntl_setfl(sigfd, O_NONBLOCK);
> @@ -109,7 +108,7 @@ static int qemu_signal_init(void)
>
> #else /* _WIN32 */
>
> -static int qemu_signal_init(void)
> +static int qemu_signal_init(Error **errp)
> {
> return 0;
> }
> @@ -148,7 +147,7 @@ int qemu_init_main_loop(Error **errp)
>
> init_clocks(qemu_timer_notify_cb);
>
> - ret = qemu_signal_init();
> + ret = qemu_signal_init(errp);
> if (ret) {
> return ret;
> }
> --
> 2.13.7
>
>
Regards,
--
Peter Xu
On 09/20/2018 11:28 AM, Peter Xu wrote:
> On Wed, Sep 19, 2018 at 09:35:17PM +0800, Fei Li wrote:
>> Currently, when qemu_signal_init() fails it only returns a non-zero
>> value but without propagating any Error. But its callers need a
>> non-null err when runs error_report_err(err), or else 0->msg occurs.
>>
>> To avoid such segmentation fault, add a new Error parameter to make
>> the call trace to propagate the err to the final caller.
>>
>> This patch also adds the omitted error handling when creating signalfd
>> pipe fails in qemu_signalfd_compat().
>>
>> Signed-off-by: Fei Li <fli@suse.com>
> Hi, Fei,
>
> Please do s/comc/com/ in the CC list, then I can receive your
> emails. :)
So sorry for the misspelling, somehow an unanticipated "xx.com"c is sent. :(
>
> Could you explain a bit how the segfault is triggered? malloc() and
> pipe() aren't something that will fault easily to me, so I would think
> we just assert (especially it's in a very early phase of the process).
>
> Thanks,
This is actually triggered after the 7th patch is applied, when I do the
test
by hard coding the returning value of pthread_create() as EPERM or EINVAL.
Considering there is already an "exit()“ if qemu_init_main_loop() fails, let
us use this exit()?
Have a nice day, thanks for the review. :)
Fei
>
>> ---
>> include/qemu/osdep.h | 2 +-
>> util/compatfd.c | 9 ++++++---
>> util/main-loop.c | 11 +++++------
>> 3 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index a91068df0e..09ed85fcb8 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
>> additional fields in the future) */
>> };
>>
>> -int qemu_signalfd(const sigset_t *mask);
>> +int qemu_signalfd(const sigset_t *mask, Error **errp);
>> void sigaction_invoke(struct sigaction *action,
>> struct qemu_signalfd_siginfo *info);
>> #endif
>> diff --git a/util/compatfd.c b/util/compatfd.c
>> index 980bd33e52..d3ed890405 100644
>> --- a/util/compatfd.c
>> +++ b/util/compatfd.c
>> @@ -16,6 +16,7 @@
>> #include "qemu/osdep.h"
>> #include "qemu-common.h"
>> #include "qemu/thread.h"
>> +#include "qapi/error.h"
>>
>> #include <sys/syscall.h>
>>
>> @@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
>> }
>> }
>>
>> -static int qemu_signalfd_compat(const sigset_t *mask)
>> +static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
>> {
>> struct sigfd_compat_info *info;
>> QemuThread thread;
>> @@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>>
>> info = malloc(sizeof(*info));
>> if (info == NULL) {
>> + error_setg(errp, "Failed to allocate signalfd memory");
>> errno = ENOMEM;
>> return -1;
>> }
>>
>> if (pipe(fds) == -1) {
>> + error_setg(errp, "Failed to create signalfd pipe");
>> free(info);
>> return -1;
>> }
>> @@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>> return fds[0];
>> }
>>
>> -int qemu_signalfd(const sigset_t *mask)
>> +int qemu_signalfd(const sigset_t *mask, Error **errp)
>> {
>> #if defined(CONFIG_SIGNALFD)
>> int ret;
>> @@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
>> }
>> #endif
>>
>> - return qemu_signalfd_compat(mask);
>> + return qemu_signalfd_compat(mask, errp);
>> }
>> diff --git a/util/main-loop.c b/util/main-loop.c
>> index affe0403c5..b783f3c806 100644
>> --- a/util/main-loop.c
>> +++ b/util/main-loop.c
>> @@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
>> }
>> }
>>
>> -static int qemu_signal_init(void)
>> +static int qemu_signal_init(Error **errp)
>> {
>> int sigfd;
>> sigset_t set;
>> @@ -94,10 +94,9 @@ static int qemu_signal_init(void)
>> pthread_sigmask(SIG_BLOCK, &set, NULL);
>>
>> sigdelset(&set, SIG_IPI);
>> - sigfd = qemu_signalfd(&set);
>> + sigfd = qemu_signalfd(&set, errp);
>> if (sigfd == -1) {
>> - fprintf(stderr, "failed to create signalfd\n");
>> - return -errno;
>> + return -1;
>> }
>>
>> fcntl_setfl(sigfd, O_NONBLOCK);
>> @@ -109,7 +108,7 @@ static int qemu_signal_init(void)
>>
>> #else /* _WIN32 */
>>
>> -static int qemu_signal_init(void)
>> +static int qemu_signal_init(Error **errp)
>> {
>> return 0;
>> }
>> @@ -148,7 +147,7 @@ int qemu_init_main_loop(Error **errp)
>>
>> init_clocks(qemu_timer_notify_cb);
>>
>> - ret = qemu_signal_init();
>> + ret = qemu_signal_init(errp);
>> if (ret) {
>> return ret;
>> }
>> --
>> 2.13.7
>>
>>
> Regards,
>
On Thu, Sep 20, 2018 at 12:46:39PM +0800, Fei Li wrote: > > > On 09/20/2018 11:28 AM, Peter Xu wrote: > > On Wed, Sep 19, 2018 at 09:35:17PM +0800, Fei Li wrote: > > > Currently, when qemu_signal_init() fails it only returns a non-zero > > > value but without propagating any Error. But its callers need a > > > non-null err when runs error_report_err(err), or else 0->msg occurs. > > > > > > To avoid such segmentation fault, add a new Error parameter to make > > > the call trace to propagate the err to the final caller. > > > > > > This patch also adds the omitted error handling when creating signalfd > > > pipe fails in qemu_signalfd_compat(). > > > > > > Signed-off-by: Fei Li <fli@suse.com> > > Hi, Fei, > > > > Please do s/comc/com/ in the CC list, then I can receive your > > emails. :) > So sorry for the misspelling, somehow an unanticipated "xx.com"c is sent. :( > > > > Could you explain a bit how the segfault is triggered? malloc() and > > pipe() aren't something that will fault easily to me, so I would think > > we just assert (especially it's in a very early phase of the process). > > > > Thanks, > This is actually triggered after the 7th patch is applied, when I do the > test > by hard coding the returning value of pthread_create() as EPERM or EINVAL. > Considering there is already an "exit()“ if qemu_init_main_loop() fails, let > us use this exit()? I see. For me, null-referencing is already a good debugging tool itself which works just like assertions (then we see the stack clearly enough when rare bad things happened), so I'll just leave these to the other reviewers (I saw that Fam reviewed most of the series already). Regards, -- Peter Xu
© 2016 - 2026 Red Hat, Inc.