WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH,RFC]: Introduce libxl_domain_create()
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Tue, 21 Dec 2010 08:35:14 +0000
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Tue, 21 Dec 2010 00:36:38 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <19719.51049.147431.357608@xxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1292353212.11246.35.camel@xxxxxxxxxxxxxxxxxxxxxx> <19719.51049.147431.357608@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Tue, 2010-12-14 at 19:37 +0000, Ian Jackson wrote:
> 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 ?

AFAICT this is just so that we don't run the bootloader during
resume/migrate-receive. I've been a bit of a scaredy cat and left
seperate flags for each bit of logic that was in the original function.

To be honest, I'm not sure I want to stick with the flags based API.

> > +#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.

Well spotted

> > +    console->output = strdup("pty");
> 
> I think this should be some kind of checked strdup, surely ?

Again, yes.

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

Heh, actualyl I wasn't sure if this was needed or not but I think it is
if we had run the daemon. Need to fix that.

Gianni


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