On 08/30/12 14:28, Anand Jain wrote:
>
>> The original patch could be revised with this support easily.
>> How about using one structure and one ioctl number for both of them?
>> i.e,
>>
>> #define BTRFS_IOC_FSLABEL_CTL _IOW(BTRFS_IOCTL_MAGIC, 50, struct
>> btrfs_ioctl_fslabel_ctl_args)
>>
>> #define BTRFS_FSLABEL_CTL_GET 0
>> #define BTRFS_FSLABEL_CTL_SET 1
>>
>> struct btrfs_ioctl_fslabel_ctl_args {
>> char label[BTRFS_LABEL_SIZE];
>> u32 flags;
>> };
>>
>> so that get/set label from user tools would looks like,
>>
>> struct btrfs_ioctl_fslabel_ctl_args arg;
>> arg.flags = BTRFS_FSLABEL_CTL_GET; /* get label */
>> or
>> arg.flags = BTRFS_FSLABEL_CTL_SET; /* set label */
>> ....
>>
>> ioctl(fd, BTRFS_FSLABEL_CTL,&arg);
>
>
> I would prefer separating GET and SET label ioctl,
> (to have one operation with an ioctl define) that
> will be similar to rest of the ioctl in btrfs.
> Further we don't need ioctl-arg-struct here, unless
> if you are keeping the flags.
>
Thanks for the suggestions.
Yes, I noticed Btrfs perform add/rm devices, as well as set/set flags,
etc... all with two separated ioctls,
It's better to make the code style in a manner consistent with them.
However, get/set label is assigned one number in previous, the next
number(51) has been assigned to David,
51 - compressed file size - David Sterba
To make the code style in a consistent manner,
We can re-assign those numbers if Chris and David would happy to hear that.
> And in my understanding in kernel memcpy are better
> instead of strcpy.
If we do copy binary structure/elements, memcpy is nice.
However, consider we're copying a label in string, I think the
difference between them in kernel is
same to the user lib, which means that the copy is stops or not when it
encounters a NUL.
By contrast , I think strcpy is better here, if the label is specified
with a NUL('\0', why do that?) in the middle of it,
and we do memcpy for it in kernel, but, the user will get surprised
when he/she fetch the label and printf(3) it out.
Thanks,
-Jeff
>
> If you could add GET that will be nicer.
>
> I would need this for an experiment to add the label
> for the subvol/snapshots.
>
> 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
