On 1/31/18 7:36 AM, Anand Jain wrote:
>
>
> On 01/31/2018 09:42 PM, Nikolay Borisov wrote:
>
>
>>>> So usually this should be functionality handled by the raid/san
>>>> controller I guess, > but given that btrfs is playing the role of a
>>>> controller here at what point are we drawing the line of not
>>>> implementing block-level functionality into the filesystem ?
>>>
>>> Don't worry this is not invading into the block layer. How
>>> can you even build this functionality in the block layer ?
>>> Block layer even won't know that disks are mirrored. RAID
>>> does or BTRFS in our case.
>>>
>>
>> By block layer I guess I meant the storage driver of a particular raid
>> card. Because what is currently happening is re-implementing
>> functionality that will generally sit in the driver. So my question was
>> more generic and high-level - at what point do we draw the line of
>> implementing feature that are generally implemented in hardware devices
>> (be it their drivers or firmware).
>
> Not all HW configs use RAID capable HBAs. A server connected to a SATA
> JBOD using a SATA HBA without MD will relay on BTRFS to provide all the
> features and capabilities that otherwise would have provided by such a
> presumable HW config.
That does sort of sound like means implementing some portion of the
HBA features/capabilities in the filesystem.
To me it seems this this could be workable at the fs level, provided it
deals just with policies and remains hardware-neutral. However most
of the use cases appear to involve some hardware-dependent knowledge
or assumptions. What happens when someone sets this on a virtual disk,
or say a (persistent) memory-backed block device? Case #6 seems to
open up some potential for unexpected interactions (which may be hard
to reproduce, esp. in error/recovery scenarios).
Case #2 takes a devid, but I notice btrfs_device::devid says, "the
internal btrfs device id". How does a user obtain that internal value
so it can be set as a mount option?
Thanks,
Ed
>>>>> ::
>>>>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>>>>> index 39ba59832f38..478623e6e074 100644
>>>>>>> --- a/fs/btrfs/volumes.c
>>>>>>> +++ b/fs/btrfs/volumes.c
>>>>>>> @@ -5270,6 +5270,16 @@ static int find_live_mirror(struct
>>>>>>> btrfs_fs_info *fs_info,
>>>>>>> num = map->num_stripes;
>>>>>>> switch(fs_info->read_mirror_policy) {
>>>>>>> + case BTRFS_READ_MIRROR_BY_DEV:
>>>>>>> + optimal = first;
>>>>>>> + if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
>>>>>>> + &map->stripes[optimal].dev->dev_state))
>>>>>>> + break;
>>>>>>> + if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
>>>>>>> + &map->stripes[++optimal].dev->dev_state))
>>>>>>> + break;
>>>>>>> + optimal = first;
>>>>>>
>>>>>> you set optimal 2 times, the second one seems redundant.
>>>>>
>>>>> No actually. When both the disks containing the stripe does not
>>>>> have the BTRFS_DEV_STATE_READ_MIRROR, then I would just want to
>>>>> use first found stripe.
>>>>
>>>> Yes, and the fact that you've already set optimal = first right after
>>>> BTRFS_READ_MIRROR_BY_DEV ensures that, no ? Why do you need to again
>>>> set
>>>> optimal right before the final break? What am I missing here?
>>>
>>> Ah. I think you are missing ++optimal in the 2nd if.
>>
>> You are right, but I'd prefer you index the stripes array with 'optimal'
>> and 'optimal + 1' and leave just a single assignment
>
> Ok. Will improve that.
>
> Thanks, Anand
>
>
>>>
>>> Thanks, Anand
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
