* [Qemu-devel] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
@ 2014-09-02 16:22 ` Chen Gang
0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-02 16:22 UTC (permalink / raw)
To: stefano.stabellini; +Cc: xen-devel, qemu-devel
When failure occurs, it need use "return -1" instead of exit(1), so can
let upper caller has chance to print failure information, too, then user
can know the failure result more clearly.
xen_hvm_init() may also return -errno, which may let upper caller think
more (e.g. free some other related resources and try again), although at
present, all related upper callers still exit(1).
It is not a normal function which does not release related resources, if
return -1. So need give the related comments for it.
It passes common test under fedora 20 x86_64:
"./configure --enable-xen && make -j4 && make check"
execute result: "echo $? == 0".
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
xen-hvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen-hvm.c b/xen-hvm.c
index 0d09940..35efec0 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
}
+/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
MemoryRegion **ram_memory)
{
@@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
state->xenstore = xs_daemon_open();
if (state->xenstore == NULL) {
perror("xen: xenstore open");
+ xc_evtchn_close(state->xce_handle);
g_free(state);
return -errno;
}
@@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
/* Initialize backend core & drivers */
if (xen_be_init() != 0) {
fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
- exit(1);
+ return -1;
}
xen_be_register("console", &xen_console_ops);
xen_be_register("vkbd", &xen_kbdmouse_ops);
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
@ 2014-09-02 16:22 ` Chen Gang
0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-02 16:22 UTC (permalink / raw)
To: stefano.stabellini; +Cc: xen-devel, qemu-devel
When failure occurs, it need use "return -1" instead of exit(1), so can
let upper caller has chance to print failure information, too, then user
can know the failure result more clearly.
xen_hvm_init() may also return -errno, which may let upper caller think
more (e.g. free some other related resources and try again), although at
present, all related upper callers still exit(1).
It is not a normal function which does not release related resources, if
return -1. So need give the related comments for it.
It passes common test under fedora 20 x86_64:
"./configure --enable-xen && make -j4 && make check"
execute result: "echo $? == 0".
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
xen-hvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen-hvm.c b/xen-hvm.c
index 0d09940..35efec0 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
}
+/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
MemoryRegion **ram_memory)
{
@@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
state->xenstore = xs_daemon_open();
if (state->xenstore == NULL) {
perror("xen: xenstore open");
+ xc_evtchn_close(state->xce_handle);
g_free(state);
return -errno;
}
@@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
/* Initialize backend core & drivers */
if (xen_be_init() != 0) {
fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
- exit(1);
+ return -1;
}
xen_be_register("console", &xen_console_ops);
xen_be_register("vkbd", &xen_kbdmouse_ops);
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
2014-09-02 16:22 ` Chen Gang
@ 2014-09-03 1:31 ` Chen Gang
-1 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-03 1:31 UTC (permalink / raw)
To: stefano.stabellini; +Cc: QEMU Trivial, xen-devel, qemu-devel
Oh, sorry, forgot Cc to qemu trivial.
Thanks.
On 9/3/14 0:22, Chen Gang wrote:
> When failure occurs, it need use "return -1" instead of exit(1), so can
> let upper caller has chance to print failure information, too, then user
> can know the failure result more clearly.
>
> xen_hvm_init() may also return -errno, which may let upper caller think
> more (e.g. free some other related resources and try again), although at
> present, all related upper callers still exit(1).
>
> It is not a normal function which does not release related resources, if
> return -1. So need give the related comments for it.
>
> It passes common test under fedora 20 x86_64:
>
> "./configure --enable-xen && make -j4 && make check"
> execute result: "echo $? == 0".
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> xen-hvm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 0d09940..35efec0 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
> xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
> }
>
> +/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> MemoryRegion **ram_memory)
> {
> @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> state->xenstore = xs_daemon_open();
> if (state->xenstore == NULL) {
> perror("xen: xenstore open");
> + xc_evtchn_close(state->xce_handle);
> g_free(state);
> return -errno;
> }
> @@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> /* Initialize backend core & drivers */
> if (xen_be_init() != 0) {
> fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
> - exit(1);
> + return -1;
> }
> xen_be_register("console", &xen_console_ops);
> xen_be_register("vkbd", &xen_kbdmouse_ops);
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
@ 2014-09-03 1:31 ` Chen Gang
0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-03 1:31 UTC (permalink / raw)
To: stefano.stabellini; +Cc: QEMU Trivial, xen-devel, qemu-devel
Oh, sorry, forgot Cc to qemu trivial.
Thanks.
On 9/3/14 0:22, Chen Gang wrote:
> When failure occurs, it need use "return -1" instead of exit(1), so can
> let upper caller has chance to print failure information, too, then user
> can know the failure result more clearly.
>
> xen_hvm_init() may also return -errno, which may let upper caller think
> more (e.g. free some other related resources and try again), although at
> present, all related upper callers still exit(1).
>
> It is not a normal function which does not release related resources, if
> return -1. So need give the related comments for it.
>
> It passes common test under fedora 20 x86_64:
>
> "./configure --enable-xen && make -j4 && make check"
> execute result: "echo $? == 0".
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> xen-hvm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 0d09940..35efec0 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
> xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
> }
>
> +/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> MemoryRegion **ram_memory)
> {
> @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> state->xenstore = xs_daemon_open();
> if (state->xenstore == NULL) {
> perror("xen: xenstore open");
> + xc_evtchn_close(state->xce_handle);
> g_free(state);
> return -errno;
> }
> @@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> /* Initialize backend core & drivers */
> if (xen_be_init() != 0) {
> fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
> - exit(1);
> + return -1;
> }
> xen_be_register("console", &xen_console_ops);
> xen_be_register("vkbd", &xen_kbdmouse_ops);
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
2014-09-03 1:31 ` Chen Gang
@ 2014-09-03 23:32 ` Stefano Stabellini
-1 siblings, 0 replies; 8+ messages in thread
From: Stefano Stabellini @ 2014-09-03 23:32 UTC (permalink / raw)
To: Chen Gang; +Cc: QEMU Trivial, xen-devel, qemu-devel, stefano.stabellini
On Wed, 3 Sep 2014, Chen Gang wrote:
> Oh, sorry, forgot Cc to qemu trivial.
>
> Thanks.
> On 9/3/14 0:22, Chen Gang wrote:
> > When failure occurs, it need use "return -1" instead of exit(1), so can
> > let upper caller has chance to print failure information, too, then user
> > can know the failure result more clearly.
good point
> > xen_hvm_init() may also return -errno, which may let upper caller think
> > more (e.g. free some other related resources and try again), although at
> > present, all related upper callers still exit(1).
I think we should return -1 in the other cases too
> > It is not a normal function which does not release related resources, if
> > return -1. So need give the related comments for it.
> >
> > It passes common test under fedora 20 x86_64:
> >
> > "./configure --enable-xen && make -j4 && make check"
> > execute result: "echo $? == 0".
> >
> > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > ---
> > xen-hvm.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen-hvm.c b/xen-hvm.c
> > index 0d09940..35efec0 100644
> > --- a/xen-hvm.c
> > +++ b/xen-hvm.c
> > @@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
> > xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
> > }
> >
> > +/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
> > int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > MemoryRegion **ram_memory)
> > {
> > @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > state->xenstore = xs_daemon_open();
> > if (state->xenstore == NULL) {
> > perror("xen: xenstore open");
> > + xc_evtchn_close(state->xce_handle);
> > g_free(state);
> > return -errno;
> > }
just return -1 and do the same in the above check (xce_handle ==
XC_HANDLER_INITIAL_VALUE)
> > @@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > /* Initialize backend core & drivers */
> > if (xen_be_init() != 0) {
> > fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
> > - exit(1);
> > + return -1;
> > }
> > xen_be_register("console", &xen_console_ops);
> > xen_be_register("vkbd", &xen_kbdmouse_ops);
> >
>
> --
> Chen Gang
>
> Open, share, and attitude like air, water, and life which God blessed
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
@ 2014-09-03 23:32 ` Stefano Stabellini
0 siblings, 0 replies; 8+ messages in thread
From: Stefano Stabellini @ 2014-09-03 23:32 UTC (permalink / raw)
To: Chen Gang; +Cc: QEMU Trivial, xen-devel, qemu-devel, stefano.stabellini
On Wed, 3 Sep 2014, Chen Gang wrote:
> Oh, sorry, forgot Cc to qemu trivial.
>
> Thanks.
> On 9/3/14 0:22, Chen Gang wrote:
> > When failure occurs, it need use "return -1" instead of exit(1), so can
> > let upper caller has chance to print failure information, too, then user
> > can know the failure result more clearly.
good point
> > xen_hvm_init() may also return -errno, which may let upper caller think
> > more (e.g. free some other related resources and try again), although at
> > present, all related upper callers still exit(1).
I think we should return -1 in the other cases too
> > It is not a normal function which does not release related resources, if
> > return -1. So need give the related comments for it.
> >
> > It passes common test under fedora 20 x86_64:
> >
> > "./configure --enable-xen && make -j4 && make check"
> > execute result: "echo $? == 0".
> >
> > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > ---
> > xen-hvm.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen-hvm.c b/xen-hvm.c
> > index 0d09940..35efec0 100644
> > --- a/xen-hvm.c
> > +++ b/xen-hvm.c
> > @@ -978,6 +978,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
> > xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
> > }
> >
> > +/* return value: 0 is OK, -errno is failure, -1 is critical issue -- exit(1) */
> > int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > MemoryRegion **ram_memory)
> > {
> > @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > state->xenstore = xs_daemon_open();
> > if (state->xenstore == NULL) {
> > perror("xen: xenstore open");
> > + xc_evtchn_close(state->xce_handle);
> > g_free(state);
> > return -errno;
> > }
just return -1 and do the same in the above check (xce_handle ==
XC_HANDLER_INITIAL_VALUE)
> > @@ -1069,7 +1071,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> > /* Initialize backend core & drivers */
> > if (xen_be_init() != 0) {
> > fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
> > - exit(1);
> > + return -1;
> > }
> > xen_be_register("console", &xen_console_ops);
> > xen_be_register("vkbd", &xen_kbdmouse_ops);
> >
>
> --
> Chen Gang
>
> Open, share, and attitude like air, water, and life which God blessed
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
2014-09-03 23:32 ` Stefano Stabellini
@ 2014-09-04 1:31 ` Chen Gang
-1 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-04 1:31 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: QEMU Trivial, xen-devel, qemu-devel
On 9/4/14 7:32, Stefano Stabellini wrote:
> On Wed, 3 Sep 2014, Chen Gang wrote:
>> On 9/3/14 0:22, Chen Gang wrote:
[...]
>>> xen_hvm_init() may also return -errno, which may let upper caller think
>>> more (e.g. free some other related resources and try again), although at
>>> present, all related upper callers still exit(1).
>
> I think we should return -1 in the other cases too
>
[...]
>>> @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
>>> state->xenstore = xs_daemon_open();
>>> if (state->xenstore == NULL) {
>>> perror("xen: xenstore open");
>>> + xc_evtchn_close(state->xce_handle);
>>> g_free(state);
>>> return -errno;
>>> }
>
> just return -1 and do the same in the above check (xce_handle ==
> XC_HANDLER_INITIAL_VALUE)
>
>
OK thanks, what you said sounds OK to me. I shall send patch v2 for it
within the 2 days (within 2014-09-05).
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init()
@ 2014-09-04 1:31 ` Chen Gang
0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-09-04 1:31 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: QEMU Trivial, xen-devel, qemu-devel
On 9/4/14 7:32, Stefano Stabellini wrote:
> On Wed, 3 Sep 2014, Chen Gang wrote:
>> On 9/3/14 0:22, Chen Gang wrote:
[...]
>>> xen_hvm_init() may also return -errno, which may let upper caller think
>>> more (e.g. free some other related resources and try again), although at
>>> present, all related upper callers still exit(1).
>
> I think we should return -1 in the other cases too
>
[...]
>>> @@ -998,6 +999,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
>>> state->xenstore = xs_daemon_open();
>>> if (state->xenstore == NULL) {
>>> perror("xen: xenstore open");
>>> + xc_evtchn_close(state->xce_handle);
>>> g_free(state);
>>> return -errno;
>>> }
>
> just return -1 and do the same in the above check (xce_handle ==
> XC_HANDLER_INITIAL_VALUE)
>
>
OK thanks, what you said sounds OK to me. I shall send patch v2 for it
within the 2 days (within 2014-09-05).
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-04 1:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-02 16:22 [Qemu-devel] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init() Chen Gang
2014-09-02 16:22 ` Chen Gang
2014-09-03 1:31 ` [Qemu-devel] " Chen Gang
2014-09-03 1:31 ` Chen Gang
2014-09-03 23:32 ` [Qemu-devel] " Stefano Stabellini
2014-09-03 23:32 ` Stefano Stabellini
2014-09-04 1:31 ` [Qemu-devel] " Chen Gang
2014-09-04 1:31 ` Chen Gang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.