Takeuchi-san
On Thu, 10 Jul 2014 16:33:23 +0900, Satoru Takeuchi wrote:
> (2014/07/10 12:05), Qu Wenruo wrote:
>> Before this patch, find_mount_root() and the caller both output error
>> message, which sometimes make the output duplicated and hard to judge
>> what the problem is.
>>
>> This pathh will integrate all the error messages output into
>> find_mount_root() to give more meaning error prompt and remove the
>> unneeded caller error messages.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
>> ---
>> cmds-receive.c | 2 --
>> cmds-send.c | 8 +-------
>> cmds-subvolume.c | 5 +----
>> utils.c | 15 ++++++++++++---
>> 4 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/cmds-receive.c b/cmds-receive.c
>> index 48380a5..084d97d 100644
>> --- a/cmds-receive.c
>> +++ b/cmds-receive.c
>> @@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
>> ret = find_mount_root(dest_dir_full_path, &r->root_path);
>> if (ret < 0) {
>> ret = -EINVAL;
>> - fprintf(stderr, "ERROR: failed to determine mount point "
>> - "for %s\n", dest_dir_full_path);
>> goto out;
>> }
>> r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
>> diff --git a/cmds-send.c b/cmds-send.c
>> index 9a73b32..091f32b 100644
>> --- a/cmds-send.c
>> +++ b/cmds-send.c
>> @@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
>> ret = find_mount_root(subvol, &s->root_path);
>> if (ret < 0) {
>> ret = -EINVAL;
>> - fprintf(stderr, "ERROR: failed to determine mount point "
>> - "for %s\n", subvol);
>> goto out;
>> }
>>
>> @@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
>> }
>>
>> ret = find_mount_root(subvol, &mount_root);
>> - if (ret < 0) {
>> - fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>> - "%s\n", subvol,
>> - strerror(-ret));
>> + if (ret < 0)
>> goto out;
>> - }
>> if (strcmp(send.root_path, mount_root) != 0) {
>> ret = -EINVAL;
>> fprintf(stderr, "ERROR: all subvols must be from the "
>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>> index 639fb10..b252eab 100644
>> --- a/cmds-subvolume.c
>> +++ b/cmds-subvolume.c
>> @@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
>> }
>>
>> ret = find_mount_root(fullpath, &mnt);
>> - if (ret < 0) {
>> - fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>> - "%s\n", fullpath, strerror(-ret));
>> + if (ret < 0)
>> goto out;
>> - }
>> ret = 1;
>> svpath = get_subvol_name(mnt, fullpath);
>>
>> diff --git a/utils.c b/utils.c
>> index 507ec6c..07173ee 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
>> char *longest_match = NULL;
>>
>> fd = open(path, O_RDONLY | O_NOATIME);
>> - if (fd < 0)
>> + if (fd < 0) {
>> + fprintf(stderr, "ERROR: Failed to open %s: %s\n",
>> + path, strerror(errno));
>
> It drops part of original messages. It doesn't show this error
> is from find_mount_root(). I consider the original meaning keep as is.
> How do you think?
I think it is strange for the common users to show the name of a internal function.
Maybe we should introduce two kinds of the message, one is for the common users,
the other is for the developers to debug.
Thanks
Miao
> Thanks,
> Satoru
>
>> return -errno;
>> + }
>> close(fd);
>>
>> mnttab = setmntent("/proc/self/mounts", "r");
>> - if (!mnttab)
>> + if (!mnttab) {
>> + fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
>> + strerror(errno));
>> return -errno;
>> + }
>>
>> while ((ent = getmntent(mnttab))) {
>> len = strlen(ent->mnt_dir);
>> @@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
>>
>> ret = 0;
>> *mount_root = realpath(longest_match, NULL);
>> - if (!*mount_root)
>> + if (!*mount_root) {
>> + fprintf(stderr, "Failed to resolve path %s: %s\n",
>> + longest_match, strerror(errno));
>> ret = -errno;
>> + }
>>
>> free(longest_match);
>> return ret;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
