[Xen-devel] Re: [PATCH,RFC]: Introduce libxl_domain_create()

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Gianni Tedesco writes ("[PATCH,RFC]: Introduce libxl_domain_create()"):
> Any thoughts or suggestions?

Thanks.  It seems to be roughly along the right lines.

I'll put my comments inline ...

>  /* domain related functions */
> +#define LIBXL_CREATE_RESTORE            (1<<0)
> +#define LIBXL_CREATE_RUN_BOOTLOADER     (1<<1)
> +#define LIBXL_CREATE_CONSOLE_CONNECT    (1<<2)

If we don't say CREATE_RUN_BOOTLOADER, where or when does the
bootloader run ?

I'm not sure how a daemon caller of the library supposed to deal with
this.  Supppose the library caller is virt-manager, which is a
daemon.  Is the bootloader going to run with no console connected, or
what ?

> +#define MUST( call ) ({                                                 \
> +        int must_rc = (call);                                           \
> +        if (must_rc < 0) {                                                  \
> +            fprintf(stderr,"xl: fatal error: %s:%d, rc=%d: %s\n",       \
> +                    __FILE__,__LINE__, must_rc, #call);                 \
> +            exit(-must_rc);                                             \
> +        }                                                               \
> +    })

As a library function, the error handling needs to be changed.  It
needs to not cause your whole process to exit.  Instead, it should
return an error to the caller.  And error messages should go to the
log.

> +    console->output = strdup("pty");

I think this should be some kind of checked strdup, surely ?

> +#if 0
>  waitpid_out:
>      if (child_console_pid > 0 &&
>              waitpid(child_console_pid, &status, 0) < 0 && errno == EINTR)
>          goto waitpid_out;
> +#endif

Uh ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel