From: Jun Li <junmuzi@gmail.com>
To: Eric Blake <eblake@redhat.com>
Cc: kwolf@redhat.com, famz@redhat.com, juli@redhat.com,
qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH] Modify qemu_opt_rename to realize renaming all items in opts
Date: Wed, 24 Sep 2014 13:23:16 +0800 [thread overview]
Message-ID: <20140924052315.GA8021@localhost.localdomain> (raw)
In-Reply-To: <5421A412.2080505@redhat.com>
On Tue, 09/23 10:47, Eric Blake wrote:
> On 09/23/2014 07:13 AM, Jun Li wrote:
> > Add realization of rename all items in opts for qemu_opt_rename.
> > e.g:
> > When add bps twice in command line, need to rename all bps to
> > throttling.bps-total.
> >
> > Signed-off-by: Jun Li <junmuzi@gmail.com>
> > ---
> > This patch solved following bug:
> > Bug 1145586 - qemu-kvm will give strange hint when add bps twice for a drive
> > ref:https://bugzilla.redhat.com/show_bug.cgi?id=1145586
>
> Including that bug link in the commit message might be nice for someone
> visiting this patch a year from now.
ok, got it.
>
> > ---
> > blockdev.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index b361fbb..7c39a06 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -536,10 +536,15 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
> > {
> > const char *value;
> >
> > - value = qemu_opt_get(opts, from);
> > - if (value) {
> > - qemu_opt_set(opts, to, value);
> > - qemu_opt_unset(opts, from);
> > + /* rename all items */
> > + while (1) {
> > + value = qemu_opt_get(opts, from);
>
> Can't this just be written as:
>
> while ((value = qemu_opt_get(opts, from))) {
>
> > + if (value) {
> > + qemu_opt_set(opts, to, value);
> > + qemu_opt_unset(opts, from);
> > + } else {
> > + break;
> > + }
>
> and lose the if/else and break? But that's style, not functional, so:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks. I will submit a new version.
Best Regards,
Jun Li
prev parent reply other threads:[~2014-09-24 5:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-23 13:13 [Qemu-devel] [PATCH] Modify qemu_opt_rename to realize renaming all items in opts Jun Li
2014-09-23 16:47 ` Eric Blake
2014-09-24 5:23 ` Jun Li [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140924052315.GA8021@localhost.localdomain \
--to=junmuzi@gmail.com \
--cc=eblake@redhat.com \
--cc=famz@redhat.com \
--cc=juli@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.