| Message ID | 20170117180051.11958-1-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 17/01/2017 19:00, Eduardo Habkost wrote: > The existing default_config_files table in arch_init.c has a > single entry, making it completely unnecessary. The whole code > can be replaced by a single qemu_read_config_file() call in vl.c. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > include/qemu/config-file.h | 4 ---- > arch_init.c | 27 --------------------------- > vl.c | 18 ++++++++++++++---- > 3 files changed, 14 insertions(+), 35 deletions(-) > > diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h > index 8d4b2b6d94..c80d5c8a33 100644 > --- a/include/qemu/config-file.h > +++ b/include/qemu/config-file.h > @@ -23,8 +23,4 @@ int qemu_read_config_file(const char *filename); > void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists, > Error **errp); > > -/* Read default QEMU config files > - */ > -int qemu_read_default_config_files(bool userconfig); > - > #endif /* QEMU_CONFIG_FILE_H */ > diff --git a/arch_init.c b/arch_init.c > index 5cc58b2c35..e9b6d62d18 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -84,33 +84,6 @@ int graphic_depth = 32; > > const uint32_t arch_type = QEMU_ARCH; > > -static struct defconfig_file { > - const char *filename; > - /* Indicates it is an user config file (disabled by -no-user-config) */ > - bool userconfig; > -} default_config_files[] = { > - { CONFIG_QEMU_CONFDIR "/qemu.conf", true }, > - { NULL }, /* end of list */ > -}; > - > -int qemu_read_default_config_files(bool userconfig) > -{ > - int ret; > - struct defconfig_file *f; > - > - for (f = default_config_files; f->filename; f++) { > - if (!userconfig && f->userconfig) { > - continue; > - } > - ret = qemu_read_config_file(f->filename); > - if (ret < 0 && ret != -ENOENT) { > - return ret; > - } > - } > - > - return 0; > -} > - > struct soundhw { > const char *name; > const char *descr; > diff --git a/vl.c b/vl.c > index c643d3ff3a..b563f9b924 100644 > --- a/vl.c > +++ b/vl.c > @@ -2990,6 +2990,18 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) > return 0; > } > > +static int qemu_read_default_config_file(void) > +{ > + int ret; > + > + ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf"); > + if (ret < 0 && ret != -ENOENT) { > + return ret; > + } > + > + return 0; > +} > + > int main(int argc, char **argv, char **envp) > { > int i; > @@ -3117,10 +3129,8 @@ int main(int argc, char **argv, char **envp) > } > } > > - if (defconfig) { > - int ret; > - ret = qemu_read_default_config_files(userconfig); > - if (ret < 0) { > + if (defconfig && userconfig) { > + if (qemu_read_default_config_file() < 0) { > exit(1); > } > } > Looks good! Please include it yourself in your next pull request. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Though maybe we should just remove .conf file support completely... who's using it?!? Paolo
On Wed, Jan 18, 2017 at 10:17:17AM +0100, Paolo Bonzini wrote: > > > On 17/01/2017 19:00, Eduardo Habkost wrote: > > The existing default_config_files table in arch_init.c has a > > single entry, making it completely unnecessary. The whole code > > can be replaced by a single qemu_read_config_file() call in vl.c. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > include/qemu/config-file.h | 4 ---- > > arch_init.c | 27 --------------------------- > > vl.c | 18 ++++++++++++++---- > > 3 files changed, 14 insertions(+), 35 deletions(-) > > > > diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h > > index 8d4b2b6d94..c80d5c8a33 100644 > > --- a/include/qemu/config-file.h > > +++ b/include/qemu/config-file.h > > @@ -23,8 +23,4 @@ int qemu_read_config_file(const char *filename); > > void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists, > > Error **errp); > > > > -/* Read default QEMU config files > > - */ > > -int qemu_read_default_config_files(bool userconfig); > > - > > #endif /* QEMU_CONFIG_FILE_H */ > > diff --git a/arch_init.c b/arch_init.c > > index 5cc58b2c35..e9b6d62d18 100644 > > --- a/arch_init.c > > +++ b/arch_init.c > > @@ -84,33 +84,6 @@ int graphic_depth = 32; > > > > const uint32_t arch_type = QEMU_ARCH; > > > > -static struct defconfig_file { > > - const char *filename; > > - /* Indicates it is an user config file (disabled by -no-user-config) */ > > - bool userconfig; > > -} default_config_files[] = { > > - { CONFIG_QEMU_CONFDIR "/qemu.conf", true }, > > - { NULL }, /* end of list */ > > -}; > > - > > -int qemu_read_default_config_files(bool userconfig) > > -{ > > - int ret; > > - struct defconfig_file *f; > > - > > - for (f = default_config_files; f->filename; f++) { > > - if (!userconfig && f->userconfig) { > > - continue; > > - } > > - ret = qemu_read_config_file(f->filename); > > - if (ret < 0 && ret != -ENOENT) { > > - return ret; > > - } > > - } > > - > > - return 0; > > -} > > - > > struct soundhw { > > const char *name; > > const char *descr; > > diff --git a/vl.c b/vl.c > > index c643d3ff3a..b563f9b924 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -2990,6 +2990,18 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) > > return 0; > > } > > > > +static int qemu_read_default_config_file(void) > > +{ > > + int ret; > > + > > + ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf"); > > + if (ret < 0 && ret != -ENOENT) { > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > int main(int argc, char **argv, char **envp) > > { > > int i; > > @@ -3117,10 +3129,8 @@ int main(int argc, char **argv, char **envp) > > } > > } > > > > - if (defconfig) { > > - int ret; > > - ret = qemu_read_default_config_files(userconfig); > > - if (ret < 0) { > > + if (defconfig && userconfig) { > > + if (qemu_read_default_config_file() < 0) { > > exit(1); > > } > > } > > > > Looks good! Please include it yourself in your next pull request. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Though maybe we should just remove .conf file support completely... > who's using it?!? You mean removing /etc/qemu.conf, or removing -readconfig completely? The former doesn't seem to be used often. The latter looks very useful for people trying to write scripts around QEMU and to avoid command-line length limits, so I will be surprised if nobody is using it.
On 18/01/2017 13:52, Eduardo Habkost wrote: >> >> Though maybe we should just remove .conf file support completely... >> who's using it?!? > You mean removing /etc/qemu.conf, or removing -readconfig > completely? > > The former doesn't seem to be used often. The latter looks very > useful for people trying to write scripts around QEMU and to > avoid command-line length limits, so I will be surprised if > nobody is using it. Of course /etc/qemu.conf only (so that -nodefconfig/-nouserconfig become no-ops). Paolo
On Wed, Jan 18, 2017 at 01:55:21PM +0100, Paolo Bonzini wrote: > > > On 18/01/2017 13:52, Eduardo Habkost wrote: > >> > >> Though maybe we should just remove .conf file support completely... > >> who's using it?!? > > You mean removing /etc/qemu.conf, or removing -readconfig > > completely? > > > > The former doesn't seem to be used often. The latter looks very > > useful for people trying to write scripts around QEMU and to > > avoid command-line length limits, so I will be surprised if > > nobody is using it. > > Of course /etc/qemu.conf only (so that -nodefconfig/-nouserconfig become > no-ops). Agreed. Should we print a warning in 2.9 and remove it in 2.10?
On Wed, Jan 18, 2017 at 10:17:17AM +0100, Paolo Bonzini wrote: > > > On 17/01/2017 19:00, Eduardo Habkost wrote: > > The existing default_config_files table in arch_init.c has a > > single entry, making it completely unnecessary. The whole code > > can be replaced by a single qemu_read_config_file() call in vl.c. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> [...] > > Looks good! Please include it yourself in your next pull request. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Applied to machine-next. Thanks!
On 18/01/2017 13:58, Eduardo Habkost wrote: > On Wed, Jan 18, 2017 at 01:55:21PM +0100, Paolo Bonzini wrote: >> >> >> On 18/01/2017 13:52, Eduardo Habkost wrote: >>>> >>>> Though maybe we should just remove .conf file support completely... >>>> who's using it?!? >>> You mean removing /etc/qemu.conf, or removing -readconfig >>> completely? >>> >>> The former doesn't seem to be used often. The latter looks very >>> useful for people trying to write scripts around QEMU and to >>> avoid command-line length limits, so I will be surprised if >>> nobody is using it. >> >> Of course /etc/qemu.conf only (so that -nodefconfig/-nouserconfig become >> no-ops). > > Agreed. Should we print a warning in 2.9 and remove it in 2.10? I would just remove it, but that sounds like a plan too. :) Paolo
diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h index 8d4b2b6d94..c80d5c8a33 100644 --- a/include/qemu/config-file.h +++ b/include/qemu/config-file.h @@ -23,8 +23,4 @@ int qemu_read_config_file(const char *filename); void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists, Error **errp); -/* Read default QEMU config files - */ -int qemu_read_default_config_files(bool userconfig); - #endif /* QEMU_CONFIG_FILE_H */ diff --git a/arch_init.c b/arch_init.c index 5cc58b2c35..e9b6d62d18 100644 --- a/arch_init.c +++ b/arch_init.c @@ -84,33 +84,6 @@ int graphic_depth = 32; const uint32_t arch_type = QEMU_ARCH; -static struct defconfig_file { - const char *filename; - /* Indicates it is an user config file (disabled by -no-user-config) */ - bool userconfig; -} default_config_files[] = { - { CONFIG_QEMU_CONFDIR "/qemu.conf", true }, - { NULL }, /* end of list */ -}; - -int qemu_read_default_config_files(bool userconfig) -{ - int ret; - struct defconfig_file *f; - - for (f = default_config_files; f->filename; f++) { - if (!userconfig && f->userconfig) { - continue; - } - ret = qemu_read_config_file(f->filename); - if (ret < 0 && ret != -ENOENT) { - return ret; - } - } - - return 0; -} - struct soundhw { const char *name; const char *descr; diff --git a/vl.c b/vl.c index c643d3ff3a..b563f9b924 100644 --- a/vl.c +++ b/vl.c @@ -2990,6 +2990,18 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) return 0; } +static int qemu_read_default_config_file(void) +{ + int ret; + + ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf"); + if (ret < 0 && ret != -ENOENT) { + return ret; + } + + return 0; +} + int main(int argc, char **argv, char **envp) { int i; @@ -3117,10 +3129,8 @@ int main(int argc, char **argv, char **envp) } } - if (defconfig) { - int ret; - ret = qemu_read_default_config_files(userconfig); - if (ret < 0) { + if (defconfig && userconfig) { + if (qemu_read_default_config_file() < 0) { exit(1); } }
The existing default_config_files table in arch_init.c has a single entry, making it completely unnecessary. The whole code can be replaced by a single qemu_read_config_file() call in vl.c. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/qemu/config-file.h | 4 ---- arch_init.c | 27 --------------------------- vl.c | 18 ++++++++++++++---- 3 files changed, 14 insertions(+), 35 deletions(-)