| Message ID | 20170710083553.6167-1-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 07/10 11:35, Denis V. Lunev wrote: > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) > CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) Maybe use "--noroles"? The parenthesis part is not really useful. > --- > block/io.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/block/io.c b/block/io.c > index ed31810c0a..e5c6dc77d3 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, > assert(max_pdiscard >= bs->bl.request_alignment); > > while (count > 0) { > - int ret; > int num = count; > > if (head) { > -- > 2.11.0 > > Reviewed-by: Fam Zheng <famz@redhat.com>
On 07/10/2017 11:56 AM, Fam Zheng wrote: > On Mon, 07/10 11:35, Denis V. Lunev wrote: >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) >> CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) > Maybe use "--noroles"? The parenthesis part is not really useful. good thing. May be this should be default. I have missed to clear that at commit stage. Den >> --- >> block/io.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/block/io.c b/block/io.c >> index ed31810c0a..e5c6dc77d3 100644 >> --- a/block/io.c >> +++ b/block/io.c >> @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, >> assert(max_pdiscard >= bs->bl.request_alignment); >> >> while (count > 0) { >> - int ret; >> int num = count; >> >> if (head) { >> -- >> 2.11.0 >> >> > Reviewed-by: Fam Zheng <famz@redhat.com>
On 07/10/2017 03:35 AM, Denis V. Lunev wrote: > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) > CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) The commit message is a bit sparse; I might have written: block: fix shadowed variable in bdrv_co_pdiscard We've had a shadowed 'ret' variable, which risks returning the wrong value, introduced in commit b9c64947. > --- > block/io.c | 1 - > 1 file changed, 1 deletion(-) Adding qemu-stable in cc, and you can add: Reviewed-by: Eric Blake <eblake@redhat.com> [and I wonder why gcc doesn't flag the shadowed variable for me - I definitely touched that code in commit 9f1963b3, but didn't notice that I kept the shadowing in place. I guess we don't have that compiler warning flag turned on for default configuration] > > diff --git a/block/io.c b/block/io.c > index ed31810c0a..e5c6dc77d3 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, > assert(max_pdiscard >= bs->bl.request_alignment); > > while (count > 0) { > - int ret; > int num = count; > > if (head) { >
Am 10.07.2017 um 10:35 hat Denis V. Lunev geschrieben: > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) > CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) > --- > block/io.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/block/io.c b/block/io.c > index ed31810c0a..e5c6dc77d3 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, > assert(max_pdiscard >= bs->bl.request_alignment); > > while (count > 0) { > - int ret; > int num = count; > > if (head) { Good catch, and the fix looks fine. This needs rebasing, though (conflicts with commit f5a5ca7 'block: change variable names in BlockDriverState'). Kevin
On 07/10/2017 04:51 PM, Eric Blake wrote: > On 07/10/2017 03:35 AM, Denis V. Lunev wrote: >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) >> CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) > The commit message is a bit sparse; I might have written: > > block: fix shadowed variable in bdrv_co_pdiscard > > We've had a shadowed 'ret' variable, which risks returning the wrong > value, introduced in commit b9c64947. thank you for the proposal. Den
On 07/10/2017 04:59 PM, Kevin Wolf wrote: > Am 10.07.2017 um 10:35 hat Denis V. Lunev geschrieben: >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) >> CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) >> --- >> block/io.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/block/io.c b/block/io.c >> index ed31810c0a..e5c6dc77d3 100644 >> --- a/block/io.c >> +++ b/block/io.c >> @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, >> assert(max_pdiscard >= bs->bl.request_alignment); >> >> while (count > 0) { >> - int ret; >> int num = count; >> >> if (head) { > Good catch, and the fix looks fine. > > This needs rebasing, though (conflicts with commit f5a5ca7 'block: > change variable names in BlockDriverState'). > > Kevin ok, will re-spin. Den
diff --git a/block/io.c b/block/io.c index ed31810c0a..e5c6dc77d3 100644 --- a/block/io.c +++ b/block/io.c @@ -2464,7 +2464,6 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, assert(max_pdiscard >= bs->bl.request_alignment); while (count > 0) { - int ret; int num = count; if (head) {
Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block I/O path) CC: Fam Zheng <famz@redhat.com> (supporter:Block I/O path) --- block/io.c | 1 - 1 file changed, 1 deletion(-)