Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
Am 02.02.2015 um 11:02 schrieb Marcel Apfelbaum:
On 01/31/2015 11:23 AM, Jan Kiszka wrote:
On 2015-01-05 12:37, Jan Kiszka wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
Just pulled current master, and at least the iommu machine option is
still triggering an abort.
What is the status of fixing these fallouts? Was anything else addressed
by now, just this one forgotten?
No, is not forgotten. I waited to see that the approach to fix this issue
is accepted by the reviewers.
Thanks for the reminder, patches will be submitted soon.
Marcel
Anything to look at a branch or something? Adding new machine bool options
is currently pretty hard - as long as it doesnt work ;-)
Christian
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 02/04/2015 04:18 PM, Christian Borntraeger wrote:
Am 02.02.2015 um 11:02 schrieb Marcel Apfelbaum:
On 01/31/2015 11:23 AM, Jan Kiszka wrote:
On 2015-01-05 12:37, Jan Kiszka wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
Just pulled current master, and at least the iommu machine option is
still triggering an abort.
What is the status of fixing these fallouts? Was anything else addressed
by now, just this one forgotten?
No, is not forgotten. I waited to see that the approach to fix this issue
is accepted by the reviewers.
Thanks for the reminder, patches will be submitted soon.
Marcel
Anything to look at a branch or something? Adding new machine bool options
is currently pretty hard - as long as it doesnt work ;-)
It will be ready today or tomorrow :)
Thanks,
Marcel
Christian
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 01/31/2015 11:23 AM, Jan Kiszka wrote:
On 2015-01-05 12:37, Jan Kiszka wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
Just pulled current master, and at least the iommu machine option is
still triggering an abort.
What is the status of fixing these fallouts? Was anything else addressed
by now, just this one forgotten?
No, is not forgotten. I waited to see that the approach to fix this issue
is accepted by the reviewers.
Thanks for the reminder, patches will be submitted soon.
Marcel
Jan
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015-01-05 12:37, Jan Kiszka wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
Just pulled current master, and at least the iommu machine option is
still triggering an abort.
What is the status of fixing these fallouts? Was anything else addressed
by now, just this one forgotten?
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015/1/6 14:20, Shannon Zhao wrote:
On 2015/1/6 10:37, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks for your test.
Tiejun
Thanks,
Shannon
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
return desc-def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
str = g_strdup(desc-def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const
char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_bool(name, desc-def_value_str, ret, error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_number(name, desc-def_value_str, ret,
error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_size(name, desc-def_value_str, ret, error_abort);
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On Tue, Jan 06, 2015 at 10:37:25AM +0800, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char
*name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
No, this is incorrect because now it just assigns the default value and
ignores the value from the command-line!
Stefan
pgpfvnLJwCm1m.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 01/06/2015 11:01 AM, Chen, Tiejun wrote:
On 2015/1/6 14:20, Shannon Zhao wrote:
On 2015/1/6 10:37, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks for your test.
Hi,
I just posted a fix on the mailing list.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272607.html
Please check if it works for you,
Thanks,
Marcel
Tiejun
Thanks,
Shannon
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
return desc-def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
str = g_strdup(desc-def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const
char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_bool(name, desc-def_value_str, ret, error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_number(name, desc-def_value_str, ret,
error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_size(name, desc-def_value_str, ret, error_abort);
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015/1/6 10:37, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks,
Shannon
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
return desc-def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
str = g_strdup(desc-def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_bool(name, desc-def_value_str, ret, error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts
*opts, const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_number(name, desc-def_value_str, ret,
error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
parse_option_size(name, desc-def_value_str, ret, error_abort);
[Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
--
2.1.0
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
This means we need to modify the way default values are processed.
MachineState fields should start with the default value and be
overridden by command-line options. Right now it works differently:
we query the command-line option and fall back to the default if the
option wasn't set. That approach doesn't work for MachineState fields
since C types (bool, int, etc) aren't tristate in the general case, so
they cannot indicate whether or not they were set.
The benefit of doing this work is that we eliminate the QemuOpts layer
and work directly with QOM properties instead.
Marcel: Do you want to do this or do you have another fix in mind?
Stefan
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
This means we need to modify the way default values are processed.
MachineState fields should start with the default value and be
overridden by command-line options. Right now it works differently:
we query the command-line option and fall back to the default if the
option wasn't set. That approach doesn't work for MachineState fields
since C types (bool, int, etc) aren't tristate in the general case, so
they cannot indicate whether or not they were set.
The benefit of doing this work is that we eliminate the QemuOpts layer
and work directly with QOM properties instead.
Marcel: Do you want to do this or do you have another fix in mind?
Hi, sorry for not getting to this earlier, I thought Paolo already fixed
it, but I was wrong.
I think this is the right direction, I'll am going to take care of it. (At
least I'll try...)
Thanks,
Marcel
Stefan
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char
*name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc,
name);
if (desc desc-def_value_str) {
return desc-def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt-desc) {
desc = find_desc_by_name(opts-list-desc, name);
if (desc desc-def_value_str) {
str = g_strdup(desc-def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc,
name);
if (desc desc-def_value_str) {
parse_option_bool(name, desc-def_value_str, ret,
error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts
*opts, const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc,
name);
if (desc desc-def_value_str) {
parse_option_number(name, desc-def_value_str, ret,
error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts
*opts, const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt-desc) {
const QemuOptDesc *desc = find_desc_by_name(opts-list-desc,
name);
if (desc desc-def_value_str) {
parse_option_size(name, desc-def_value_str, ret,
error_abort);
--
Thanks
Tiejun
This means we need to modify the way default values are processed.
MachineState fields should start with the default value and be
overridden by command-line options. Right now it works differently:
we query the command-line option and fall back to the default if the
option wasn't set. That approach doesn't work for MachineState fields
since C types (bool, int, etc) aren't tristate in the general case, so
they cannot indicate whether or not they were set.
The benefit of doing this work is that we eliminate the QemuOpts layer
and work directly with QOM properties instead.
Marcel: Do you want to do this or do you have another fix in mind?
Hi, sorry for not getting to this earlier, I thought Paolo already fixed
it, but I was wrong.
I think this is the right direction, I'll am going to take care of it.
(At least I'll try...)
Thanks,
Marcel
Stefan
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 01/05/15 13:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka jan.kis...@siemens.com
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 (machine: remove
qemu_machine_opts global list) removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt-desc opt-desc-type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState-usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum marce...@redhat.com
Cc: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), usb,
- has_defaults default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), usb)) {
+return current_machine-usb;
+} else {
+return has_defaults default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
I agree. That's what I thought should be done after I reported
http://thread.gmane.org/gmane.comp.emulators.qemu/312139
and (significantly later!) attempted to look into it.
Converting all callers looked like a gargantuan task though, so I hid,
ultimately.
This means we need to modify the way default values are processed.
MachineState fields should start with the default value and be
overridden by command-line options. Right now it works differently:
we query the command-line option and fall back to the default if the
option wasn't set. That approach doesn't work for MachineState fields
since C types (bool, int, etc) aren't tristate in the general case, so
they cannot indicate whether or not they were set.
The benefit of doing this work is that we eliminate the QemuOpts layer
and work directly with QOM properties instead.
Marcel: Do you want to do this or do you have another fix in mind?
Hi, sorry for not getting to this earlier, I thought Paolo already fixed
it, but I was wrong.
That's my fault, apologies -- Paolo posted a quick patch which I applied
and tested -- it made the crash go away, which was the only thing I
cared about at that point:
http://thread.gmane.org/gmane.comp.emulators.qemu/312139/focus=312174
However, as Paolo said in the thread almost immediately after,
http://thread.gmane.org/gmane.comp.emulators.qemu/312139/focus=312174
the -usb option itself stopped working (which I had not tested at all).
You saw my rash Tested-by, but (apparently) missed Paolo's followup.
I think this is the right direction, I'll am going to take care of it.
(At least I'll try...)
Thanks, and sorry about the confusion.
Laszlo
