On 10/11/2017 04:42 AM, Kevin Wolf wrote: > Am 10.10.2017 um 21:24 hat John Snow geschrieben: >> >> >> On 10/10/2017 03:00 PM, Eric Blake wrote: >>> On 10/10/2017 09:43 AM, Eric Blake wrote: >>> >>>>>> --- >>>>>> v5: use second label for cleaner exit logic [John], use local_pnum >>>> >>>>>> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn >>>>>> bdrv_co_get_block_status(BlockDriverState *bs, >>>>>> int64_t total_sectors; >>>>>> int64_t n; >>>>>> int64_t ret, ret2; >>>>>> + BlockDriverState *local_file = NULL; >>>>>> + int local_pnum = 0; >>>>> >>>>> I don't quite see what the point of local_pnum is if we assert anyway >>>>> that the real pnum is non-NULL. >>>> >>>> I did it in parallel with fallout from John's review on v4: >>>> https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06958.html >>>> >>>> but since it wasn't specifically asked for, and is now getting >>>> questions, I'm fine with not having it in v6. >>> >>> Okay, I re-read v4, and here's the comment (on 21/23) that led to my >>> experiment in v5 patch 1 with local_pnum: >>> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00293.html >>> >>> and I did argue: >>> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00311.html >> >> Well, Kevin's the boss :D > > I'm not sure how renaming *pnum into local_pnum addresses your concerns? > We still update local_pnum before we undo our alignment corrections. Or > are you talking about some other part of these mails? > > Kevin >
I made only a stylistic comment and it looked to me at a glance like it had resulted in conflicting feedback from you (since I all but asked for a local pnum) -- but my feedback was only minor stylistic stuff, so I'm deferring to you. --js