2018-02-09 07:12:25

by jiangyiwen

[permalink] [raw]
Subject: [V9fs-developer] [RFC] we should solve create-unlink-getattr idiom

Hi Eric and Greg,

I encountered the similar problem with create-unlink-getattr idiom.
I use the testcase that create-unlink-setattr idiom, and I see the
bug is reported at https://bugs.launchpad.net/qemu/+bug/1336794.
Then I also see you already fix the issue and push the patch to upstream.
https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
http://patchwork.ozlabs.org/patch/626194/

Unfortunately, the two patches are not merged into master, I don't know
the reason, so I suggest if the patche can be merged into master, and
it will solve the create-unlink-getattr idiom.

Thanks,
Yiwen



2018-02-09 07:23:02

by Veaceslav Falico

[permalink] [raw]
Subject: Re: [V9fs-developer] [RFC] we should solve create-unlink-getattr idiom

Hi Yiwen, all,

On 2/9/2018 8:10 AM, jiangyiwen wrote:
> Hi Eric and Greg,
>
> I encountered the similar problem with create-unlink-getattr idiom.
> I use the testcase that create-unlink-setattr idiom, and I see the
> bug is reported at https://bugs.launchpad.net/qemu/+bug/1336794.
> Then I also see you already fix the issue and push the patch to upstream.
> https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
> http://patchwork.ozlabs.org/patch/626194/
>
> Unfortunately, the two patches are not merged into master, I don't know
> the reason, so I suggest if the patche can be merged into master, and
> it will solve the create-unlink-getattr idiom.

As a follow up - the create-unlink-setattr (mainly ftruncate and anything
else which works on fd instead of path) isn't fixed by these patches, but
I'm currently working on a new patch, obviously on top of those two, to
make the setattr work too.

It's based on the same logic as the above patches though - use FIDs with
open fd's guest side and use open fd's host side if possible with f*
functions, otherwise path with l* functions.

It's bigger than the QEMU getattr patch, as there are no f* functions
available for ftruncate case, for example.

So if those two patches could be merged it'd be a lot easier to then
go forward with the setattr fix.

Thank you!

>
> Thanks,
> Yiwen
>
> .
>



2018-02-09 11:42:12

by Greg Kurz

[permalink] [raw]
Subject: Re: [V9fs-developer] [RFC] we should solve create-unlink-getattr idiom

On Fri, 9 Feb 2018 15:10:46 +0800
jiangyiwen <[email protected]> wrote:

> Hi Eric and Greg,
>
> I encountered the similar problem with create-unlink-getattr idiom.
> I use the testcase that create-unlink-setattr idiom, and I see the
> bug is reported at https://bugs.launchpad.net/qemu/+bug/1336794.
> Then I also see you already fix the issue and push the patch to upstream.
> https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
> http://patchwork.ozlabs.org/patch/626194/
>
> Unfortunately, the two patches are not merged into master, I don't know
> the reason, so I suggest if the patche can be merged into master, and
> it will solve the create-unlink-getattr idiom.
>

I had tried to go a bit further and address the general issue of f*() syscalls
versus unlinked files:

QEMU:
http://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07586.html

Linux 9p driver:
https://sourceforge.net/p/v9fs/mailman/message/35175775/

I remember that some issues were then reported during review of the
linux patches, and I never got bandwidth to investigate further...

But if you'd like to resurrect these threads, please do. :)

> Thanks,
> Yiwen
>

Cheers,

--
Greg

2018-02-09 12:33:20

by Greg Kurz

[permalink] [raw]
Subject: Re: [V9fs-developer] [RFC] we should solve create-unlink-getattr idiom

On Fri, 9 Feb 2018 08:21:52 +0100
Veaceslav Falico <[email protected]> wrote:

> Hi Yiwen, all,
>
> On 2/9/2018 8:10 AM, jiangyiwen wrote:
> > Hi Eric and Greg,
> >
> > I encountered the similar problem with create-unlink-getattr idiom.
> > I use the testcase that create-unlink-setattr idiom, and I see the
> > bug is reported at https://bugs.launchpad.net/qemu/+bug/1336794.
> > Then I also see you already fix the issue and push the patch to upstream.
> > https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
> > http://patchwork.ozlabs.org/patch/626194/
> >
> > Unfortunately, the two patches are not merged into master, I don't know
> > the reason, so I suggest if the patche can be merged into master, and
> > it will solve the create-unlink-getattr idiom.
>
> As a follow up - the create-unlink-setattr (mainly ftruncate and anything
> else which works on fd instead of path) isn't fixed by these patches, but
> I'm currently working on a new patch, obviously on top of those two, to
> make the setattr work too.
>
> It's based on the same logic as the above patches though - use FIDs with
> open fd's guest side and use open fd's host side if possible with f*
> functions, otherwise path with l* functions.
>
> It's bigger than the QEMU getattr patch, as there are no f* functions
> available for ftruncate case, for example.
>

As I was saying to Yiwen, maybe have a look at:

https://github.com/gkurz/qemu/commits/9p-attr-fixes

It is probably too old to rebase cleanly on current master, but it gives
the general idea.

IIRC, the cause for this not moving forward was because of an issue
unveiled/introduced by patch 3/3 in the linux 9p driver:

https://sourceforge.net/p/v9fs/mailman/v9fs-developer/thread/20160704141655.GA5799%40u-isr-cdi-08/#msg35199720

and a general lack of care for the 9p code at the time... but it seems
you guys are willing to go forward, and that's cool ! :)

Cheers,

--
Greg

> So if those two patches could be merged it'd be a lot easier to then
> go forward with the setattr fix.
>
> Thank you!
>
> >
> > Thanks,
> > Yiwen
> >
> > .
> >
>
>


2018-02-11 02:35:04

by jiangyiwen

[permalink] [raw]
Subject: Re: [V9fs-developer] [RFC] we should solve create-unlink-getattr idiom

On 2018/2/9 19:33, Greg Kurz wrote:
> On Fri, 9 Feb 2018 15:10:46 +0800
> jiangyiwen <[email protected]> wrote:
>
>> Hi Eric and Greg,
>>
>> I encountered the similar problem with create-unlink-getattr idiom.
>> I use the testcase that create-unlink-setattr idiom, and I see the
>> bug is reported at https://bugs.launchpad.net/qemu/+bug/1336794.
>> Then I also see you already fix the issue and push the patch to upstream.
>> https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
>> http://patchwork.ozlabs.org/patch/626194/
>>
>> Unfortunately, the two patches are not merged into master, I don't know
>> the reason, so I suggest if the patche can be merged into master, and
>> it will solve the create-unlink-getattr idiom.
>>
>
> I had tried to go a bit further and address the general issue of f*() syscalls
> versus unlinked files:
>
> QEMU:
> http://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07586.html
>
> Linux 9p driver:
> https://sourceforge.net/p/v9fs/mailman/message/35175775/
>
> I remember that some issues were then reported during review of the
> linux patches, and I never got bandwidth to investigate further...
>
> But if you'd like to resurrect these threads, please do. :)
>
>> Thanks,
>> Yiwen
>>
>
> Cheers,
>
> --
> Greg
>
> .
>
Thanks Greg,

Ok, we will move forward, I hope we can solve these problems that you
encountered.

Thanks,
Yiwen.