* [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
@ 2019-08-21 6:10 Liu Bo
2019-08-21 6:17 ` Eryu Guan
2019-08-22 20:16 ` Vivek Goyal
0 siblings, 2 replies; 6+ messages in thread
From: Liu Bo @ 2019-08-21 6:10 UTC (permalink / raw)
To: virtio-fs
Set ret = 0 if everything runs fine.
Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
fs/fuse/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index eb7543a..d3c5e45 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
out_fill_iomap:
fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
+ ret = 0;
out_err:
up_write(&fi->i_dmap_sem);
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
2019-08-21 6:10 [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping Liu Bo
@ 2019-08-21 6:17 ` Eryu Guan
2019-08-21 6:19 ` piaojun
2019-08-22 20:16 ` Vivek Goyal
1 sibling, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2019-08-21 6:17 UTC (permalink / raw)
To: Liu Bo; +Cc: virtio-fs
On Wed, Aug 21, 2019 at 02:10:38PM +0800, Liu Bo wrote:
> Set ret = 0 if everything runs fine.
>
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
FYI, this fixes generic/344 failure.
> ---
> fs/fuse/file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index eb7543a..d3c5e45 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
>
> out_fill_iomap:
> fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
> + ret = 0;
I'd prefer setting ret in the if (dmap->writable) case, which seems more
clear to me. i.e.
if (dmap->writable) {
ret = 0;
goto out_fill_iomap;
}
Anyway,
Reviewed-by: Eryu Guan <eguan@linux.alibaba.com>
Thanks!
Eryu
> out_err:
> up_write(&fi->i_dmap_sem);
> return ret;
> --
> 1.8.3.1
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
2019-08-21 6:17 ` Eryu Guan
@ 2019-08-21 6:19 ` piaojun
0 siblings, 0 replies; 6+ messages in thread
From: piaojun @ 2019-08-21 6:19 UTC (permalink / raw)
To: Eryu Guan, Liu Bo; +Cc: virtio-fs
On 2019/8/21 14:17, Eryu Guan wrote:
> On Wed, Aug 21, 2019 at 02:10:38PM +0800, Liu Bo wrote:
>> Set ret = 0 if everything runs fine.
>>
>> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>
> FYI, this fixes generic/344 failure.
>
>> ---
>> fs/fuse/file.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index eb7543a..d3c5e45 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
>>
>> out_fill_iomap:
>> fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
>> + ret = 0;
>
> I'd prefer setting ret in the if (dmap->writable) case, which seems more
> clear to me. i.e.
>
> if (dmap->writable) {
> ret = 0;
> goto out_fill_iomap;
> }
Agreed, and feel free to add:
Reviewed-by: Jun Piao <piaojun@huawei.com>
Jun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
2019-08-21 6:10 [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping Liu Bo
2019-08-21 6:17 ` Eryu Guan
@ 2019-08-22 20:16 ` Vivek Goyal
2019-08-23 0:22 ` piaojun
1 sibling, 1 reply; 6+ messages in thread
From: Vivek Goyal @ 2019-08-22 20:16 UTC (permalink / raw)
To: Liu Bo; +Cc: virtio-fs
On Wed, Aug 21, 2019 at 02:10:38PM +0800, Liu Bo wrote:
> Set ret = 0 if everything runs fine.
>
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> ---
> fs/fuse/file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index eb7543a..d3c5e45 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
>
> out_fill_iomap:
> fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
> + ret = 0;
Thanks. I initialized ret = 0 in the beginning itself now.
Vivek
> out_err:
> up_write(&fi->i_dmap_sem);
> return ret;
> --
> 1.8.3.1
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
2019-08-22 20:16 ` Vivek Goyal
@ 2019-08-23 0:22 ` piaojun
2019-08-23 12:52 ` Vivek Goyal
0 siblings, 1 reply; 6+ messages in thread
From: piaojun @ 2019-08-23 0:22 UTC (permalink / raw)
To: Vivek Goyal, Liu Bo; +Cc: virtio-fs
On 2019/8/23 4:16, Vivek Goyal wrote:
> On Wed, Aug 21, 2019 at 02:10:38PM +0800, Liu Bo wrote:
>> Set ret = 0 if everything runs fine.
>>
>> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>> ---
>> fs/fuse/file.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index eb7543a..d3c5e45 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
>>
>> out_fill_iomap:
>> fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
>> + ret = 0;
>
> Thanks. I initialized ret = 0 in the beginning itself now.
This probably would not work as ret will be set -EIO again immediately.
Jun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping
2019-08-23 0:22 ` piaojun
@ 2019-08-23 12:52 ` Vivek Goyal
0 siblings, 0 replies; 6+ messages in thread
From: Vivek Goyal @ 2019-08-23 12:52 UTC (permalink / raw)
To: piaojun; +Cc: virtio-fs
On Fri, Aug 23, 2019 at 08:22:20AM +0800, piaojun wrote:
>
>
> On 2019/8/23 4:16, Vivek Goyal wrote:
> > On Wed, Aug 21, 2019 at 02:10:38PM +0800, Liu Bo wrote:
> >> Set ret = 0 if everything runs fine.
> >>
> >> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> >> ---
> >> fs/fuse/file.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >> index eb7543a..d3c5e45 100644
> >> --- a/fs/fuse/file.c
> >> +++ b/fs/fuse/file.c
> >> @@ -1985,6 +1985,7 @@ static int iomap_begin_upgrade_mapping(struct inode *inode, loff_t pos,
> >>
> >> out_fill_iomap:
> >> fuse_fill_iomap(inode, pos, length, iomap, dmap, flags);
> >> + ret = 0;
> >
> > Thanks. I initialized ret = 0 in the beginning itself now.
>
> This probably would not work as ret will be set -EIO again immediately.
I changed that ret = -EIO as well. I now set it only when error happens.
Vivek
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-23 12:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 6:10 [Virtio-fs] [PATCH] virtiofs: fix return value of iomap_begin_upgrade_mapping Liu Bo
2019-08-21 6:17 ` Eryu Guan
2019-08-21 6:19 ` piaojun
2019-08-22 20:16 ` Vivek Goyal
2019-08-23 0:22 ` piaojun
2019-08-23 12:52 ` Vivek Goyal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.