* [PATCH 1/4] vl: Remove unused variable in configure_accelerators
2020-01-09 2:17 [PATCH 0/4] vl: Fixes for cleanups to -accel Richard Henderson
@ 2020-01-09 2:17 ` Richard Henderson
2020-01-09 11:01 ` Alex Bennée
` (2 more replies)
2020-01-09 2:17 ` [PATCH 2/4] vl: Free accel_list " Richard Henderson
` (3 subsequent siblings)
4 siblings, 3 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-09 2:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
The accel_initialised variable no longer has any setters.
Fixes: 6f6e1698a68c
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
vl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index 86474a55c9..be79b03c1a 100644
--- a/vl.c
+++ b/vl.c
@@ -2749,7 +2749,6 @@ static void configure_accelerators(const char *progname)
{
const char *accel;
char **accel_list, **tmp;
- bool accel_initialised = false;
bool init_failed = false;
qemu_opts_foreach(qemu_find_opts("icount"),
@@ -2776,7 +2775,7 @@ static void configure_accelerators(const char *progname)
accel_list = g_strsplit(accel, ":", 0);
- for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
+ for (tmp = accel_list; tmp && *tmp; tmp++) {
/*
* Filter invalid accelerators here, to prevent obscenities
* such as "-machine accel=tcg,,thread=single".
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] vl: Remove unused variable in configure_accelerators
2020-01-09 2:17 ` [PATCH 1/4] vl: Remove unused variable in configure_accelerators Richard Henderson
@ 2020-01-09 11:01 ` Alex Bennée
2020-01-09 12:06 ` Aleksandar Markovic
2020-01-09 12:23 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2020-01-09 11:01 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Richard Henderson <richard.henderson@linaro.org> writes:
> The accel_initialised variable no longer has any setters.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> vl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 86474a55c9..be79b03c1a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2749,7 +2749,6 @@ static void configure_accelerators(const char *progname)
> {
> const char *accel;
> char **accel_list, **tmp;
> - bool accel_initialised = false;
> bool init_failed = false;
>
> qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2776,7 +2775,7 @@ static void configure_accelerators(const char *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> + for (tmp = accel_list; tmp && *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] vl: Remove unused variable in configure_accelerators
2020-01-09 2:17 ` [PATCH 1/4] vl: Remove unused variable in configure_accelerators Richard Henderson
2020-01-09 11:01 ` Alex Bennée
@ 2020-01-09 12:06 ` Aleksandar Markovic
2020-01-09 12:23 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2020-01-09 12:06 UTC (permalink / raw)
To: Richard Henderson; +Cc: pbonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
On Thursday, January 9, 2020, Richard Henderson <
richard.henderson@linaro.org> wrote:
> The accel_initialised variable no longer has any setters.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> vl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>
Reviewed by: Aleksandar Markovic <amarkovic@wavecomp.com>
> diff --git a/vl.c b/vl.c
> index 86474a55c9..be79b03c1a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2749,7 +2749,6 @@ static void configure_accelerators(const char
> *progname)
> {
> const char *accel;
> char **accel_list, **tmp;
> - bool accel_initialised = false;
> bool init_failed = false;
>
> qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2776,7 +2775,7 @@ static void configure_accelerators(const char
> *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> + for (tmp = accel_list; tmp && *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
> --
> 2.20.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2081 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] vl: Remove unused variable in configure_accelerators
2020-01-09 2:17 ` [PATCH 1/4] vl: Remove unused variable in configure_accelerators Richard Henderson
2020-01-09 11:01 ` Alex Bennée
2020-01-09 12:06 ` Aleksandar Markovic
@ 2020-01-09 12:23 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-09 12:23 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini
On 1/9/20 3:17 AM, Richard Henderson wrote:
> The accel_initialised variable no longer has any setters.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> vl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 86474a55c9..be79b03c1a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2749,7 +2749,6 @@ static void configure_accelerators(const char *progname)
> {
> const char *accel;
> char **accel_list, **tmp;
> - bool accel_initialised = false;
> bool init_failed = false;
>
> qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2776,7 +2775,7 @@ static void configure_accelerators(const char *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> + for (tmp = accel_list; tmp && *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] vl: Free accel_list in configure_accelerators
2020-01-09 2:17 [PATCH 0/4] vl: Fixes for cleanups to -accel Richard Henderson
2020-01-09 2:17 ` [PATCH 1/4] vl: Remove unused variable in configure_accelerators Richard Henderson
@ 2020-01-09 2:17 ` Richard Henderson
2020-01-09 8:18 ` Thomas Huth
2020-01-09 11:04 ` Alex Bennée
2020-01-09 2:17 ` [PATCH 3/4] vl: Remove useless test " Richard Henderson
` (2 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-09 2:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
We allocate the list with g_strsplit, so free it too.
This freeing was lost during one of the rearrangements.
Fixes: 6f6e1698a68c
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
vl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index be79b03c1a..c9329fe699 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
static void configure_accelerators(const char *progname)
{
const char *accel;
- char **accel_list, **tmp;
bool init_failed = false;
qemu_opts_foreach(qemu_find_opts("icount"),
@@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+ char **accel_list, **tmp;
+
if (accel == NULL) {
/* Select the default accelerator */
if (!accel_find("tcg") && !accel_find("kvm")) {
@@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
error_report("invalid accelerator %s", *tmp);
}
}
+ g_strfreev(accel_list);
} else {
if (accel != NULL) {
error_report("The -accel and \"-machine accel=\" options are incompatible");
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
2020-01-09 2:17 ` [PATCH 2/4] vl: Free accel_list " Richard Henderson
@ 2020-01-09 8:18 ` Thomas Huth
2020-01-09 8:24 ` Laurent Vivier
2020-01-09 11:04 ` Alex Bennée
1 sibling, 1 reply; 18+ messages in thread
From: Thomas Huth @ 2020-01-09 8:18 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Laurent Vivier, pbonzini
On 09/01/2020 03.17, Richard Henderson wrote:
> We allocate the list with g_strsplit, so free it too.
> This freeing was lost during one of the rearrangements.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> vl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index be79b03c1a..c9329fe699 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
> static void configure_accelerators(const char *progname)
> {
> const char *accel;
> - char **accel_list, **tmp;
> bool init_failed = false;
>
> qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>
> accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> + char **accel_list, **tmp;
> +
> if (accel == NULL) {
> /* Select the default accelerator */
> if (!accel_find("tcg") && !accel_find("kvm")) {
> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
> error_report("invalid accelerator %s", *tmp);
> }
> }
> + g_strfreev(accel_list);
> } else {
> if (accel != NULL) {
> error_report("The -accel and \"-machine accel=\" options are incompatible");
>
FYI, a fix for this is already part of Laurent's "Trivial branch
patches" PULL request from yesterday.
Thomas
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
2020-01-09 8:18 ` Thomas Huth
@ 2020-01-09 8:24 ` Laurent Vivier
2020-01-09 11:59 ` Aleksandar Markovic
0 siblings, 1 reply; 18+ messages in thread
From: Laurent Vivier @ 2020-01-09 8:24 UTC (permalink / raw)
To: Thomas Huth, Richard Henderson, qemu-devel; +Cc: pbonzini
On 09/01/2020 09:18, Thomas Huth wrote:
> On 09/01/2020 03.17, Richard Henderson wrote:
>> We allocate the list with g_strsplit, so free it too.
>> This freeing was lost during one of the rearrangements.
>>
>> Fixes: 6f6e1698a68c
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> vl.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/vl.c b/vl.c
>> index be79b03c1a..c9329fe699 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>> static void configure_accelerators(const char *progname)
>> {
>> const char *accel;
>> - char **accel_list, **tmp;
>> bool init_failed = false;
>>
>> qemu_opts_foreach(qemu_find_opts("icount"),
>> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>>
>> accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>> if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
>> + char **accel_list, **tmp;
>> +
>> if (accel == NULL) {
>> /* Select the default accelerator */
>> if (!accel_find("tcg") && !accel_find("kvm")) {
>> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
>> error_report("invalid accelerator %s", *tmp);
>> }
>> }
>> + g_strfreev(accel_list);
>> } else {
>> if (accel != NULL) {
>> error_report("The -accel and \"-machine accel=\" options are incompatible");
>>
>
> FYI, a fix for this is already part of Laurent's "Trivial branch
> patches" PULL request from yesterday.
https://patchew.org/QEMU/20200108160233.991134-1-laurent@vivier.eu/20200108160233.991134-6-laurent@vivier.eu/
Thanks,
Laurent
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] vl: Free accel_list in configure_accelerators
2020-01-09 8:24 ` Laurent Vivier
@ 2020-01-09 11:59 ` Aleksandar Markovic
0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2020-01-09 11:59 UTC (permalink / raw)
To: Laurent Vivier; +Cc: pbonzini, Thomas Huth, Richard Henderson, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2224 bytes --]
On Thursday, January 9, 2020, Laurent Vivier <lvivier@redhat.com> wrote:
> On 09/01/2020 09:18, Thomas Huth wrote:
> > On 09/01/2020 03.17, Richard Henderson wrote:
> >> We allocate the list with g_strsplit, so free it too.
> >> This freeing was lost during one of the rearrangements.
> >>
> >> Fixes: 6f6e1698a68c
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> vl.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/vl.c b/vl.c
> >> index be79b03c1a..c9329fe699 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque,
> QemuOpts *opts, Error **errp)
> >> static void configure_accelerators(const char *progname)
> >> {
> >> const char *accel;
> >> - char **accel_list, **tmp;
> >> bool init_failed = false;
> >>
> >> qemu_opts_foreach(qemu_find_opts("icount"),
> >> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char
> *progname)
> >>
> >> accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >> if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> >> + char **accel_list, **tmp;
> >> +
> >> if (accel == NULL) {
> >> /* Select the default accelerator */
> >> if (!accel_find("tcg") && !accel_find("kvm")) {
> >> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char
> *progname)
> >> error_report("invalid accelerator %s", *tmp);
> >> }
> >> }
> >> + g_strfreev(accel_list);
> >> } else {
> >> if (accel != NULL) {
> >> error_report("The -accel and \"-machine accel=\" options
> are incompatible");
> >>
> >
> > FYI, a fix for this is already part of Laurent's "Trivial branch
> > patches" PULL request from yesterday.
>
> https://patchew.org/QEMU/20200108160233.991134-1-laurent@
> vivier.eu/20200108160233.991134-6-laurent@vivier.eu/
>
>
If this (from Laurent's PR) patch is merged, Richard could transform his
patch into "vl: Fix the scope of variables accel_list and tmp in
configure_accelerator()" in v2.
Whatever happens:
Reviewed by: Aleksandar Markovic <amarkovic@wavecomp.com>
> Thanks,
> Laurent
>
>
>
[-- Attachment #2: Type: text/html, Size: 3493 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
2020-01-09 2:17 ` [PATCH 2/4] vl: Free accel_list " Richard Henderson
2020-01-09 8:18 ` Thomas Huth
@ 2020-01-09 11:04 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2020-01-09 11:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Richard Henderson <richard.henderson@linaro.org> writes:
> We allocate the list with g_strsplit, so free it too.
> This freeing was lost during one of the rearrangements.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> vl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index be79b03c1a..c9329fe699 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
> static void configure_accelerators(const char *progname)
> {
> const char *accel;
> - char **accel_list, **tmp;
> bool init_failed = false;
>
> qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>
> accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> + char **accel_list, **tmp;
> +
> if (accel == NULL) {
> /* Select the default accelerator */
> if (!accel_find("tcg") && !accel_find("kvm")) {
> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
> error_report("invalid accelerator %s", *tmp);
> }
> }
> + g_strfreev(accel_list);
> } else {
> if (accel != NULL) {
> error_report("The -accel and \"-machine accel=\" options are incompatible");
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4] vl: Remove useless test in configure_accelerators
2020-01-09 2:17 [PATCH 0/4] vl: Fixes for cleanups to -accel Richard Henderson
2020-01-09 2:17 ` [PATCH 1/4] vl: Remove unused variable in configure_accelerators Richard Henderson
2020-01-09 2:17 ` [PATCH 2/4] vl: Free accel_list " Richard Henderson
@ 2020-01-09 2:17 ` Richard Henderson
2020-01-09 11:05 ` Alex Bennée
` (2 more replies)
2020-01-09 2:17 ` [PATCH 4/4] vl: Only choose enabled accelerators " Richard Henderson
2020-01-09 9:00 ` [PATCH 0/4] vl: Fixes for cleanups to -accel Paolo Bonzini
4 siblings, 3 replies; 18+ messages in thread
From: Richard Henderson @ 2020-01-09 2:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
The result of g_strsplit is never NULL.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
vl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index c9329fe699..887dbfbb5d 100644
--- a/vl.c
+++ b/vl.c
@@ -2776,7 +2776,7 @@ static void configure_accelerators(const char *progname)
accel_list = g_strsplit(accel, ":", 0);
- for (tmp = accel_list; tmp && *tmp; tmp++) {
+ for (tmp = accel_list; *tmp; tmp++) {
/*
* Filter invalid accelerators here, to prevent obscenities
* such as "-machine accel=tcg,,thread=single".
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] vl: Remove useless test in configure_accelerators
2020-01-09 2:17 ` [PATCH 3/4] vl: Remove useless test " Richard Henderson
@ 2020-01-09 11:05 ` Alex Bennée
2020-01-09 12:07 ` Aleksandar Markovic
2020-01-09 12:24 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2020-01-09 11:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Richard Henderson <richard.henderson@linaro.org> writes:
> The result of g_strsplit is never NULL.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> vl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index c9329fe699..887dbfbb5d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2776,7 +2776,7 @@ static void configure_accelerators(const char *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; tmp && *tmp; tmp++) {
> + for (tmp = accel_list; *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] vl: Remove useless test in configure_accelerators
2020-01-09 2:17 ` [PATCH 3/4] vl: Remove useless test " Richard Henderson
2020-01-09 11:05 ` Alex Bennée
@ 2020-01-09 12:07 ` Aleksandar Markovic
2020-01-09 12:24 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2020-01-09 12:07 UTC (permalink / raw)
To: Richard Henderson; +Cc: pbonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 843 bytes --]
On Thursday, January 9, 2020, Richard Henderson <
richard.henderson@linaro.org> wrote:
> The result of g_strsplit is never NULL.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> vl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
Reviewed by: Aleksandar Markovic <amarkovic@wavecomp.com>
> diff --git a/vl.c b/vl.c
> index c9329fe699..887dbfbb5d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2776,7 +2776,7 @@ static void configure_accelerators(const char
> *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; tmp && *tmp; tmp++) {
> + for (tmp = accel_list; *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
> --
> 2.20.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 1681 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] vl: Remove useless test in configure_accelerators
2020-01-09 2:17 ` [PATCH 3/4] vl: Remove useless test " Richard Henderson
2020-01-09 11:05 ` Alex Bennée
2020-01-09 12:07 ` Aleksandar Markovic
@ 2020-01-09 12:24 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-09 12:24 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini
On 1/9/20 3:17 AM, Richard Henderson wrote:
> The result of g_strsplit is never NULL.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> vl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index c9329fe699..887dbfbb5d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2776,7 +2776,7 @@ static void configure_accelerators(const char *progname)
>
> accel_list = g_strsplit(accel, ":", 0);
>
> - for (tmp = accel_list; tmp && *tmp; tmp++) {
> + for (tmp = accel_list; *tmp; tmp++) {
> /*
> * Filter invalid accelerators here, to prevent obscenities
> * such as "-machine accel=tcg,,thread=single".
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] vl: Only choose enabled accelerators in configure_accelerators
2020-01-09 2:17 [PATCH 0/4] vl: Fixes for cleanups to -accel Richard Henderson
` (2 preceding siblings ...)
2020-01-09 2:17 ` [PATCH 3/4] vl: Remove useless test " Richard Henderson
@ 2020-01-09 2:17 ` Richard Henderson
2020-01-09 11:35 ` Alex Bennée
2020-01-09 9:00 ` [PATCH 0/4] vl: Fixes for cleanups to -accel Paolo Bonzini
4 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2020-01-09 2:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
By choosing "tcg:kvm" when kvm is not enabled, we generate
an incorrect warning: "invalid accelerator kvm".
Presumably the inverse is also true with --disable-tcg.
Fixes: 28a0961757fc
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
vl.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/vl.c b/vl.c
index 887dbfbb5d..9b7651c80d 100644
--- a/vl.c
+++ b/vl.c
@@ -2759,11 +2759,10 @@ static void configure_accelerators(const char *progname)
if (accel == NULL) {
/* Select the default accelerator */
- if (!accel_find("tcg") && !accel_find("kvm")) {
- error_report("No accelerator selected and"
- " no default accelerator available");
- exit(1);
- } else {
+ bool have_tcg = accel_find("tcg");
+ bool have_kvm = accel_find("kvm");
+
+ if (have_tcg && have_kvm) {
int pnlen = strlen(progname);
if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
/* If the program name ends with "kvm", we prefer KVM */
@@ -2771,9 +2770,16 @@ static void configure_accelerators(const char *progname)
} else {
accel = "tcg:kvm";
}
+ } else if (have_kvm) {
+ accel = "kvm";
+ } else if (have_tcg) {
+ accel = "tcg";
+ } else {
+ error_report("No accelerator selected and"
+ " no default accelerator available");
+ exit(1);
}
}
-
accel_list = g_strsplit(accel, ":", 0);
for (tmp = accel_list; *tmp; tmp++) {
--
2.20.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] vl: Only choose enabled accelerators in configure_accelerators
2020-01-09 2:17 ` [PATCH 4/4] vl: Only choose enabled accelerators " Richard Henderson
@ 2020-01-09 11:35 ` Alex Bennée
2020-01-09 12:10 ` Aleksandar Markovic
0 siblings, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2020-01-09 11:35 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Richard Henderson <richard.henderson@linaro.org> writes:
> By choosing "tcg:kvm" when kvm is not enabled, we generate
> an incorrect warning: "invalid accelerator kvm".
>
> Presumably the inverse is also true with --disable-tcg.
>
> Fixes: 28a0961757fc
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> vl.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 887dbfbb5d..9b7651c80d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2759,11 +2759,10 @@ static void configure_accelerators(const char *progname)
>
> if (accel == NULL) {
> /* Select the default accelerator */
> - if (!accel_find("tcg") && !accel_find("kvm")) {
> - error_report("No accelerator selected and"
> - " no default accelerator available");
> - exit(1);
> - } else {
> + bool have_tcg = accel_find("tcg");
> + bool have_kvm = accel_find("kvm");
> +
> + if (have_tcg && have_kvm) {
> int pnlen = strlen(progname);
> if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3],
> "kvm")) {
I know you're not touching this bit but:
modified vl.c
@@ -2763,8 +2763,7 @@ static void configure_accelerators(const char *progname)
bool have_kvm = accel_find("kvm");
if (have_tcg && have_kvm) {
- int pnlen = strlen(progname);
- if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
+ if (g_str_has_suffix(progname, "kvm")) {
/* If the program name ends with "kvm", we prefer KVM */
accel = "kvm:tcg";
} else {
> /* If the program name ends with "kvm", we prefer KVM */
> @@ -2771,9 +2770,16 @@ static void configure_accelerators(const char *progname)
> } else {
> accel = "tcg:kvm";
> }
> + } else if (have_kvm) {
> + accel = "kvm";
> + } else if (have_tcg) {
> + accel = "tcg";
> + } else {
> + error_report("No accelerator selected and"
> + " no default accelerator available");
> + exit(1);
> }
> }
> -
> accel_list = g_strsplit(accel, ":", 0);
>
> for (tmp = accel_list; *tmp; tmp++) {
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] vl: Only choose enabled accelerators in configure_accelerators
2020-01-09 11:35 ` Alex Bennée
@ 2020-01-09 12:10 ` Aleksandar Markovic
0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2020-01-09 12:10 UTC (permalink / raw)
To: Alex Bennée; +Cc: pbonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3086 bytes --]
On Thursday, January 9, 2020, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > By choosing "tcg:kvm" when kvm is not enabled, we generate
> > an incorrect warning: "invalid accelerator kvm".
> >
> > Presumably the inverse is also true with --disable-tcg.
> >
> > Fixes: 28a0961757fc
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> > vl.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 887dbfbb5d..9b7651c80d 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2759,11 +2759,10 @@ static void configure_accelerators(const char
> *progname)
> >
> > if (accel == NULL) {
> > /* Select the default accelerator */
> > - if (!accel_find("tcg") && !accel_find("kvm")) {
> > - error_report("No accelerator selected and"
> > - " no default accelerator available");
> > - exit(1);
> > - } else {
> > + bool have_tcg = accel_find("tcg");
> > + bool have_kvm = accel_find("kvm");
> > +
> > + if (have_tcg && have_kvm) {
> > int pnlen = strlen(progname);
> > if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3],
> > "kvm")) {
>
> I know you're not touching this bit but:
>
> modified vl.c
> @@ -2763,8 +2763,7 @@ static void configure_accelerators(const char
> *progname)
> bool have_kvm = accel_find("kvm");
>
> if (have_tcg && have_kvm) {
> - int pnlen = strlen(progname);
> - if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3],
> "kvm")) {
> + if (g_str_has_suffix(progname, "kvm")) {
> /* If the program name ends with "kvm", we prefer KVM
> */
> accel = "kvm:tcg";
> } else {
>
>
> > /* If the program name ends with "kvm", we prefer
> KVM */
> > @@ -2771,9 +2770,16 @@ static void configure_accelerators(const char
> *progname)
> > } else {
> > accel = "tcg:kvm";
> > }
> > + } else if (have_kvm) {
> > + accel = "kvm";
> > + } else if (have_tcg) {
> > + accel = "tcg";
> > + } else {
> > + error_report("No accelerator selected and"
> > + " no default accelerator available");
> > + exit(1);
> > }
> > }
> > -
> > accel_list = g_strsplit(accel, ":", 0);
> >
> > for (tmp = accel_list; *tmp; tmp++) {
>
> Anyway:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
>
With or without Alex' hint:
Reviewed by: Aleksandar Markovic <amarkovic@wavecomp.com>
Alex' hint could be a separate patch. If you decide to do it, for that
patch too you can include my r-b.
--
> Alex Bennée
>
>
[-- Attachment #2: Type: text/html, Size: 4862 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] vl: Fixes for cleanups to -accel
2020-01-09 2:17 [PATCH 0/4] vl: Fixes for cleanups to -accel Richard Henderson
` (3 preceding siblings ...)
2020-01-09 2:17 ` [PATCH 4/4] vl: Only choose enabled accelerators " Richard Henderson
@ 2020-01-09 9:00 ` Paolo Bonzini
4 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2020-01-09 9:00 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 09/01/20 03:17, Richard Henderson wrote:
> Running qemu-system-foo with no options should not generate
> a warning for "invalid accelerator bar".
>
> Also, fix some mistakes made while moving the code from accel/accel.c.
>
>
> r~
>
>
> Richard Henderson (4):
> vl: Remove unused variable in configure_accelerators
> vl: Free accel_list in configure_accelerators
> vl: Remove useless test in configure_accelerators
> vl: Only choose enabled accelerators in configure_accelerators
>
> vl.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
I've queued it, but feel free to beat me to sending a pull request.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread