On Mon, 28 Apr 2014 17:54:46 +0000
Serge Hallyn <[email protected]> wrote:

> Quoting Dwight Engen ([email protected]):
> > This change accepts the same strings for lxc.arch that setarch(8)
> > does.
> > 
> > When outputting the setting, use i386 instead of x86 since the
> > later is not a valid input to setarch, nor will the kernel output
> > UTS_MACHINE as x86. The kernel sets utsname.machine to i[3456]86,
> > which are all map to PER_LINUX32.
> > 
> > Note that we continue to parse plain x86 as PER_LINUX32 so as not
> > to break existing lxc configuration files.
> > 
> > Signed-off-by: Dwight Engen <[email protected]>
> 
> Hi,
> 
> the first part (expanding the list of allowed inputs) seems fine, but
> the second part seems like it could break existing callers.

That's a good point. Using an older lxc version with a config that's
been cloned with a newer one won't work because the older one won't
parse i386. Maybe we should output i686 instead since older lxc's will
parse that.

Or are you thinking it is too dangerous that there are callers outside
of lxc that would be confused by seeing i686 instead of x86? I can drop
the output part, it just seemed a bit wrong that we're using a value
that isn't valid from the kernel nor for setarch.

> > ---
> >  src/lxc/confile.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index 90fb344..f03c191 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -1723,9 +1723,15 @@ signed long lxc_config_parse_arch(const char
> > *arch) struct per_name {
> >             char *name;
> >             unsigned long per;
> > -   } pername[4] = {
> > +   } pername[] = {
> >             { "x86", PER_LINUX32 },
> > +           { "linux32", PER_LINUX32 },
> > +           { "i386", PER_LINUX32 },
> > +           { "i486", PER_LINUX32 },
> > +           { "i586", PER_LINUX32 },
> >             { "i686", PER_LINUX32 },
> > +           { "athlon", PER_LINUX32 },
> > +           { "linux64", PER_LINUX },
> >             { "x86_64", PER_LINUX },
> >             { "amd64", PER_LINUX },
> >     };
> > @@ -1801,7 +1807,7 @@ static int lxc_get_arch_entry(struct lxc_conf
> > *c, char *retv, int inlen) int len = 0;
> >  
> >     switch(c->personality) {
> > -   case PER_LINUX32: strprint(retv, inlen, "x86"); break;
> > +   case PER_LINUX32: strprint(retv, inlen, "i386"); break;
> >     case PER_LINUX: strprint(retv, inlen, "x86_64"); break;
> >     default: break;
> >     }
> > @@ -2253,7 +2259,7 @@ void write_config(FILE *fout, struct lxc_conf
> > *c) fprintf(fout, "lxc.stopsignal = SIG%s\n",
> > sig_name(c->stopsignal)); #if HAVE_SYS_PERSONALITY_H
> >     switch(c->personality) {
> > -   case PER_LINUX32: fprintf(fout, "lxc.arch = x86\n"); break;
> > +   case PER_LINUX32: fprintf(fout, "lxc.arch = i386\n");
> > break; case PER_LINUX: fprintf(fout, "lxc.arch = x86_64\n"); break;
> >     default: break;
> >     }
> > -- 
> > 1.9.0
> > 
> > _______________________________________________
> > lxc-devel mailing list
> > [email protected]
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> [email protected]
> http://lists.linuxcontainers.org/listinfo/lxc-devel

_______________________________________________
lxc-devel mailing list
[email protected]
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to