[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] xen-hvm.c: Improve the return method for xen_
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-trivial] [PATCH] xen-hvm.c: Improve the return method for xen_hvm_init() |
Date: |
Thu, 4 Sep 2014 00:32:09 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
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 <address@hidden>
> > ---
> > 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
>