On Thu, Jul 24, 2014 at 5:46 PM, Ilya Dryomov <ilya.dryomov@xxxxxxxxxxx> 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..
BTW, while we are on the subject of struct rbd_mapping, is there any
particular reason to keep around both the base image values and actual
mapping values? I am tempted to change the mapping sequence so that 1)
snap context is read in immediately after watch setup, before anything
else is done, 2) supplied snap name is resolved into an id and 3) the
actual (i.e. based on snap_id) mapping size, features, parent_overlap,
etc are read in directly into struct rbd_image_header. That would
allow to rip struct rbd_mapping entirely and make the code more clear.
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
