On 07/24/2014 08:46 AM, Ilya Dryomov wrote:
> On Thu, Jul 24, 2014 at 5:25 PM, Alex Elder <elder@xxxxxxxx> wrote:
>> On 07/24/2014 03:42 AM, Ilya Dryomov wrote:
>>> There is no sense in trying to update the mapping size before it's even
>>> been set.
>>
>> It took me a bit to follow this. But basically there is
>> no mapping unless it's mapped. So previously this was
>> updating the mapping information even for unmapped
>> parent (or other) images. There's no need to update
>> the mapping size for a snapshot--it'll never change.
>>
>> Is that right? If not, please advise; otherwise:
>
> No. Previously it was updating the mapping size both on the inital map
> and on refresh (of the base image). Doing that on the inital map
> doesn't make sense: not only struct rbd_mapping fields aren't properly
> initialized at that point - rbd_dev_mapping_set() is called much later
> in the map sequence, snap_id isn't initialized either (at least in the
> format 2 case, I haven't looked too closely at the format 1 case). And
> just in general, trying to update something before it had a chance to
> change is bogus..
OK, now I see it. Hopefully this makes sense. Here is how it
was previously structured:
rbd_add()
do_rbd_add()
|rbd_dev_image_probe()
| rbd_dev_header_info()
| rbd_dev_v1_header_info() or rbd_dev_v2_header_info()
| rbd_header_from_disk() <update mapping>
| <update mapping>
| . . .
|rbd_dev_device_setup()
| rbd_dev_mapping_set()
So neither rbd_header_from_disk() nor rbd_dev_v2_header_info()
should be using the values in the rbd_dev->mapping field during
initial image probe, because they have not yet been initialized
at that point.
Meanwhile, the only reason we need to *update* a mapping size
is if we learn it may have changed, which will be covered by a
refresh, so doing so in rbd_dev_refresh() makes sense. And
as long as you know whether it's mapping the base image you
might as well do the rbd_exists_validate() call conditionally.
OK, this all looks good and makes good sense to me.
Thank you for explaining it.
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
>
> Thanks,
>
> Ilya
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
