* [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
@ 2010-11-24 22:45 john cooper
0 siblings, 0 replies; 6+ messages in thread
From: john cooper @ 2010-11-24 22:45 UTC (permalink / raw)
To: qemu-devel; +Cc: john cooper, Anthony Liguori
Failure by qemu to open a default config file isn't cause to
error exit -- it just quietly continues on. After puzzling
issues with otherwise opaque config file locations and
startup handling numerous times, some help from qemu seemed
justified.
The prior version of this patch overloaded "-readconfig ?"
to enable verbose handling. Here we add a global "-config"
flag which takes [verbose][,nodefault] as options modifying
config file handling. Note "-nodefconfig" has been removed
and functionally replaced here by the "nodefault" flag option.
Signed-off-by: john cooper <john.cooper@redhat.com>
---
diff --git a/qemu-config.c b/qemu-config.c
index 52f18be..1b871d6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -630,21 +630,31 @@ out:
return res;
}
-int qemu_read_config_file(const char *filename)
+/* attempt to open and parse config file, report problems if vflag
+ */
+int qemu_read_config_file(const char *filename, int vflag)
{
FILE *f = fopen(filename, "r");
- int ret;
+ int rv = 0;
+ const char *err;
if (f == NULL) {
- return -errno;
+ rv = -errno;
+ err = "open";
}
-
- ret = qemu_config_parse(f, vm_config_groups, filename);
- fclose(f);
-
- if (ret == 0) {
- return 0;
- } else {
- return -EINVAL;
+ else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
+ rv = -EINVAL;
+ err = "parse";
+ }
+ else if (vflag) {
+ fprintf(stderr, "parsed config file %s\n", filename);
+ }
+ if (f) {
+ fclose(f);
+ }
+ if (rv && vflag) {
+ fprintf(stderr, "can't %s config file %s: %s\n",
+ err, filename, strerror(-rv));
}
+ return rv;
}
diff --git a/qemu-config.h b/qemu-config.h
index 20d707f..b90a7cc 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -14,6 +14,6 @@ void qemu_add_globals(void);
void qemu_config_write(FILE *fp);
int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
-int qemu_read_config_file(const char *filename);
+int qemu_read_config_file(const char *filename, int vflag);
#endif /* QEMU_CONFIG_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index 4d99a58..00c0845 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2290,16 +2290,19 @@ STEXI
@findex -writeconfig
Write device configuration to @var{file}.
ETEXI
-DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig,
- "-nodefconfig\n"
- " do not load default config files at startup\n",
+DEF("config", HAS_ARG, QEMU_OPTION_config,
+ "-config [nodefault][,verbose]\n"
+ " set global config file options.\n",
QEMU_ARCH_ALL)
STEXI
-@item -nodefconfig
-@findex -nodefconfig
-Normally QEMU loads a configuration file from @var{sysconfdir}/qemu.conf and
-@var{sysconfdir}/target-@var{ARCH}.conf on startup. The @code{-nodefconfig}
-option will prevent QEMU from loading these configuration files at startup.
+@item -config
+@findex -config
+By default QEMU loads a configuration file from @var{sysconfdir}/qemu.conf and
+@var{sysconfdir}/target-@var{ARCH}.conf on startup.
+The @code{-config nodefault} option will prevent QEMU from loading these
+configuration files at startup.
+Additionally @code{-config verbose} will provide detail to the user of
+all config file handling.
ETEXI
#ifdef CONFIG_SIMPLE_TRACE
DEF("trace", HAS_ARG, QEMU_OPTION_trace,
diff --git a/vl.c b/vl.c
index 805e11f..160f14b 100644
--- a/vl.c
+++ b/vl.c
@@ -1806,6 +1806,7 @@ int main(int argc, char **argv, char **envp)
const char *incoming = NULL;
int show_vnc_port = 0;
int defconfig = 1;
+ int defconfig_verbose = 0;
#ifdef CONFIG_SIMPLE_TRACE
const char *trace_file = NULL;
@@ -1854,8 +1855,25 @@ int main(int argc, char **argv, char **envp)
popt = lookup_opt(argc, argv, &optarg, &optind);
switch (popt->index) {
- case QEMU_OPTION_nodefconfig:
- defconfig=0;
+ case QEMU_OPTION_config:
+ {
+ char *p, *q, *r;
+
+ for (r = p = strdup(optarg); (q = strtok(r, ",")); r = NULL)
+ if (!strcmp(q, "nodefault")) {
+ defconfig = 0;
+ } else if (!strcmp(q, "verbose")) {
+ defconfig_verbose = 1;
+ } else {
+ fprintf(stderr, "unknown option \"%s\"\n", q);
+ exit(1);
+ }
+ free(p);
+ break;
+ }
+ case QEMU_OPTION_readconfig:
+ if (qemu_read_config_file(optarg, defconfig_verbose) < 0)
+ exit(1);
break;
}
}
@@ -1864,12 +1882,13 @@ int main(int argc, char **argv, char **envp)
if (defconfig) {
int ret;
- ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+ ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
+ defconfig_verbose);
if (ret < 0 && ret != -ENOENT) {
exit(1);
}
- ret = qemu_read_config_file(arch_config_name);
+ ret = qemu_read_config_file(arch_config_name, defconfig_verbose);
if (ret < 0 && ret != -ENOENT) {
exit(1);
}
@@ -2577,16 +2596,6 @@ int main(int argc, char **argv, char **envp)
}
break;
#endif
- case QEMU_OPTION_readconfig:
- {
- int ret = qemu_read_config_file(optarg);
- if (ret < 0) {
- fprintf(stderr, "read config %s: %s\n", optarg,
- strerror(-ret));
- exit(1);
- }
- break;
- }
case QEMU_OPTION_spice:
olist = qemu_find_opts("spice");
if (!olist) {
--
john.cooper@redhat.com
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
@ 2010-09-07 12:31 john cooper
2010-09-07 18:38 ` Blue Swirl
0 siblings, 1 reply; 6+ messages in thread
From: john cooper @ 2010-09-07 12:31 UTC (permalink / raw)
To: qemu-devel; +Cc: john cooper
Failure by qemu to open a default config file isn't cause to
error exit -- it just quietly continues on. After puzzling
issues with otherwise opaque config file locations and
startup handling numerous times, some help from qemu seemed
justified.
In the case of a "?" pseudo filename arg to -readconfig,
verbose open of all config files will be enabled. Normal
handling of config files is otherwise unaffected by this
option.
Signed-off-by: john cooper <john.cooper@redhat.com>
---
diff --git a/qemu-config.c b/qemu-config.c
index 1efdbec..805fcc6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -534,21 +534,31 @@ out:
return res;
}
-int qemu_read_config_file(const char *filename)
+/* attempt to open and parse config file, report problems if vflag
+ */
+int qemu_read_config_file(const char *filename, int vflag)
{
FILE *f = fopen(filename, "r");
- int ret;
+ int rv = 0;
+ const char *err;
if (f == NULL) {
- return -errno;
+ rv = -errno;
+ err = "open";
}
-
- ret = qemu_config_parse(f, vm_config_groups, filename);
- fclose(f);
-
- if (ret == 0) {
- return 0;
- } else {
- return -EINVAL;
+ else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
+ rv = -EINVAL;
+ err = "parse";
+ }
+ else if (vflag) {
+ fprintf(stderr, "parsed config file %s\n", filename);
+ }
+ if (f) {
+ fclose(f);
+ }
+ if (rv && vflag) {
+ fprintf(stderr, "can't %s config file %s: %s\n",
+ err, filename, strerror(-rv));
}
+ return rv;
}
diff --git a/qemu-config.h b/qemu-config.h
index 533a049..897fbce 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -13,6 +13,6 @@ void qemu_add_globals(void);
void qemu_config_write(FILE *fp);
int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
-int qemu_read_config_file(const char *filename);
+int qemu_read_config_file(const char *filename, int vflag);
#endif /* QEMU_CONFIG_H */
diff --git a/vl.c b/vl.c
index fd491ba..d192b51 100644
--- a/vl.c
+++ b/vl.c
@@ -1822,6 +1822,7 @@ int main(int argc, char **argv, char **envp)
const char *incoming = NULL;
int show_vnc_port = 0;
int defconfig = 1;
+ int defconfig_verbose = 0;
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
@@ -1875,6 +1876,11 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_nodefconfig:
defconfig=0;
break;
+ case QEMU_OPTION_readconfig:
+ /* pseudo filename "?" enables verbose config file handling */
+ if (!strcmp(optarg, "?"))
+ defconfig_verbose = 1;
+ break;
}
}
}
@@ -1882,12 +1888,13 @@ int main(int argc, char **argv, char **envp)
if (defconfig) {
int ret;
- ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+ ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
+ defconfig_verbose);
if (ret < 0 && ret != -ENOENT) {
exit(1);
}
- ret = qemu_read_config_file(arch_config_name);
+ ret = qemu_read_config_file(arch_config_name, defconfig_verbose);
if (ret < 0 && ret != -ENOENT) {
exit(1);
}
@@ -2596,15 +2603,10 @@ int main(int argc, char **argv, char **envp)
xen_mode = XEN_ATTACH;
break;
case QEMU_OPTION_readconfig:
- {
- int ret = qemu_read_config_file(optarg);
- if (ret < 0) {
- fprintf(stderr, "read config %s: %s\n", optarg,
- strerror(-ret));
+ if (strcmp(optarg, "?") &&
+ qemu_read_config_file(optarg, defconfig_verbose) < 0)
exit(1);
- }
- break;
- }
+ break;
case QEMU_OPTION_writeconfig:
{
FILE *fp;
--
john.cooper@redhat.com
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
2010-09-07 12:31 john cooper
@ 2010-09-07 18:38 ` Blue Swirl
2010-09-09 3:48 ` john cooper
0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2010-09-07 18:38 UTC (permalink / raw)
To: john cooper; +Cc: qemu-devel
On Tue, Sep 7, 2010 at 12:31 PM, john cooper <john.cooper@redhat.com> wrote:
> Failure by qemu to open a default config file isn't cause to
> error exit -- it just quietly continues on. After puzzling
> issues with otherwise opaque config file locations and
> startup handling numerous times, some help from qemu seemed
> justified.
Maybe there should be an error exit if the user specifies a config
file but there are problems with it?
> In the case of a "?" pseudo filename arg to -readconfig,
> verbose open of all config files will be enabled. Normal
> handling of config files is otherwise unaffected by this
> option.
I think '?' is not very good name. Could we add flags to -readconfig,
like -readconfig verbose,nodefaultconfig,file='', to match other
options' syntax?
> Signed-off-by: john cooper <john.cooper@redhat.com>
> ---
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 1efdbec..805fcc6 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -534,21 +534,31 @@ out:
> return res;
> }
>
> -int qemu_read_config_file(const char *filename)
> +/* attempt to open and parse config file, report problems if vflag
> + */
> +int qemu_read_config_file(const char *filename, int vflag)
> {
> FILE *f = fopen(filename, "r");
> - int ret;
> + int rv = 0;
> + const char *err;
>
> if (f == NULL) {
> - return -errno;
> + rv = -errno;
> + err = "open";
> }
> -
> - ret = qemu_config_parse(f, vm_config_groups, filename);
> - fclose(f);
> -
> - if (ret == 0) {
> - return 0;
> - } else {
> - return -EINVAL;
> + else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
> + rv = -EINVAL;
> + err = "parse";
> + }
> + else if (vflag) {
> + fprintf(stderr, "parsed config file %s\n", filename);
> + }
> + if (f) {
> + fclose(f);
> + }
> + if (rv && vflag) {
> + fprintf(stderr, "can't %s config file %s: %s\n",
> + err, filename, strerror(-rv));
I'd just duplicate this for both open and parse.
> }
> + return rv;
> }
> diff --git a/qemu-config.h b/qemu-config.h
> index 533a049..897fbce 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -13,6 +13,6 @@ void qemu_add_globals(void);
> void qemu_config_write(FILE *fp);
> int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
>
> -int qemu_read_config_file(const char *filename);
> +int qemu_read_config_file(const char *filename, int vflag);
>
> #endif /* QEMU_CONFIG_H */
> diff --git a/vl.c b/vl.c
> index fd491ba..d192b51 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1822,6 +1822,7 @@ int main(int argc, char **argv, char **envp)
> const char *incoming = NULL;
> int show_vnc_port = 0;
> int defconfig = 1;
> + int defconfig_verbose = 0;
>
> atexit(qemu_run_exit_notifiers);
> error_set_progname(argv[0]);
> @@ -1875,6 +1876,11 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_nodefconfig:
> defconfig=0;
> break;
> + case QEMU_OPTION_readconfig:
> + /* pseudo filename "?" enables verbose config file handling */
> + if (!strcmp(optarg, "?"))
> + defconfig_verbose = 1;
> + break;
> }
> }
> }
> @@ -1882,12 +1888,13 @@ int main(int argc, char **argv, char **envp)
> if (defconfig) {
> int ret;
>
> - ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
> + ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
> + defconfig_verbose);
> if (ret < 0 && ret != -ENOENT) {
> exit(1);
> }
>
> - ret = qemu_read_config_file(arch_config_name);
> + ret = qemu_read_config_file(arch_config_name, defconfig_verbose);
> if (ret < 0 && ret != -ENOENT) {
> exit(1);
> }
> @@ -2596,15 +2603,10 @@ int main(int argc, char **argv, char **envp)
> xen_mode = XEN_ATTACH;
> break;
> case QEMU_OPTION_readconfig:
> - {
> - int ret = qemu_read_config_file(optarg);
> - if (ret < 0) {
> - fprintf(stderr, "read config %s: %s\n", optarg,
> - strerror(-ret));
> + if (strcmp(optarg, "?") &&
> + qemu_read_config_file(optarg, defconfig_verbose) < 0)
> exit(1);
> - }
> - break;
> - }
> + break;
> case QEMU_OPTION_writeconfig:
> {
> FILE *fp;
> --
> john.cooper@redhat.com
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
2010-09-07 18:38 ` Blue Swirl
@ 2010-09-09 3:48 ` john cooper
2010-09-09 18:23 ` Blue Swirl
0 siblings, 1 reply; 6+ messages in thread
From: john cooper @ 2010-09-09 3:48 UTC (permalink / raw)
To: Blue Swirl; +Cc: john cooper, qemu-devel
Blue Swirl wrote:
> On Tue, Sep 7, 2010 at 12:31 PM, john cooper <john.cooper@redhat.com> wrote:
>> Failure by qemu to open a default config file isn't cause to
>> error exit -- it just quietly continues on. After puzzling
>> issues with otherwise opaque config file locations and
>> startup handling numerous times, some help from qemu seemed
>> justified.
>
> Maybe there should be an error exit if the user specifies a config
> file but there are problems with it?
That's one possibility. However given the preexisting
behavior where open of at least one of the config files
routinely fails and is quietly dismissed, issuing warnings
would seem distracting to the user.
I think one config file is all which is needed, and the
config syntax can be extended to allow including other
vendor/install specific files as needed. I particularly
feel so as we've locally had to add yet a third config file
to push system quasi-static config data out of the way of
possible user modification for libvirt concerns. That was
a last-minute bandaid solution which just makes the problem
worse. Anyway such vendor specific config structure should
be handled within the config mechanism itself vs. hard coding
it into qemu startup.
>> In the case of a "?" pseudo filename arg to -readconfig,
>> verbose open of all config files will be enabled. Normal
>> handling of config files is otherwise unaffected by this
>> option.
>
> I think '?' is not very good name.
I agree, a shell meta char wasn't my first choice. However
it follows the precedent of '?' used in similar query operations
and was chosen only for CLI consistency.
> Could we add flags to -readconfig,
> like -readconfig verbose,nodefaultconfig,file='', to match other
> options' syntax?
That seems most natural for options specific to the associated
config file. However the verbose flag was intended as a
global action rather than local to a given config file. The
preexisting "nodefconfig" is also a global option.
Thanks,
-john
--
john.cooper@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
2010-09-09 3:48 ` john cooper
@ 2010-09-09 18:23 ` Blue Swirl
2010-09-15 5:29 ` john cooper
0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2010-09-09 18:23 UTC (permalink / raw)
To: john cooper; +Cc: qemu-devel
On Thu, Sep 9, 2010 at 3:48 AM, john cooper <john.cooper@redhat.com> wrote:
> Blue Swirl wrote:
>> On Tue, Sep 7, 2010 at 12:31 PM, john cooper <john.cooper@redhat.com> wrote:
>>> Failure by qemu to open a default config file isn't cause to
>>> error exit -- it just quietly continues on. After puzzling
>>> issues with otherwise opaque config file locations and
>>> startup handling numerous times, some help from qemu seemed
>>> justified.
>>
>> Maybe there should be an error exit if the user specifies a config
>> file but there are problems with it?
>
> That's one possibility. However given the preexisting
> behavior where open of at least one of the config files
> routinely fails and is quietly dismissed, issuing warnings
> would seem distracting to the user.
>
> I think one config file is all which is needed, and the
> config syntax can be extended to allow including other
> vendor/install specific files as needed. I particularly
> feel so as we've locally had to add yet a third config file
> to push system quasi-static config data out of the way of
> possible user modification for libvirt concerns. That was
> a last-minute bandaid solution which just makes the problem
> worse. Anyway such vendor specific config structure should
> be handled within the config mechanism itself vs. hard coding
> it into qemu startup.
>
>>> In the case of a "?" pseudo filename arg to -readconfig,
>>> verbose open of all config files will be enabled. Normal
>>> handling of config files is otherwise unaffected by this
>>> option.
>>
>> I think '?' is not very good name.
>
> I agree, a shell meta char wasn't my first choice. However
> it follows the precedent of '?' used in similar query operations
> and was chosen only for CLI consistency.
But '?' is used for other purposes: query available options. It would
be more logical if -readconfig '?' instead could be used to query the
default config files.
>> Could we add flags to -readconfig,
>> like -readconfig verbose,nodefaultconfig,file='', to match other
>> options' syntax?
>
> That seems most natural for options specific to the associated
> config file. However the verbose flag was intended as a
> global action rather than local to a given config file. The
> preexisting "nodefconfig" is also a global option.
Right. It just seems that there are a lot of global flags. How about
-config nodefaults,verbose?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling
2010-09-09 18:23 ` Blue Swirl
@ 2010-09-15 5:29 ` john cooper
0 siblings, 0 replies; 6+ messages in thread
From: john cooper @ 2010-09-15 5:29 UTC (permalink / raw)
To: Blue Swirl; +Cc: john cooper, qemu-devel
Blue Swirl wrote:
> On Thu, Sep 9, 2010 at 3:48 AM, john cooper <john.cooper@redhat.com> wrote:
>>
>>> I think '?' is not very good name.
>> I agree, a shell meta char wasn't my first choice. However
>> it follows the precedent of '?' used in similar query operations
>> and was chosen only for CLI consistency.
>
> But '?' is used for other purposes: query available options. It would
> be more logical if -readconfig '?' instead could be used to query the
> default config files.
>
Which is what -writeconfig does, sort of. Although the structure
of potential multiple input config files is lost and rather the
resulting flat config space is output. But the alternative is
probably more of a headache than it's worth.
>>> Could we add flags to -readconfig,
>>> like -readconfig verbose,nodefaultconfig,file='', to match other
>>> options' syntax?
>> That seems most natural for options specific to the associated
>> config file. However the verbose flag was intended as a
>> global action rather than local to a given config file. The
>> preexisting "nodefconfig" is also a global option.
>
> Right. It just seems that there are a lot of global flags. How about
> -config nodefaults,verbose?
>
Agreed this is more logical. The reason I avoided doing so initially
was concern over impacting existing usage. But it isn't really much
in retrospect. Updated patch follows.
-john
--
john.cooper@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-24 22:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-24 22:45 [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling john cooper
-- strict thread matches above, loose matches on Subject: below --
2010-09-07 12:31 john cooper
2010-09-07 18:38 ` Blue Swirl
2010-09-09 3:48 ` john cooper
2010-09-09 18:23 ` Blue Swirl
2010-09-15 5:29 ` john cooper
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).