[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_bl
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 01/23] block: Allow NULL file for bdrv_get_block_status() |
Date: |
Tue, 10 Oct 2017 14:00:48 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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
>> Is it asking for trouble to be updating pnum here before we undo our
>> alignment corrections? For readability reasons and preventing an
>> accidental context-based oopsy-daisy.
>
> As in, write the code to make all calculations in a temporary, and then
> assign *pnum only at the end? I suppose I can tweak the code along
> those lines, but I'm not sure it will make the end result any more legible.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v5 03/23] block: Make bdrv_round_to_clusters() signature more useful, Eric Blake, 2017/10/03
[Qemu-devel] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based, Eric Blake, 2017/10/03
[Qemu-devel] [PATCH v5 05/23] block: Switch bdrv_make_zero() to byte-based, Eric Blake, 2017/10/03
[Qemu-devel] [PATCH v5 06/23] qemu-img: Switch get_block_status() to byte-based, Eric Blake, 2017/10/03
[Qemu-devel] [PATCH v5 08/23] block: Switch bdrv_co_get_block_status() to byte-based, Eric Blake, 2017/10/03