* Re: [PATCH LIBVIRT] libxl: Support cmdline= in xl config files [not found] <1450267744-19273-1-git-send-email-ian.campbell@citrix.com> @ 2016-01-19 12:03 ` Ian Campbell 2016-01-20 4:46 ` Jim Fehlig [not found] ` <569F1133.3010501@suse.com> 0 siblings, 2 replies; 3+ messages in thread From: Ian Campbell @ 2016-01-19 12:03 UTC (permalink / raw) To: libvir-list, Jim Fehlig; +Cc: xen-devel I went to ping this but noticed that I had sent it to "jimfehlig" (i.e. no domain), so no wonder there was no reply! To: line fixed here, let me know if you would prefer a resend. Ian. On Wed, 2015-12-16 at 12:09 +0000, 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), except I was unable > to figure out how/where to route the warning about ignoring > root+extra if cmdline was specified. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > --- > src/xenconfig/xen_xl.c | 62 ++++++++++++++++++++++++++++++------------ > -------- > 1 file changed, 37 insertions(+), 25 deletions(-) > > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > index 91cdff6..ba8b938 100644 > --- a/src/xenconfig/xen_xl.c > +++ b/src/xenconfig/xen_xl.c > @@ -58,11 +58,45 @@ extern int xlu_disk_parse(XLU_Config *cfg, > libxl_device_disk *disk); > #endif > > +static int xenParseCmdline(virConfPtr conf, char **r_cmdline) > +{ > + char *cmdline = NULL; > + const char *root = NULL, *extra = NULL, *buf = NULL; > + > + 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; > + /* root or extra are ignored in this case. */ > + } 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; > + return 0; > +} > + > static int > xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) > { > size_t i; > - const char *extra, *root; > > if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { > const char *boot; > @@ -84,19 +118,8 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, > virCapsPtr caps) > if (xenConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd) < > 0) > return -1; > > - if (xenConfigGetString(conf, "extra", &extra, NULL) < 0) > - return -1; > - > - if (xenConfigGetString(conf, "root", &root, NULL) < 0) > + if (xenParseCmdline(conf, &def->os.cmdline) < 0) > return -1; > - > - if (root) { > - if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) > < 0) > - return -1; > - } else { > - if (VIR_STRDUP(def->os.cmdline, extra) < 0) > - return -1; > - } > #endif > > if (xenConfigGetString(conf, "boot", &boot, "c") < 0) > @@ -132,19 +155,8 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, > virCapsPtr caps) > if (xenConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd) < > 0) > return -1; > > - if (xenConfigGetString(conf, "extra", &extra, NULL) < 0) > - return -1; > - > - if (xenConfigGetString(conf, "root", &root, NULL) < 0) > + if (xenParseCmdline(conf, &def->os.cmdline) < 0) > return -1; > - > - if (root) { > - if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) > < 0) > - return -1; > - } else { > - if (VIR_STRDUP(def->os.cmdline, extra) < 0) > - return -1; > - } > } > > return 0; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH LIBVIRT] libxl: Support cmdline= in xl config files 2016-01-19 12:03 ` [PATCH LIBVIRT] libxl: Support cmdline= in xl config files Ian Campbell @ 2016-01-20 4:46 ` Jim Fehlig [not found] ` <569F1133.3010501@suse.com> 1 sibling, 0 replies; 3+ messages in thread From: Jim Fehlig @ 2016-01-20 4:46 UTC (permalink / raw) To: Ian Campbell, libvir-list; +Cc: xen-devel On 01/19/2016 05:03 AM, Ian Campbell wrote: > I went to ping this but noticed that I had sent it to "jimfehlig" (i.e. no > domain), so no wonder there was no reply! > > To: line fixed here, let me know if you would prefer a resend. That would be much appreciated, thanks! > > Ian. > > On Wed, 2015-12-16 at 12:09 +0000, 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), except I was unable >> to figure out how/where to route the warning about ignoring >> root+extra if cmdline was specified. I think VIR_WARN() would be appropriate. >> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >> --- >> src/xenconfig/xen_xl.c | 62 ++++++++++++++++++++++++++++++------------ >> -------- >> 1 file changed, 37 insertions(+), 25 deletions(-) >> >> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c >> index 91cdff6..ba8b938 100644 >> --- a/src/xenconfig/xen_xl.c >> +++ b/src/xenconfig/xen_xl.c >> @@ -58,11 +58,45 @@ extern int xlu_disk_parse(XLU_Config *cfg, >> libxl_device_disk *disk); >> #endif >> >> +static int xenParseCmdline(virConfPtr conf, char **r_cmdline) >> +{ >> + char *cmdline = NULL; >> + const char *root = NULL, *extra = NULL, *buf = NULL; In theory, these three don't need to be initialized since xenConfigGetString will do that. But in practice, I worry that Coverity might complain :-/. Regards, Jim >> + >> + 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; >> + /* root or extra are ignored in this case. */ >> + } 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; >> + return 0; >> +} >> + >> static int >> xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) >> { >> size_t i; >> - const char *extra, *root; >> >> if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { >> const char *boot; >> @@ -84,19 +118,8 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, >> virCapsPtr caps) >> if (xenConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd) < >> 0) >> return -1; >> >> - if (xenConfigGetString(conf, "extra", &extra, NULL) < 0) >> - return -1; >> - >> - if (xenConfigGetString(conf, "root", &root, NULL) < 0) >> + if (xenParseCmdline(conf, &def->os.cmdline) < 0) >> return -1; >> - >> - if (root) { >> - if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) >> < 0) >> - return -1; >> - } else { >> - if (VIR_STRDUP(def->os.cmdline, extra) < 0) >> - return -1; >> - } >> #endif >> >> if (xenConfigGetString(conf, "boot", &boot, "c") < 0) >> @@ -132,19 +155,8 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, >> virCapsPtr caps) >> if (xenConfigCopyStringOpt(conf, "ramdisk", &def->os.initrd) < >> 0) >> return -1; >> >> - if (xenConfigGetString(conf, "extra", &extra, NULL) < 0) >> - return -1; >> - >> - if (xenConfigGetString(conf, "root", &root, NULL) < 0) >> + if (xenParseCmdline(conf, &def->os.cmdline) < 0) >> return -1; >> - >> - if (root) { >> - if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) >> < 0) >> - return -1; >> - } else { >> - if (VIR_STRDUP(def->os.cmdline, extra) < 0) >> - return -1; >> - } >> } >> >> return 0; > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <569F1133.3010501@suse.com>]
* Re: [PATCH LIBVIRT] libxl: Support cmdline= in xl config files [not found] ` <569F1133.3010501@suse.com> @ 2016-01-20 10:43 ` Ian Campbell 0 siblings, 0 replies; 3+ messages in thread From: Ian Campbell @ 2016-01-20 10:43 UTC (permalink / raw) To: Jim Fehlig, libvir-list; +Cc: xen-devel On Tue, 2016-01-19 at 21:46 -0700, Jim Fehlig wrote: > On 01/19/2016 05:03 AM, Ian Campbell wrote: > > I went to ping this but noticed that I had sent it to "jimfehlig" (i.e. > > no > > domain), so no wonder there was no reply! > > > > To: line fixed here, let me know if you would prefer a resend. > > That would be much appreciated, thanks! > > > > > Ian. > > > > On Wed, 2015-12-16 at 12:09 +0000, 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), except I was > > > unable > > > to figure out how/where to route the warning about ignoring > > > root+extra if cmdline was specified. > > I think VIR_WARN() would be appropriate. Ok, will do, thanks. > > > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > --- > > > src/xenconfig/xen_xl.c | 62 ++++++++++++++++++++++++++++++-------- > > > ---- > > > -------- > > > 1 file changed, 37 insertions(+), 25 deletions(-) > > > > > > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > > > index 91cdff6..ba8b938 100644 > > > --- a/src/xenconfig/xen_xl.c > > > +++ b/src/xenconfig/xen_xl.c > > > @@ -58,11 +58,45 @@ extern int xlu_disk_parse(XLU_Config *cfg, > > > libxl_device_disk *disk); > > > #endif > > > > > > +static int xenParseCmdline(virConfPtr conf, char **r_cmdline) > > > +{ > > > + char *cmdline = NULL; > > > + const char *root = NULL, *extra = NULL, *buf = NULL; > > In theory, these three don't need to be initialized since > xenConfigGetString > will do that. But in practice, I worry that Coverity might complain :-/. It looks like some of the callers of xenConfigGetString initialise the value to NULL, while others don't. I can't see any public libvirt scan results to look if some of the ones which don't have been picked up or not. I've just noticed also that the code I am moving/removing didn't initialise to NULL, so I think I'll remove these initialisers. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-20 10:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1450267744-19273-1-git-send-email-ian.campbell@citrix.com>
2016-01-19 12:03 ` [PATCH LIBVIRT] libxl: Support cmdline= in xl config files Ian Campbell
2016-01-20 4:46 ` Jim Fehlig
[not found] ` <569F1133.3010501@suse.com>
2016-01-20 10:43 ` Ian Campbell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).