* [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() @ 2014-03-27 1:16 Chen Gang 2014-03-27 7:55 ` Marcel Apfelbaum 2014-03-27 8:59 ` Markus Armbruster 0 siblings, 2 replies; 23+ messages in thread From: Chen Gang @ 2014-03-27 1:16 UTC (permalink / raw) To: aliguori, QEMU Developers At present, each 'opt_name' of 'accel_list' is uniq with each other, so 'buf' can only match one 'opt_name'. When drop into the matching code block, can 'break' outside related 'for' looping after finish processing it (just like the other 'break' within the matching block). After print "... not support for this target", it can avoid to print "... accelerator does not exist". Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 842e897..b4f98fa 100644 --- a/vl.c +++ b/vl.c @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) if (!accel_list[i].available()) { printf("%s not supported for this target\n", accel_list[i].name); - continue; + break; } *(accel_list[i].allowed) = true; ret = accel_list[i].init(machine); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-27 1:16 [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() Chen Gang @ 2014-03-27 7:55 ` Marcel Apfelbaum 2014-03-27 9:54 ` Chen Gang 2014-03-27 8:59 ` Markus Armbruster 1 sibling, 1 reply; 23+ messages in thread From: Marcel Apfelbaum @ 2014-03-27 7:55 UTC (permalink / raw) To: Chen Gang; +Cc: QEMU Developers, aliguori On Thu, 2014-03-27 at 09:16 +0800, Chen Gang wrote: > At present, each 'opt_name' of 'accel_list' is uniq with each other, so > 'buf' can only match one 'opt_name'. > > When drop into the matching code block, can 'break' outside related > 'for' looping after finish processing it (just like the other 'break' > within the matching block). > > After print "... not support for this target", it can avoid to print > "... accelerator does not exist". Hi, Thanks for the patch! I would re-write 1. The commit subject to: Fix wrong warning on configure_accelerator 2. The commit message to: No need to continue to iterate over the accelerators list after a match is found, even if it is not supported. > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > vl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/vl.c b/vl.c > index 842e897..b4f98fa 100644 > --- a/vl.c > +++ b/vl.c > @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) > if (!accel_list[i].available()) { > printf("%s not supported for this target\n", > accel_list[i].name); > - continue; > + break; Seems like the right thing to do. Thanks, Marcel > } > *(accel_list[i].allowed) = true; > ret = accel_list[i].init(machine); ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-27 7:55 ` Marcel Apfelbaum @ 2014-03-27 9:54 ` Chen Gang 0 siblings, 0 replies; 23+ messages in thread From: Chen Gang @ 2014-03-27 9:54 UTC (permalink / raw) To: marcel.a; +Cc: QEMU Developers, aliguori On 03/27/2014 03:55 PM, Marcel Apfelbaum wrote: > On Thu, 2014-03-27 at 09:16 +0800, Chen Gang wrote: >> At present, each 'opt_name' of 'accel_list' is uniq with each other, so >> 'buf' can only match one 'opt_name'. >> >> When drop into the matching code block, can 'break' outside related >> 'for' looping after finish processing it (just like the other 'break' >> within the matching block). >> >> After print "... not support for this target", it can avoid to print >> "... accelerator does not exist". > Hi, > Thanks for the patch! > > I would re-write > 1. The commit subject to: > Fix wrong warning on configure_accelerator > 2. The commit message to: > No need to continue to iterate over the accelerators list > after a match is found, even if it is not supported. > OK, thank you for your work (re-write it before commit it). Next I will/should notice about it. :-) >> >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >> --- >> vl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/vl.c b/vl.c >> index 842e897..b4f98fa 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) >> if (!accel_list[i].available()) { >> printf("%s not supported for this target\n", >> accel_list[i].name); >> - continue; >> + break; > Seems like the right thing to do. > > Thanks, > Marcel > >> } >> *(accel_list[i].allowed) = true; >> ret = accel_list[i].init(machine); > > > -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-27 1:16 [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() Chen Gang 2014-03-27 7:55 ` Marcel Apfelbaum @ 2014-03-27 8:59 ` Markus Armbruster 2014-03-27 10:01 ` Chen Gang 1 sibling, 1 reply; 23+ messages in thread From: Markus Armbruster @ 2014-03-27 8:59 UTC (permalink / raw) To: Chen Gang; +Cc: QEMU Developers, aliguori Chen Gang <gang.chen.5i5j@gmail.com> writes: > At present, each 'opt_name' of 'accel_list' is uniq with each other, so > 'buf' can only match one 'opt_name'. > > When drop into the matching code block, can 'break' outside related > 'for' looping after finish processing it (just like the other 'break' > within the matching block). > > After print "... not support for this target", it can avoid to print > "... accelerator does not exist". > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > vl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/vl.c b/vl.c > index 842e897..b4f98fa 100644 > --- a/vl.c > +++ b/vl.c > @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) > if (!accel_list[i].available()) { > printf("%s not supported for this target\n", > accel_list[i].name); > - continue; > + break; > } > *(accel_list[i].allowed) = true; > ret = accel_list[i].init(machine); Works, because the opt_name in accel_list[] are distinct, as you explained in your commit message. Apropos commit message. You first explain what you do, and only then state the problem you're trying to solve. That's backwards. Start with the problem. Only then explain your solution, if it needs explaining. This one, I think, doesn't. Suggested commit message: vl: Report accelerator not supported for target more nicely When you ask for an accelerator not supported for your target, you get a bogus "accelerator does not exist" message: $ qemu-system-arm -machine none,accel=kvm KVM not supported for this target "kvm" accelerator does not exist. No accelerator found! Suppress it. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-27 8:59 ` Markus Armbruster @ 2014-03-27 10:01 ` Chen Gang 2014-03-30 14:44 ` Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-03-27 10:01 UTC (permalink / raw) To: Markus Armbruster; +Cc: QEMU Developers, aliguori On 03/27/2014 04:59 PM, Markus Armbruster wrote: > Chen Gang <gang.chen.5i5j@gmail.com> writes: > >> At present, each 'opt_name' of 'accel_list' is uniq with each other, so >> 'buf' can only match one 'opt_name'. >> >> When drop into the matching code block, can 'break' outside related >> 'for' looping after finish processing it (just like the other 'break' >> within the matching block). >> >> After print "... not support for this target", it can avoid to print >> "... accelerator does not exist". >> >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >> --- >> vl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/vl.c b/vl.c >> index 842e897..b4f98fa 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) >> if (!accel_list[i].available()) { >> printf("%s not supported for this target\n", >> accel_list[i].name); >> - continue; >> + break; >> } >> *(accel_list[i].allowed) = true; >> ret = accel_list[i].init(machine); > > Works, because the opt_name in accel_list[] are distinct, as you > explained in your commit message. > > Apropos commit message. You first explain what you do, and only then > state the problem you're trying to solve. That's backwards. Start with > the problem. Only then explain your solution, if it needs explaining. > This one, I think, doesn't. > > Suggested commit message: > > vl: Report accelerator not supported for target more nicely > > When you ask for an accelerator not supported for your target, you > get a bogus "accelerator does not exist" message: > > $ qemu-system-arm -machine none,accel=kvm > KVM not supported for this target > "kvm" accelerator does not exist. > No accelerator found! > > Suppress it. > Thank you for your details, that sounds reasonable to me. Next, I will/should notice start with the issue, and then explain the solution. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-27 10:01 ` Chen Gang @ 2014-03-30 14:44 ` Chen Gang 2014-03-31 12:38 ` Markus Armbruster 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-03-30 14:44 UTC (permalink / raw) To: Markus Armbruster; +Cc: QEMU Developers, aliguori Hello Maintainers: If it is necessary to send patch v2 by me, please let me know, I will/should send. Thanks. On 03/27/2014 06:01 PM, Chen Gang wrote: > > > On 03/27/2014 04:59 PM, Markus Armbruster wrote: >> Chen Gang <gang.chen.5i5j@gmail.com> writes: >> >>> At present, each 'opt_name' of 'accel_list' is uniq with each other, so >>> 'buf' can only match one 'opt_name'. >>> >>> When drop into the matching code block, can 'break' outside related >>> 'for' looping after finish processing it (just like the other 'break' >>> within the matching block). >>> >>> After print "... not support for this target", it can avoid to print >>> "... accelerator does not exist". >>> >>> >>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >>> --- >>> vl.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/vl.c b/vl.c >>> index 842e897..b4f98fa 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) >>> if (!accel_list[i].available()) { >>> printf("%s not supported for this target\n", >>> accel_list[i].name); >>> - continue; >>> + break; >>> } >>> *(accel_list[i].allowed) = true; >>> ret = accel_list[i].init(machine); >> >> Works, because the opt_name in accel_list[] are distinct, as you >> explained in your commit message. >> >> Apropos commit message. You first explain what you do, and only then >> state the problem you're trying to solve. That's backwards. Start with >> the problem. Only then explain your solution, if it needs explaining. >> This one, I think, doesn't. >> >> Suggested commit message: >> >> vl: Report accelerator not supported for target more nicely >> >> When you ask for an accelerator not supported for your target, you >> get a bogus "accelerator does not exist" message: >> >> $ qemu-system-arm -machine none,accel=kvm >> KVM not supported for this target >> "kvm" accelerator does not exist. >> No accelerator found! >> >> Suppress it. >> > > Thank you for your details, that sounds reasonable to me. > > Next, I will/should notice start with the issue, and then explain the > solution. > > > Thanks. > -- Chen Gang Open, share and attitude like air, water and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-30 14:44 ` Chen Gang @ 2014-03-31 12:38 ` Markus Armbruster 2014-03-31 12:53 ` Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Markus Armbruster @ 2014-03-31 12:38 UTC (permalink / raw) To: Chen Gang; +Cc: QEMU Developers, aliguori Chen Gang <gang.chen.5i5j@gmail.com> writes: > Hello Maintainers: > > If it is necessary to send patch v2 by me, please let me know, I > will/should send. Not a maintainer, but if you send a v2 with an improved commit message, I'll R-by it, which can only help getting it merged. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 12:38 ` Markus Armbruster @ 2014-03-31 12:53 ` Chen Gang 2014-03-31 13:01 ` Peter Maydell 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-03-31 12:53 UTC (permalink / raw) To: Markus Armbruster; +Cc: QEMU Developers, aliguori On 03/31/2014 08:38 PM, Markus Armbruster wrote: > Chen Gang <gang.chen.5i5j@gmail.com> writes: > >> Hello Maintainers: >> >> If it is necessary to send patch v2 by me, please let me know, I >> will/should send. > > Not a maintainer, but if you send a v2 with an improved commit message, > I'll R-by it, which can only help getting it merged. > I guess your meaning is "not quite necessary" (for me, minor useful patches almost like spam). So if sending patch v2 is really required, please let me know, thanks. At present, I am just trying to know about Qemu by reading code (start from 'vl.c' and continue), if I can find related issue, I will try to fix it and send the related patch for it. Welcome any additional suggestions and completions for what I am doing. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 12:53 ` Chen Gang @ 2014-03-31 13:01 ` Peter Maydell 2014-03-31 13:12 ` Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Peter Maydell @ 2014-03-31 13:01 UTC (permalink / raw) To: Chen Gang Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 31 March 2014 13:53, Chen Gang <gang.chen.5i5j@gmail.com> wrote: > On 03/31/2014 08:38 PM, Markus Armbruster wrote: >> Chen Gang <gang.chen.5i5j@gmail.com> writes: >> >>> Hello Maintainers: >>> >>> If it is necessary to send patch v2 by me, please let me know, I >>> will/should send. >> >> Not a maintainer, but if you send a v2 with an improved commit message, >> I'll R-by it, which can only help getting it merged. >> > > I guess your meaning is "not quite necessary" (for me, minor useful > patches almost like spam). So if sending patch v2 is really required, > please let me know, thanks. Basically, asking a maintainer to make changes to a patch as they apply it is asking them to do extra work beyond what they would normally do. Sometimes people will agree to do this, but in general it's better just to send a fixed version of the patch yourself. (I've cc'd qemu-trivial since that's probably the best tree to take this patch.) thanks -- PMM ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 13:01 ` Peter Maydell @ 2014-03-31 13:12 ` Chen Gang 2014-03-31 13:16 ` Peter Maydell 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-03-31 13:12 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 03/31/2014 09:01 PM, Peter Maydell wrote: > On 31 March 2014 13:53, Chen Gang <gang.chen.5i5j@gmail.com> wrote: >> On 03/31/2014 08:38 PM, Markus Armbruster wrote: >>> Chen Gang <gang.chen.5i5j@gmail.com> writes: >>> >>>> Hello Maintainers: >>>> >>>> If it is necessary to send patch v2 by me, please let me know, I >>>> will/should send. >>> >>> Not a maintainer, but if you send a v2 with an improved commit message, >>> I'll R-by it, which can only help getting it merged. >>> >> >> I guess your meaning is "not quite necessary" (for me, minor useful >> patches almost like spam). So if sending patch v2 is really required, >> please let me know, thanks. > > Basically, asking a maintainer to make changes to a patch > as they apply it is asking them to do extra work beyond > what they would normally do. Sometimes people will agree > to do this, but in general it's better just to send a fixed > version of the patch yourself. > > (I've cc'd qemu-trivial since that's probably the best tree > to take this patch.) > OK, thanks. And excuse me, my English is not quite well, I guess, I misunderstood the original replier's meaning. I will/should send patch v2 for it within this week (2014-04-06). Next, when I send trivial patches, I will/should cc to qemu-trivial. I guess, most of my future patches will be trivial patches (and for me, trivial != minor). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 13:12 ` Chen Gang @ 2014-03-31 13:16 ` Peter Maydell 2014-03-31 13:26 ` Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Peter Maydell @ 2014-03-31 13:16 UTC (permalink / raw) To: Chen Gang Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 31 March 2014 14:12, Chen Gang <gang.chen.5i5j@gmail.com> wrote: > Next, when I send trivial patches, I will/should cc to qemu-trivial. I > guess, most of my future patches will be trivial patches (and for me, > trivial != minor). We describe on the wiki what we mean by 'trivial': http://wiki.qemu.org/Contribute/TrivialPatches thanks -- PMM ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 13:16 ` Peter Maydell @ 2014-03-31 13:26 ` Chen Gang 2014-03-31 13:33 ` Peter Maydell 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-03-31 13:26 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 03/31/2014 09:16 PM, Peter Maydell wrote: > On 31 March 2014 14:12, Chen Gang <gang.chen.5i5j@gmail.com> wrote: >> Next, when I send trivial patches, I will/should cc to qemu-trivial. I >> guess, most of my future patches will be trivial patches (and for me, >> trivial != minor). > > We describe on the wiki what we mean by 'trivial': > http://wiki.qemu.org/Contribute/TrivialPatches > Thank you for your information. Next, when I send trivial patches, I will only send to qemu-trivial (not send/cc to qemu-devel again), that will be more efficient. :-) Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 13:26 ` Chen Gang @ 2014-03-31 13:33 ` Peter Maydell 2014-03-31 23:50 ` Chen Gang 2014-04-04 9:39 ` [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely Chen Gang 0 siblings, 2 replies; 23+ messages in thread From: Peter Maydell @ 2014-03-31 13:33 UTC (permalink / raw) To: Chen Gang Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 31 March 2014 14:26, Chen Gang <gang.chen.5i5j@gmail.com> wrote: > Next, when I send trivial patches, I will only send to qemu-trivial (not > send/cc to qemu-devel again), that will be more efficient. :-) No, please always send to qemu-devel; just also cc qemu-trivial (or the relevant subsystem maintainers as listed in MAINTAINERS) if appropriate. Not everybody subscribes to qemu-trivial. thanks -- PMM ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() 2014-03-31 13:33 ` Peter Maydell @ 2014-03-31 23:50 ` Chen Gang 2014-04-04 9:39 ` [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely Chen Gang 1 sibling, 0 replies; 23+ messages in thread From: Chen Gang @ 2014-03-31 23:50 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers On 03/31/2014 09:33 PM, Peter Maydell wrote: > On 31 March 2014 14:26, Chen Gang <gang.chen.5i5j@gmail.com> wrote: >> Next, when I send trivial patches, I will only send to qemu-trivial (not >> send/cc to qemu-devel again), that will be more efficient. :-) > > No, please always send to qemu-devel; just also cc qemu-trivial > (or the relevant subsystem maintainers as listed in MAINTAINERS) > if appropriate. Not everybody subscribes to qemu-trivial. > OK, thanks. And when send trivial patch, I will/should always mark it as "[PATCH trivial]" to let another members know about it easily, and always cc to qemu-trivial. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely 2014-03-31 13:33 ` Peter Maydell 2014-03-31 23:50 ` Chen Gang @ 2014-04-04 9:39 ` Chen Gang 2014-04-04 10:57 ` Markus Armbruster 2014-04-06 6:32 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev 1 sibling, 2 replies; 23+ messages in thread From: Chen Gang @ 2014-04-04 9:39 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Trivial, Markus Armbruster, Anthony Liguori, QEMU Developers When you ask for an accelerator not supported for your target, you get a bogus "accelerator does not exist" message: $ qemu-system-arm -machine none,accel=kvm KVM not supported for this target "kvm" accelerator does not exist. No accelerator found! Suppress it. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 842e897..b4f98fa 100644 --- a/vl.c +++ b/vl.c @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) if (!accel_list[i].available()) { printf("%s not supported for this target\n", accel_list[i].name); - continue; + break; } *(accel_list[i].allowed) = true; ret = accel_list[i].init(machine); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely 2014-04-04 9:39 ` [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely Chen Gang @ 2014-04-04 10:57 ` Markus Armbruster 2014-04-06 12:30 ` Chen Gang 2014-04-06 6:32 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev 1 sibling, 1 reply; 23+ messages in thread From: Markus Armbruster @ 2014-04-04 10:57 UTC (permalink / raw) To: Chen Gang; +Cc: QEMU Trivial, Peter Maydell, QEMU Developers, Anthony Liguori Chen Gang <gang.chen.5i5j@gmail.com> writes: > When you ask for an accelerator not supported for your target, you get > a bogus "accelerator does not exist" message: > > $ qemu-system-arm -machine none,accel=kvm > KVM not supported for this target > "kvm" accelerator does not exist. > No accelerator found! > > Suppress it. > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely 2014-04-04 10:57 ` Markus Armbruster @ 2014-04-06 12:30 ` Chen Gang 0 siblings, 0 replies; 23+ messages in thread From: Chen Gang @ 2014-04-06 12:30 UTC (permalink / raw) To: Markus Armbruster Cc: QEMU Trivial, Peter Maydell, QEMU Developers, Anthony Liguori On 04/04/2014 06:57 PM, Markus Armbruster wrote: > Chen Gang <gang.chen.5i5j@gmail.com> writes: > >> When you ask for an accelerator not supported for your target, you get >> a bogus "accelerator does not exist" message: >> >> $ qemu-system-arm -machine none,accel=kvm >> KVM not supported for this target >> "kvm" accelerator does not exist. >> No accelerator found! >> >> Suppress it. >> >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely 2014-04-04 9:39 ` [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely Chen Gang 2014-04-04 10:57 ` Markus Armbruster @ 2014-04-06 6:32 ` Michael Tokarev 2014-04-06 12:32 ` Chen Gang 1 sibling, 1 reply; 23+ messages in thread From: Michael Tokarev @ 2014-04-06 6:32 UTC (permalink / raw) To: Chen Gang Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers 04.04.2014 13:39, Chen Gang wrote: > When you ask for an accelerator not supported for your target, you get > a bogus "accelerator does not exist" message: > > $ qemu-system-arm -machine none,accel=kvm > KVM not supported for this target > "kvm" accelerator does not exist. > No accelerator found! > > Suppress it. Applied to -trivial, thanks! /mjt ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely 2014-04-06 6:32 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev @ 2014-04-06 12:32 ` Chen Gang 2014-04-08 12:00 ` [Qemu-devel] [PATCH trivial 0/3] vl: simplify code for main() and get_boot_device() Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-04-06 12:32 UTC (permalink / raw) To: Michael Tokarev Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers On 04/06/2014 02:32 PM, Michael Tokarev wrote: > 04.04.2014 13:39, Chen Gang wrote: >> When you ask for an accelerator not supported for your target, you get >> a bogus "accelerator does not exist" message: >> >> $ qemu-system-arm -machine none,accel=kvm >> KVM not supported for this target >> "kvm" accelerator does not exist. >> No accelerator found! >> >> Suppress it. > > Applied to -trivial, thanks! > > /mjt > Thanks ! -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH trivial 0/3] vl: simplify code for main() and get_boot_device() 2014-04-06 12:32 ` Chen Gang @ 2014-04-08 12:00 ` Chen Gang 2014-04-08 12:01 ` [Qemu-devel] [PATCH trivial 1/3] vl: remove useless 'continue' Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-04-08 12:00 UTC (permalink / raw) To: Michael Tokarev Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers In "vl.c", at least, we can simplify the code below, so can let readers read professional C code (especially for new readers, which often start reading code at main function). - remove useless 'continue' in main(). - remove redundant local variable 'res' in get_boot_device(). - remove local variable 'args' in the middle of code block in main(). The following 3 patches are for the 3 'remove' above. And "vl.c" has a very long function main() which is about 17K lines. Next, it can be split into sub-functions (so can bypass another code style issue: multiple "{...}" styles within "swith(...)"). Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) ^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH trivial 1/3] vl: remove useless 'continue' 2014-04-08 12:00 ` [Qemu-devel] [PATCH trivial 0/3] vl: simplify code for main() and get_boot_device() Chen Gang @ 2014-04-08 12:01 ` Chen Gang 2014-04-08 12:02 ` [Qemu-devel] [PATCH trivial 2/3] vl: remove redundant local variable 'res' Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-04-08 12:01 UTC (permalink / raw) To: Michael Tokarev Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers Normal "if (...) {...} else {...}" is enough in "while(...) {...}", not need additional useless 'continue'. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/vl.c b/vl.c index 9975e5a..7505002 100644 --- a/vl.c +++ b/vl.c @@ -3034,7 +3034,6 @@ int main(int argc, char **argv, char **envp) if (argv[optind][0] != '-') { /* disk image */ optind++; - continue; } else { const QEMUOption *popt; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH trivial 2/3] vl: remove redundant local variable 'res' 2014-04-08 12:01 ` [Qemu-devel] [PATCH trivial 1/3] vl: remove useless 'continue' Chen Gang @ 2014-04-08 12:02 ` Chen Gang 2014-04-08 12:05 ` [Qemu-devel] [PATCH trivial 3/3] vl: remove local variable 'args' in the middle of code block Chen Gang 0 siblings, 1 reply; 23+ messages in thread From: Chen Gang @ 2014-04-08 12:02 UTC (permalink / raw) To: Michael Tokarev Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers In function, if no additional resources to free before quit, commonly, need not use additional local variable 'res' to notice about it. So remove it to simplify code. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index 7505002..377f962 100644 --- a/vl.c +++ b/vl.c @@ -1188,18 +1188,16 @@ DeviceState *get_boot_device(uint32_t position) { uint32_t counter = 0; FWBootEntry *i = NULL; - DeviceState *res = NULL; if (!QTAILQ_EMPTY(&fw_boot_order)) { QTAILQ_FOREACH(i, &fw_boot_order, link) { if (counter == position) { - res = i->dev; - break; + return i->dev; } counter++; } } - return res; + return NULL; } /* -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH trivial 3/3] vl: remove local variable 'args' in the middle of code block 2014-04-08 12:02 ` [Qemu-devel] [PATCH trivial 2/3] vl: remove redundant local variable 'res' Chen Gang @ 2014-04-08 12:05 ` Chen Gang 0 siblings, 0 replies; 23+ messages in thread From: Chen Gang @ 2014-04-08 12:05 UTC (permalink / raw) To: Michael Tokarev Cc: QEMU Trivial, Peter Maydell, Markus Armbruster, Anthony Liguori, QEMU Developers For C code, it does not recommend to define a local variable in the middle of code block without "{...}". The original author may want to zero members not mentioned in structure assignment. So recommend to use structure initializing block "{...}" for structure assignment in the middle of code block. And at present, we can assume that all related gcc versions will be latest enough to notice about this grammar. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/vl.c b/vl.c index 377f962..d381443 100644 --- a/vl.c +++ b/vl.c @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) qdev_machine_init(); - QEMUMachineInitArgs args = { .machine = machine, - .ram_size = ram_size, - .boot_order = boot_order, - .kernel_filename = kernel_filename, - .kernel_cmdline = kernel_cmdline, - .initrd_filename = initrd_filename, - .cpu_model = cpu_model }; - - current_machine->init_args = args; + current_machine->init_args = (QEMUMachineInitArgs) { + .machine = machine, + .ram_size = ram_size, + .boot_order = boot_order, + .kernel_filename = kernel_filename, + .kernel_cmdline = kernel_cmdline, + .initrd_filename = initrd_filename, + .cpu_model = cpu_model }; + machine->init(¤t_machine->init_args); audio_init(); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-04-08 12:05 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-03-27 1:16 [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() Chen Gang 2014-03-27 7:55 ` Marcel Apfelbaum 2014-03-27 9:54 ` Chen Gang 2014-03-27 8:59 ` Markus Armbruster 2014-03-27 10:01 ` Chen Gang 2014-03-30 14:44 ` Chen Gang 2014-03-31 12:38 ` Markus Armbruster 2014-03-31 12:53 ` Chen Gang 2014-03-31 13:01 ` Peter Maydell 2014-03-31 13:12 ` Chen Gang 2014-03-31 13:16 ` Peter Maydell 2014-03-31 13:26 ` Chen Gang 2014-03-31 13:33 ` Peter Maydell 2014-03-31 23:50 ` Chen Gang 2014-04-04 9:39 ` [Qemu-devel] [PATCH-trivial v2] vl: Report accelerator not supported for target more nicely Chen Gang 2014-04-04 10:57 ` Markus Armbruster 2014-04-06 12:30 ` Chen Gang 2014-04-06 6:32 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev 2014-04-06 12:32 ` Chen Gang 2014-04-08 12:00 ` [Qemu-devel] [PATCH trivial 0/3] vl: simplify code for main() and get_boot_device() Chen Gang 2014-04-08 12:01 ` [Qemu-devel] [PATCH trivial 1/3] vl: remove useless 'continue' Chen Gang 2014-04-08 12:02 ` [Qemu-devel] [PATCH trivial 2/3] vl: remove redundant local variable 'res' Chen Gang 2014-04-08 12:05 ` [Qemu-devel] [PATCH trivial 3/3] vl: remove local variable 'args' in the middle of code block Chen Gang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.