[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH LIBVIRT v2] libxl: Support cmdline= in xl config files
On Wed, 2016-01-20 at 11:16 -0700, Jim Fehlig wrote: > Ian Campbell wrote: > > ... and consolidate the cmdline/extra/root parsing to facilitate doing > > so. > > > > The logic is the same as xl's parse_cmdline from the current xen.git > > master > > branch (e6f0e099d2c17de47fd86e817b1998db903cab61). > > > > In order to introduce a use of VIR_WARN for logging I had to add > > virerror.h and VIR_LOG_INIT. > > > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > > --- > > NB: I was unsure if I was supposed to change VIR_FROM_NONE into > > VIR_FROM_XEN, so I didn't (but will on advice). > > It seems some of the VIR_FROM_ needs cleanup throughout the various > Xen-related > files. We already have VIR_FROM_SEXPR and VIR_FROM_XENXM. > src/xenconfig/xen_sxpr.c should use the former, src/xenconfig/xen_xm.c the > latter. And given the pattern, I think we should introduce VIR_FROM_XENXL to > cover xl.cfg related code. I can take care of this. I've acked you patches about this. > > +static int xenParseCmdline(virConfPtr conf, char **r_cmdline) > > +{ > > +ÂÂÂÂchar *cmdline; > > One too many initializers removed :-). > > > +ÂÂÂÂconst char *root, *extra, *buf; > > + > > +ÂÂÂÂif (xenConfigGetString(conf, "cmdline", &buf, NULL) < 0) > > +ÂÂÂÂÂÂÂÂreturn -1; > > + > > +ÂÂÂÂif (xenConfigGetString(conf, "root", &root, NULL) < 0) > > +ÂÂÂÂÂÂÂÂreturn -1; > > + > > +ÂÂÂÂif (xenConfigGetString(conf, "extra", &extra, NULL) < 0) > > +ÂÂÂÂÂÂÂÂreturn -1; > > + > > +ÂÂÂÂif (buf) { > > +ÂÂÂÂÂÂÂÂif (VIR_STRDUP(cmdline, buf) < 0) > > +ÂÂÂÂÂÂÂÂÂÂÂÂreturn -1; > > +ÂÂÂÂÂÂÂÂif (root || extra) > > +ÂÂÂÂÂÂÂÂÂÂÂÂVIR_WARN("ignoring root= and extra= in favour of > > cmdline="); > > +ÂÂÂÂ} else { > > +ÂÂÂÂÂÂÂÂif (root && extra) { > > +ÂÂÂÂÂÂÂÂÂÂÂÂif (virAsprintf(&cmdline, "root=%s %s", root, extra) < 0) > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -1; > > +ÂÂÂÂÂÂÂÂ} else if (root) { > > +ÂÂÂÂÂÂÂÂÂÂÂÂif (virAsprintf(&cmdline, "root=%s", root) < 0) > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -1; > > +ÂÂÂÂÂÂÂÂ} else if (extra) { > > +ÂÂÂÂÂÂÂÂÂÂÂÂif (VIR_STRDUP(cmdline, extra) < 0) > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -1; > > +ÂÂÂÂÂÂÂÂ} > > +ÂÂÂÂ} > > + > > +ÂÂÂÂ*r_cmdline = cmdline; > > If none of cmdline, extra, or root are set in the config, def->os.cmdline gets > set to garbage. xlconfigtest explodes when running 'make check'. I even thought about this quite carefully but missed this case :-/ Would you prefer and = NULL on the decl or an else cmdline = NULL at the end of that chain? > Sorry for not mentioning it in my previous review, but we should add a > test for cmdline, root, and extra. Ack, will do so for v3. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |