* [Qemu-devel] [PATCH v4 0/2] vl: Deprecate auto-loading of qemu.conf @ 2017-10-04 2:50 Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost 0 siblings, 2 replies; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 2:50 UTC (permalink / raw) To: qemu-devel Cc: Markus Armbruster, Paolo Bonzini, Eric Blake, Daniel P. Berrange This missed v2.9 and v2.10. Let's try again: we can include this on v2.11, and remove the default config file in QEMU 2.13 or 2.14. Changes v3 -> v4: * Use warn_report() instead of error_report("warning: ...") (Eric Blake) * Document as a deprecated feature in qemu-doc.texi * Updated Subject line (was "vl: Print warning if a non-empty default config file is found") Changes v2 -> v3: * Rebase to latest qemu.git master Changes v1 -> v2: * Remove unnecessary translation of qemu_config_parse() erros to -EINVAL at block/blkdebug.c:read_config() * Suggsted-by: Markus Armbruster <armbru@redhat.com> We plan to remove support for /etc/qemu/qemu.conf in the near future. Make QEMU print a warning in case there a non-empty /etc/qemu/qemu.conf is loaded, so users have time to adapt. Eduardo Habkost (2): config: qemu_config_parse() return number of config groups vl: Deprecate auto-loading of qemu.conf block/blkdebug.c | 1 - util/qemu-config.c | 15 +++++++-------- vl.c | 6 ++++++ qemu-doc.texi | 8 ++++++++ 4 files changed, 21 insertions(+), 9 deletions(-) -- 2.13.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups 2017-10-04 2:50 [Qemu-devel] [PATCH v4 0/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost @ 2017-10-04 2:50 ` Eduardo Habkost 2017-10-04 20:41 ` Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost 1 sibling, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 2:50 UTC (permalink / raw) To: qemu-devel Cc: Markus Armbruster, Paolo Bonzini, Eric Blake, Daniel P. Berrange Change qemu_config_parse() to return the number of config groups in success and -EINVAL on error. This will allow callers of qemu_config_parse() to check if something was really loaded from the config file. All existing callers of qemu_config_parse() and qemu_read_config_file() only check if the return value was negative, so the change shouldn't affect them. Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes series v3 -> series v4: * (none) Changes v2 -> v3: * None (rebase only) Changes v1 -> v2: * Remove unnecessary translation of qemu_config_parse() erros to -EINVAL at block/blkdebug.c:read_config() * Suggsted-by: Markus Armbruster <armbru@redhat.com> --- block/blkdebug.c | 1 - util/qemu-config.c | 15 +++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 46e53f2f09..dfdf9b91aa 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -244,7 +244,6 @@ static int read_config(BDRVBlkdebugState *s, const char *filename, ret = qemu_config_parse(f, config_groups, filename); if (ret < 0) { error_setg(errp, "Could not parse blkdebug config file"); - ret = -EINVAL; goto fail; } } diff --git a/util/qemu-config.c b/util/qemu-config.c index 405dd1a1d7..99b0e46fa3 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -385,6 +385,7 @@ void qemu_config_write(FILE *fp) } } +/* Returns number of config groups on success, -errno on error */ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) { char line[1024], group[64], id[64], arg[64], value[1024]; @@ -392,7 +393,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) QemuOptsList *list = NULL; Error *local_err = NULL; QemuOpts *opts = NULL; - int res = -1, lno = 0; + int res = -EINVAL, lno = 0; + int count = 0; loc_push_none(&loc); while (fgets(line, sizeof(line), fp) != NULL) { @@ -413,6 +415,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) goto out; } opts = qemu_opts_create(list, id, 1, NULL); + count++; continue; } if (sscanf(line, "[%63[^]]]", group) == 1) { @@ -423,6 +426,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) goto out; } opts = qemu_opts_create(list, NULL, 0, &error_abort); + count++; continue; } value[0] = '\0'; @@ -447,7 +451,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) error_report("error reading file"); goto out; } - res = 0; + res = count; out: loc_pop(&loc); return res; @@ -464,12 +468,7 @@ int qemu_read_config_file(const char *filename) ret = qemu_config_parse(f, vm_config_groups, filename); fclose(f); - - if (ret == 0) { - return 0; - } else { - return -EINVAL; - } + return ret; } static void config_parse_qdict_section(QDict *options, QemuOptsList *opts, -- 2.13.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups Eduardo Habkost @ 2017-10-04 20:41 ` Eduardo Habkost 0 siblings, 0 replies; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 20:41 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster On Tue, Oct 03, 2017 at 11:50:42PM -0300, Eduardo Habkost wrote: > Change qemu_config_parse() to return the number of config groups > in success and -EINVAL on error. This will allow callers of > qemu_config_parse() to check if something was really loaded from > the config file. > > All existing callers of qemu_config_parse() and > qemu_read_config_file() only check if the return value was > negative, so the change shouldn't affect them. > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Paolo, if you are OK with this I'm queueing it (only 1/2 by now) on machine-next. -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-04 2:50 [Qemu-devel] [PATCH v4 0/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups Eduardo Habkost @ 2017-10-04 2:50 ` Eduardo Habkost 2017-10-04 5:42 ` Markus Armbruster 1 sibling, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 2:50 UTC (permalink / raw) To: qemu-devel Cc: Markus Armbruster, Paolo Bonzini, Eric Blake, Daniel P. Berrange In case there were options set in the default config file, print a warning so users can update their scripts. If somebody wants to keep the config file as-is, avoid the warning and use a command-line that will work in future QEMU versions, they can use: $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf I was going to include the suggestion in the warning message, but I thought it could make it more confusing. The suggestion is documented in qemu-doc.texi. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v3 -> v4: * Use warn_report() instead of error_report("warning: ...") (Eric Blake) * Document as a deprecated feature in qemu-doc.texi * Update subject line (was: "vl: Print warning when a default config file is loaded") Changes v2 -> v3: * Rebase (no code changes) * Commit message update: suggest -no-user-config --- vl.c | 6 ++++++ qemu-doc.texi | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/vl.c b/vl.c index 3fed457921..1b0ecdf74e 100644 --- a/vl.c +++ b/vl.c @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) return ret; } + if (ret > 0) { + loc_set_none(); + warn_report("Future QEMU versions won't load %s automatically", + CONFIG_QEMU_CONFDIR "/qemu.conf"); + } + return 0; } diff --git a/qemu-doc.texi b/qemu-doc.texi index ecd186a159..a81a09d05c 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -2370,6 +2370,14 @@ they were first deprecated in the 2.10.0 release. What follows is a list of all features currently marked as deprecated. +@section Automatic loading of @file{qemu.conf} (since 2.11.0) + +The automatic loading of an user-provided @file{qemu.conf} file from the QEMU +config directory is deprecated and behavior will change in future QEMU versions. +To load an user-provided @file{qemu.conf} file and keep compatibility with +future versions, the arguments @samp{-no-user-config -readconfig +@var{CONFDIR}/qemu.conf} may be used. + @section System emulator command line arguments @subsection -drive boot=on|off (since 1.3.0) -- 2.13.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost @ 2017-10-04 5:42 ` Markus Armbruster 2017-10-04 12:23 ` Eduardo Habkost 0 siblings, 1 reply; 9+ messages in thread From: Markus Armbruster @ 2017-10-04 5:42 UTC (permalink / raw) To: Eduardo Habkost; +Cc: qemu-devel, Paolo Bonzini Eduardo Habkost <ehabkost@redhat.com> writes: > In case there were options set in the default config file, print > a warning so users can update their scripts. > > If somebody wants to keep the config file as-is, avoid the > warning and use a command-line that will work in future QEMU > versions, they can use: > > $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf > > I was going to include the suggestion in the warning message, but > I thought it could make it more confusing. The suggestion is > documented in qemu-doc.texi. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v3 -> v4: > * Use warn_report() instead of error_report("warning: ...") > (Eric Blake) > * Document as a deprecated feature in qemu-doc.texi > * Update subject line > (was: "vl: Print warning when a default config file is loaded") > > Changes v2 -> v3: > * Rebase (no code changes) > * Commit message update: suggest -no-user-config > --- > vl.c | 6 ++++++ > qemu-doc.texi | 8 ++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/vl.c b/vl.c > index 3fed457921..1b0ecdf74e 100644 > --- a/vl.c > +++ b/vl.c > @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) > return ret; > } > > + if (ret > 0) { > + loc_set_none(); Sure we need this here? > + warn_report("Future QEMU versions won't load %s automatically", > + CONFIG_QEMU_CONFDIR "/qemu.conf"); > + } > + > return 0; > } > > diff --git a/qemu-doc.texi b/qemu-doc.texi > index ecd186a159..a81a09d05c 100644 > --- a/qemu-doc.texi > +++ b/qemu-doc.texi > @@ -2370,6 +2370,14 @@ they were first deprecated in the 2.10.0 release. > What follows is a list of all features currently marked as > deprecated. > > +@section Automatic loading of @file{qemu.conf} (since 2.11.0) > + > +The automatic loading of an user-provided @file{qemu.conf} file from the QEMU > +config directory is deprecated and behavior will change in future QEMU versions. > +To load an user-provided @file{qemu.conf} file and keep compatibility with > +future versions, the arguments @samp{-no-user-config -readconfig > +@var{CONFDIR}/qemu.conf} may be used. > + > @section System emulator command line arguments > > @subsection -drive boot=on|off (since 1.3.0) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-04 5:42 ` Markus Armbruster @ 2017-10-04 12:23 ` Eduardo Habkost 2017-10-04 20:57 ` Eduardo Habkost 0 siblings, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 12:23 UTC (permalink / raw) To: Markus Armbruster; +Cc: qemu-devel, Paolo Bonzini On Wed, Oct 04, 2017 at 07:42:17AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > In case there were options set in the default config file, print > > a warning so users can update their scripts. > > > > If somebody wants to keep the config file as-is, avoid the > > warning and use a command-line that will work in future QEMU > > versions, they can use: > > > > $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf > > > > I was going to include the suggestion in the warning message, but > > I thought it could make it more confusing. The suggestion is > > documented in qemu-doc.texi. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > Changes v3 -> v4: > > * Use warn_report() instead of error_report("warning: ...") > > (Eric Blake) > > * Document as a deprecated feature in qemu-doc.texi > > * Update subject line > > (was: "vl: Print warning when a default config file is loaded") > > > > Changes v2 -> v3: > > * Rebase (no code changes) > > * Commit message update: suggest -no-user-config > > --- > > vl.c | 6 ++++++ > > qemu-doc.texi | 8 ++++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/vl.c b/vl.c > > index 3fed457921..1b0ecdf74e 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) > > return ret; > > } > > > > + if (ret > 0) { > > + loc_set_none(); > > Sure we need this here? IIRC we needed it in the original version, but I don't remember why. I will check this. > > > + warn_report("Future QEMU versions won't load %s automatically", > > + CONFIG_QEMU_CONFDIR "/qemu.conf"); > > + } > > + > > return 0; > > } > > > > diff --git a/qemu-doc.texi b/qemu-doc.texi > > index ecd186a159..a81a09d05c 100644 > > --- a/qemu-doc.texi > > +++ b/qemu-doc.texi > > @@ -2370,6 +2370,14 @@ they were first deprecated in the 2.10.0 release. > > What follows is a list of all features currently marked as > > deprecated. > > > > +@section Automatic loading of @file{qemu.conf} (since 2.11.0) > > + > > +The automatic loading of an user-provided @file{qemu.conf} file from the QEMU > > +config directory is deprecated and behavior will change in future QEMU versions. > > +To load an user-provided @file{qemu.conf} file and keep compatibility with > > +future versions, the arguments @samp{-no-user-config -readconfig > > +@var{CONFDIR}/qemu.conf} may be used. > > + > > @section System emulator command line arguments > > > > @subsection -drive boot=on|off (since 1.3.0) -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-04 12:23 ` Eduardo Habkost @ 2017-10-04 20:57 ` Eduardo Habkost 2017-10-05 5:00 ` Markus Armbruster 0 siblings, 1 reply; 9+ messages in thread From: Eduardo Habkost @ 2017-10-04 20:57 UTC (permalink / raw) To: Markus Armbruster; +Cc: Paolo Bonzini, qemu-devel On Wed, Oct 04, 2017 at 09:23:08AM -0300, Eduardo Habkost wrote: > On Wed, Oct 04, 2017 at 07:42:17AM +0200, Markus Armbruster wrote: > > Eduardo Habkost <ehabkost@redhat.com> writes: > > > > > In case there were options set in the default config file, print > > > a warning so users can update their scripts. > > > > > > If somebody wants to keep the config file as-is, avoid the > > > warning and use a command-line that will work in future QEMU > > > versions, they can use: > > > > > > $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf > > > > > > I was going to include the suggestion in the warning message, but > > > I thought it could make it more confusing. The suggestion is > > > documented in qemu-doc.texi. > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > Changes v3 -> v4: > > > * Use warn_report() instead of error_report("warning: ...") > > > (Eric Blake) > > > * Document as a deprecated feature in qemu-doc.texi > > > * Update subject line > > > (was: "vl: Print warning when a default config file is loaded") > > > > > > Changes v2 -> v3: > > > * Rebase (no code changes) > > > * Commit message update: suggest -no-user-config > > > --- > > > vl.c | 6 ++++++ > > > qemu-doc.texi | 8 ++++++++ > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/vl.c b/vl.c > > > index 3fed457921..1b0ecdf74e 100644 > > > --- a/vl.c > > > +++ b/vl.c > > > @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) > > > return ret; > > > } > > > > > > + if (ret > 0) { > > > + loc_set_none(); > > > > Sure we need this here? > > IIRC we needed it in the original version, but I don't remember > why. I will check this. This is the result if we don't clear error location state: $ ./x86_64-softmmu/qemu-system-x86_64 -monitor stdio -display none -device foobar qemu-system-x86_64: -device foobar: warning: Future QEMU versions won't load /usr/local/etc/qemu/qemu.conf automatically ^^^^^^^^^^^^^^ However, it's probably better to do this right after the loop, just like we already do in the second option parsing loop: Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- diff --git a/vl.c b/vl.c index f9acc17c01..a8fd247d71 100644 --- a/vl.c +++ b/vl.c @@ -3067,7 +3067,6 @@ static int qemu_read_default_config_file(void) } if (ret > 0) { - loc_set_none(); warn_report("Future QEMU versions won't load %s automatically", CONFIG_QEMU_CONFDIR "/qemu.conf"); } @@ -3224,6 +3223,11 @@ int main(int argc, char **argv, char **envp) } } } + /* + * Clear error location left behind by the loop. + * Best done right after the loop. Do not insert code here! + */ + loc_set_none(); if (userconfig) { if (qemu_read_default_config_file() < 0) { -- Eduardo ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-04 20:57 ` Eduardo Habkost @ 2017-10-05 5:00 ` Markus Armbruster 2017-10-05 12:34 ` Eduardo Habkost 0 siblings, 1 reply; 9+ messages in thread From: Markus Armbruster @ 2017-10-05 5:00 UTC (permalink / raw) To: Eduardo Habkost; +Cc: Paolo Bonzini, qemu-devel Eduardo Habkost <ehabkost@redhat.com> writes: > On Wed, Oct 04, 2017 at 09:23:08AM -0300, Eduardo Habkost wrote: >> On Wed, Oct 04, 2017 at 07:42:17AM +0200, Markus Armbruster wrote: >> > Eduardo Habkost <ehabkost@redhat.com> writes: >> > >> > > In case there were options set in the default config file, print >> > > a warning so users can update their scripts. >> > > >> > > If somebody wants to keep the config file as-is, avoid the >> > > warning and use a command-line that will work in future QEMU >> > > versions, they can use: >> > > >> > > $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf >> > > >> > > I was going to include the suggestion in the warning message, but >> > > I thought it could make it more confusing. The suggestion is >> > > documented in qemu-doc.texi. >> > > >> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >> > > --- >> > > Changes v3 -> v4: >> > > * Use warn_report() instead of error_report("warning: ...") >> > > (Eric Blake) >> > > * Document as a deprecated feature in qemu-doc.texi >> > > * Update subject line >> > > (was: "vl: Print warning when a default config file is loaded") >> > > >> > > Changes v2 -> v3: >> > > * Rebase (no code changes) >> > > * Commit message update: suggest -no-user-config >> > > --- >> > > vl.c | 6 ++++++ >> > > qemu-doc.texi | 8 ++++++++ >> > > 2 files changed, 14 insertions(+) >> > > >> > > diff --git a/vl.c b/vl.c >> > > index 3fed457921..1b0ecdf74e 100644 >> > > --- a/vl.c >> > > +++ b/vl.c >> > > @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) >> > > return ret; >> > > } >> > > >> > > + if (ret > 0) { >> > > + loc_set_none(); >> > >> > Sure we need this here? >> >> IIRC we needed it in the original version, but I don't remember >> why. I will check this. > > This is the result if we don't clear error location state: > > $ ./x86_64-softmmu/qemu-system-x86_64 -monitor stdio -display none -device foobar > qemu-system-x86_64: -device foobar: warning: Future QEMU versions won't load /usr/local/etc/qemu/qemu.conf automatically > ^^^^^^^^^^^^^^ > > However, it's probably better to do this right after the loop, > just like we already do in the second option parsing loop: > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > diff --git a/vl.c b/vl.c > index f9acc17c01..a8fd247d71 100644 > --- a/vl.c > +++ b/vl.c > @@ -3067,7 +3067,6 @@ static int qemu_read_default_config_file(void) > } > > if (ret > 0) { > - loc_set_none(); > warn_report("Future QEMU versions won't load %s automatically", > CONFIG_QEMU_CONFDIR "/qemu.conf"); > } > @@ -3224,6 +3223,11 @@ int main(int argc, char **argv, char **envp) > } > } > } > + /* > + * Clear error location left behind by the loop. > + * Best done right after the loop. Do not insert code here! > + */ > + loc_set_none(); > > if (userconfig) { > if (qemu_read_default_config_file() < 0) { Hmm, could this hunk be a bug fix? Consider before the patch: qemu_read_default_config_file() qemu_read_config_file() qemu_config_parse() error_report() or error_report_err() I suspect this uses current location left behind by the loop inappropriately. If true, the hunk should be a separate bug fix patch. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf 2017-10-05 5:00 ` Markus Armbruster @ 2017-10-05 12:34 ` Eduardo Habkost 0 siblings, 0 replies; 9+ messages in thread From: Eduardo Habkost @ 2017-10-05 12:34 UTC (permalink / raw) To: Markus Armbruster; +Cc: Paolo Bonzini, qemu-devel On Thu, Oct 05, 2017 at 07:00:33AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Wed, Oct 04, 2017 at 09:23:08AM -0300, Eduardo Habkost wrote: > >> On Wed, Oct 04, 2017 at 07:42:17AM +0200, Markus Armbruster wrote: > >> > Eduardo Habkost <ehabkost@redhat.com> writes: > >> > > >> > > In case there were options set in the default config file, print > >> > > a warning so users can update their scripts. > >> > > > >> > > If somebody wants to keep the config file as-is, avoid the > >> > > warning and use a command-line that will work in future QEMU > >> > > versions, they can use: > >> > > > >> > > $QEMU -no-user-config -readconfig /etc/qemu/qemu.conf > >> > > > >> > > I was going to include the suggestion in the warning message, but > >> > > I thought it could make it more confusing. The suggestion is > >> > > documented in qemu-doc.texi. > >> > > > >> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > >> > > --- > >> > > Changes v3 -> v4: > >> > > * Use warn_report() instead of error_report("warning: ...") > >> > > (Eric Blake) > >> > > * Document as a deprecated feature in qemu-doc.texi > >> > > * Update subject line > >> > > (was: "vl: Print warning when a default config file is loaded") > >> > > > >> > > Changes v2 -> v3: > >> > > * Rebase (no code changes) > >> > > * Commit message update: suggest -no-user-config > >> > > --- > >> > > vl.c | 6 ++++++ > >> > > qemu-doc.texi | 8 ++++++++ > >> > > 2 files changed, 14 insertions(+) > >> > > > >> > > diff --git a/vl.c b/vl.c > >> > > index 3fed457921..1b0ecdf74e 100644 > >> > > --- a/vl.c > >> > > +++ b/vl.c > >> > > @@ -3066,6 +3066,12 @@ static int qemu_read_default_config_file(void) > >> > > return ret; > >> > > } > >> > > > >> > > + if (ret > 0) { > >> > > + loc_set_none(); > >> > > >> > Sure we need this here? > >> > >> IIRC we needed it in the original version, but I don't remember > >> why. I will check this. > > > > This is the result if we don't clear error location state: > > > > $ ./x86_64-softmmu/qemu-system-x86_64 -monitor stdio -display none -device foobar > > qemu-system-x86_64: -device foobar: warning: Future QEMU versions won't load /usr/local/etc/qemu/qemu.conf automatically > > ^^^^^^^^^^^^^^ > > > > However, it's probably better to do this right after the loop, > > just like we already do in the second option parsing loop: > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > diff --git a/vl.c b/vl.c > > index f9acc17c01..a8fd247d71 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -3067,7 +3067,6 @@ static int qemu_read_default_config_file(void) > > } > > > > if (ret > 0) { > > - loc_set_none(); > > warn_report("Future QEMU versions won't load %s automatically", > > CONFIG_QEMU_CONFDIR "/qemu.conf"); > > } > > @@ -3224,6 +3223,11 @@ int main(int argc, char **argv, char **envp) > > } > > } > > } > > + /* > > + * Clear error location left behind by the loop. > > + * Best done right after the loop. Do not insert code here! > > + */ > > + loc_set_none(); > > > > if (userconfig) { > > if (qemu_read_default_config_file() < 0) { > > Hmm, could this hunk be a bug fix? Consider before the patch: > > qemu_read_default_config_file() > qemu_read_config_file() > qemu_config_parse() > error_report() or error_report_err() > > I suspect this uses current location left behind by the loop > inappropriately. If true, the hunk should be a separate bug fix patch. It does, but nothing was broken because the only thing between the two option parsing loops is qemu_read_default_config_file(), and qemu_config_parse() calls loc_push_none() immediately. But it makes sense as a separate patch anyway. I will send a new series. -- Eduardo ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-10-05 12:34 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-04 2:50 [Qemu-devel] [PATCH v4 0/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 1/2] config: qemu_config_parse() return number of config groups Eduardo Habkost 2017-10-04 20:41 ` Eduardo Habkost 2017-10-04 2:50 ` [Qemu-devel] [PATCH v4 2/2] vl: Deprecate auto-loading of qemu.conf Eduardo Habkost 2017-10-04 5:42 ` Markus Armbruster 2017-10-04 12:23 ` Eduardo Habkost 2017-10-04 20:57 ` Eduardo Habkost 2017-10-05 5:00 ` Markus Armbruster 2017-10-05 12:34 ` Eduardo Habkost
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.