[lxc-devel] [PATCH] lxc_create: add outfile option

[Dwight Engen]

On Thu, 11 Jul 2013 15:40:10 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> Quoting Dwight Engen (dwight.engen at oracle.com):
> > On Thu, 11 Jul 2013 14:48:21 -0500
> > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > 
> > > lxc-create ... -o -
> > > 
> > > will send template output to standard output (the old default).
> > > 
> > > lxc-create ... without any -o will hide template output.
> > > 
> > > lxc-create -o /tmp/xxx will send template output to the file
> > > /tmp/xxx.
> > 
> > I like having a -o for lxc-create, but I'm not sure we should
> > change the
> 
> Sorry, that was wrong in my commit msg.
> 
> only '--outfile' works (because 'o' is used by the common options for
> logfile output)
> 
> > default of lxc-create to not show the template output. For
> > interactive
> 
> We could swap the meanings so that '--outfile -' means on output, and
> not listing '--outfile' shows the output on stdout.

Sorry, I'm not getting the difference between --outfile - and just
getting output on stdout? My main concern was that lxc-create without
any extra args do what it does today (shows output).
 
> > use it can be quite useful, whereas I think silent certainly makes
> > sense for the API (which is what I think Çağlars original concern
> > was).
> 
> Oh, hm, yeah.  So my approach actually doesn't suffice.  I was
> thinking callers could do the same thing, but that's somewhat
> silly.
> 
> > Maybe the API should take fds which we can dup2() onto 0,1,2 of the
> > forked create process to handle both cases? (and would allow the
> > API to capture the output if so desired)
> 
> pass in an int* which is either NULL or contains 3 ints (i.e.
> -1, 10, 10 if 10 is an output file)?

Yeah I think that would work, or 3 separate ints like we do for
lxcapi_console(). Either way, if the caller passes -1, does that mean we
would use the callers existing stdin,out,err i.e. don't dup2() that fd
at all, or that we will ensure stdin,out,err are /dev/null? The later is
probably easier for API callers and lxc-create can just pass the fd's
it wants.

> > On a related note, I guess we should make sure that all of the
> > templates are non-interactive, especially if we're going to give
> > them /dev/zero for stdin. Just thought I'd mention it given that the
> > ssh one for example was asking for a passphrase until just
> > recently ;)
> > 
> > > Reported-by: "S.Çağlar Onur" <caglar at 10ur.org>
> > > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> > > ---
> > >  src/lxc/arguments.h  |    2 +-
> > >  src/lxc/lxc_create.c |   22 ++++++++++++++++++++++
> > >  2 files changed, 23 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/lxc/arguments.h b/src/lxc/arguments.h
> > > index 3d1df70..6ab88fc 100644
> > > --- a/src/lxc/arguments.h
> > > +++ b/src/lxc/arguments.h
> > > @@ -81,7 +81,7 @@ struct lxc_arguments {
> > >  	char *fstype;
> > >  	unsigned long fssize;
> > >  	char *lvname, *vgname;
> > > -	char *zfsroot, *lowerdir, *dir;
> > > +	char *zfsroot, *lowerdir, *dir, *outfile;
> > >  
> > >  	/* remaining arguments */
> > >  	char *const *argv;
> > > diff --git a/src/lxc/lxc_create.c b/src/lxc/lxc_create.c
> > > index 6d8ca01..32805f4 100644
> > > --- a/src/lxc/lxc_create.c
> > > +++ b/src/lxc/lxc_create.c
> > > @@ -68,6 +68,7 @@ static int my_parser(struct lxc_arguments* args,
> > > int c, char* arg) case '3': args->fssize = get_fssize(arg); break;
> > >  	case '4': args->zfsroot = arg; break;
> > >  	case '5': args->dir = arg; break;
> > > +	case '6': args->outfile = arg; break;
> > >  	}
> > >  	return 0;
> > >  }
> > > @@ -82,6 +83,7 @@ static const struct option my_longopts[] = {
> > >  	{"fssize", required_argument, 0, '3'},
> > >  	{"zfsroot", required_argument, 0, '4'},
> > >  	{"dir", required_argument, 0, '5'},
> > > +	{"outfile", required_argument, 0, '6'},
> > >  	LXC_COMMON_OPTIONS
> > >  };
> > >  
> > > @@ -134,6 +136,9 @@ Options :\n\
> > >    --fssize=SIZE     Create filesystem of size SIZE\n\
> > >                      (Default: 1G))\n\
> > >    --dir=DIR         Place rootfs directory under DIR\n\
> > > +  --outfile=FILE    Print template output to FILE\n\
> > > +                    Use '-' to indicate stdout\n\
> > > +                    If unspecified, output is hidden\n\
> > >    --zfsroot=PATH    Create zfs under given zfsroot\n\
> > >                      (Default: tank/lxc))\n",
> > >  	.options  = my_longopts,
> > > @@ -228,6 +233,23 @@ int main(int argc, char *argv[])
> > >  
> > >  	if (strcmp(my_args.bdevtype, "_unset") == 0)
> > >  		my_args.bdevtype = NULL;
> > > +
> > > +	/* redirect output if requested */
> > > +	if (!my_args.outfile) {
> > > +		close(0); close(1); close(2);
> > > +		open("/dev/zero", O_RDONLY);
> > > +		open("/dev/null", O_RDWR);
> > > +		open("/dev/null", O_RDWR);
> > > +	} else if (strcmp(my_args.outfile, "-") != 0) {
> > > +		mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP;
> > > +		close(0); close(1); close(2);
> > > +		open("/dev/zero", O_RDONLY);
> > > +		if (open(my_args.outfile, O_RDWR | O_CREAT,
> > > mode) < 0 ||
> > > +		    open(my_args.outfile, O_RDWR) < 0) {
> > > +			SYSERROR("Error opening %s\n",
> > > my_args.outfile);
> > > +			exit(1);
> > > +		}
> > > +	}
> > >  	if (!c->create(c, my_args.template, my_args.bdevtype,
> > > &spec, &argv[optind])) { ERROR("Error creating container %s",
> > > c->name); lxc_container_put(c);
> >