Re: [Qemu-devel] [PATCH V11 1/4] add def_value_str and use it in qemu_opts_print
δΊ 2013-1-25 2:26, Markus Armbruster ει:
Dong Xu Wang wdon...@vnet.linux.ibm.com writes:
qemu_opts_print has no user now, so can re-write the function safely.
qemu_opts_print will be used while using qemu-img create, it will
produce the same output as previous code.
The behavior of this function has changed:
1. Print every possible option, whether a value has been set or not.
2. Option descriptors may provide a default value.
3. Print to stdout instead of stderr.
Previously the behavior was to print every option that has been set.
Options that have not been set would be skipped.
Signed-off-by: Dong Xu Wang wdon...@vnet.linux.ibm.com
---
v10-v11:
1) print all values that have actually been assigned while accept-any
cases.
v7-v8:
1) print elements = accept any params while opts_accepts_any() ==
true.
2) since def_print_str is the default value if an option isn't set,
so rename it to def_value_str.
include/qemu/option.h | 1 +
util/qemu-option.c| 30 +-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index ba197cd..394170a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+const char *def_value_str;
} QemuOptDesc;
struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index f532b76..1aed418 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -863,13 +863,33 @@ void qemu_opts_del(QemuOpts *opts)
int qemu_opts_print(QemuOpts *opts, void *dummy)
{
QemuOpt *opt;
+QemuOptDesc *desc = opts-list-desc;
-fprintf(stderr, %s: %s:, opts-list-name,
-opts-id ? opts-id : noid);
-QTAILQ_FOREACH(opt, opts-head, next) {
-fprintf(stderr, %s=\%s\, opt-name, opt-str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, opts-head, next) {
+printf(%s=\%s\ , opt-name, opt-str);
+}
+return 0;
+}
+for (; desc desc-name; desc++) {
+const char *value = desc-def_value_str;
+QemuOpt *opt;
+
+opt = qemu_opt_find(opts, desc-name);
+if (opt) {
+value = opt-str;
+}
+
+if (!value) {
+continue;
+}
+
+if (desc-type == QEMU_OPT_STRING) {
+printf(%s='%s' , desc-name, value);
+} else {
+printf(%s=%s , desc-name, value);
+}
}
-fprintf(stderr, \n);
return 0;
}
I dislike def_value_str intensely, because it's a made up string that
needn't be related to the actual default value. Which is supplied by
the code getting the option.
If I remember correctly, you want qemu_opts_print() print the default
value, because you don't want to change output of qemu-img. Fair
enough.
Two sane ways to do that:
1. Make the caller to supply the default value. Just like the caller
supplies it when getting option values.
Easiest way is to make it set the defaults in the opts whenever it
supplies a default. Like this:
mumble = qemu_opt_get(opts, mumble);
if (!mumble) {
mumble = mutter;
qemu_opt_set(opts, mumble, mumble);
}
Problem: this doesn't set the value in the existing QemuOpt, it
appends a new one, which isn't what we want. We'd need a new
function to change the value.
2. Actually use def_value_str as default value! Replacing
if (value) {
opt-str = g_strdup(value);
}
in opt_set() by
opt-str = g_strdup(value ?: def_value_str);
could do. Looks easier than 1.
Okay, will use 2 in new version patches.
[Qemu-devel] [PATCH V11 1/4] add def_value_str and use it in qemu_opts_print
qemu_opts_print has no user now, so can re-write the function safely.
qemu_opts_print will be used while using qemu-img create, it will
produce the same output as previous code.
The behavior of this function has changed:
1. Print every possible option, whether a value has been set or not.
2. Option descriptors may provide a default value.
3. Print to stdout instead of stderr.
Previously the behavior was to print every option that has been set.
Options that have not been set would be skipped.
Signed-off-by: Dong Xu Wang wdon...@vnet.linux.ibm.com
---
v10-v11:
1) print all values that have actually been assigned while accept-any
cases.
v7-v8:
1) print elements = accept any params while opts_accepts_any() ==
true.
2) since def_print_str is the default value if an option isn't set,
so rename it to def_value_str.
include/qemu/option.h | 1 +
util/qemu-option.c| 30 +-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index ba197cd..394170a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+const char *def_value_str;
} QemuOptDesc;
struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index f532b76..1aed418 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -863,13 +863,33 @@ void qemu_opts_del(QemuOpts *opts)
int qemu_opts_print(QemuOpts *opts, void *dummy)
{
QemuOpt *opt;
+QemuOptDesc *desc = opts-list-desc;
-fprintf(stderr, %s: %s:, opts-list-name,
-opts-id ? opts-id : noid);
-QTAILQ_FOREACH(opt, opts-head, next) {
-fprintf(stderr, %s=\%s\, opt-name, opt-str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, opts-head, next) {
+printf(%s=\%s\ , opt-name, opt-str);
+}
+return 0;
+}
+for (; desc desc-name; desc++) {
+const char *value = desc-def_value_str;
+QemuOpt *opt;
+
+opt = qemu_opt_find(opts, desc-name);
+if (opt) {
+value = opt-str;
+}
+
+if (!value) {
+continue;
+}
+
+if (desc-type == QEMU_OPT_STRING) {
+printf(%s='%s' , desc-name, value);
+} else {
+printf(%s=%s , desc-name, value);
+}
}
-fprintf(stderr, \n);
return 0;
}
--
1.7.11.7
Re: [Qemu-devel] [PATCH V11 1/4] add def_value_str and use it in qemu_opts_print
Dong Xu Wang wdon...@vnet.linux.ibm.com writes:
qemu_opts_print has no user now, so can re-write the function safely.
qemu_opts_print will be used while using qemu-img create, it will
produce the same output as previous code.
The behavior of this function has changed:
1. Print every possible option, whether a value has been set or not.
2. Option descriptors may provide a default value.
3. Print to stdout instead of stderr.
Previously the behavior was to print every option that has been set.
Options that have not been set would be skipped.
Signed-off-by: Dong Xu Wang wdon...@vnet.linux.ibm.com
---
v10-v11:
1) print all values that have actually been assigned while accept-any
cases.
v7-v8:
1) print elements = accept any params while opts_accepts_any() ==
true.
2) since def_print_str is the default value if an option isn't set,
so rename it to def_value_str.
include/qemu/option.h | 1 +
util/qemu-option.c| 30 +-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index ba197cd..394170a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+const char *def_value_str;
} QemuOptDesc;
struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index f532b76..1aed418 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -863,13 +863,33 @@ void qemu_opts_del(QemuOpts *opts)
int qemu_opts_print(QemuOpts *opts, void *dummy)
{
QemuOpt *opt;
+QemuOptDesc *desc = opts-list-desc;
-fprintf(stderr, %s: %s:, opts-list-name,
-opts-id ? opts-id : noid);
-QTAILQ_FOREACH(opt, opts-head, next) {
-fprintf(stderr, %s=\%s\, opt-name, opt-str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, opts-head, next) {
+printf(%s=\%s\ , opt-name, opt-str);
+}
+return 0;
+}
+for (; desc desc-name; desc++) {
+const char *value = desc-def_value_str;
+QemuOpt *opt;
+
+opt = qemu_opt_find(opts, desc-name);
+if (opt) {
+value = opt-str;
+}
+
+if (!value) {
+continue;
+}
+
+if (desc-type == QEMU_OPT_STRING) {
+printf(%s='%s' , desc-name, value);
+} else {
+printf(%s=%s , desc-name, value);
+}
}
-fprintf(stderr, \n);
return 0;
}
I dislike def_value_str intensely, because it's a made up string that
needn't be related to the actual default value. Which is supplied by
the code getting the option.
If I remember correctly, you want qemu_opts_print() print the default
value, because you don't want to change output of qemu-img. Fair
enough.
Two sane ways to do that:
1. Make the caller to supply the default value. Just like the caller
supplies it when getting option values.
Easiest way is to make it set the defaults in the opts whenever it
supplies a default. Like this:
mumble = qemu_opt_get(opts, mumble);
if (!mumble) {
mumble = mutter;
qemu_opt_set(opts, mumble, mumble);
}
Problem: this doesn't set the value in the existing QemuOpt, it
appends a new one, which isn't what we want. We'd need a new
function to change the value.
2. Actually use def_value_str as default value! Replacing
if (value) {
opt-str = g_strdup(value);
}
in opt_set() by
opt-str = g_strdup(value ?: def_value_str);
could do. Looks easier than 1.
