gitlab.com
Open in
urlscan Pro
2606:4700:90:0:f22e:fbec:5bed:a9b9
Public Scan
URL:
https://gitlab.com/qemu-project/qemu/-/issues/103
Submission: On December 02 via api from RU — Scanned from DE
Submission: On December 02 via api from RU — Scanned from DE
Form analysis 2 forms found in the DOM
POST /qemu-project/qemu/-/issues/103.json
<form class="issuable-context-form inline-update js-issuable-update " action="/qemu-project/qemu/-/issues/103.json" accept-charset="UTF-8" data-remote="true" method="post">
<div class="block assignee gl-mt-3" data-testid="assignee-block-container">
<div data-testid="assignees-widget"><!---->
<div>
<div class="gl-flex gl-items-center gl-font-bold gl-leading-20 gl-text-default"><span data-testid="title" class="hide-collapsed"> 0 Beauftragte </span> <!----> <!----> <!----></div>
<div data-testid="collapsed-content">
<div title="Beauftragte" class="sidebar-collapsed-icon sidebar-collapsed-user"><svg data-testid="user-icon" role="img" aria-label="Keine" class="gl-icon s16 gl-fill-current">
<use href="/assets/icons-8791a66659d025e0a4c801978c79a1fbd82db1d27d85f044a35728ea7cf0ae80.svg#user"></use>
</svg> <!----></div>
<div class="issuable-assignees gl-flex gl-flex-col">
<div data-testid="none" class="hide-collapsed gl-flex gl-items-center gl-text-subtle"><span> Keine</span> <!----></div>
</div>
</div>
<div data-testid="expanded-content" class="gl-mt-3" style="display: none;">
<div class="dropdown b-dropdown gl-dropdown dropdown-menu-user -gl-mt-3 gl-w-full btn-group" id="__BVID__888"><!----><button aria-haspopup="menu" aria-expanded="false" type="button"
class="btn dropdown-toggle btn-default btn-md gl-button gl-dropdown-toggle" id="__BVID__888__BV_toggle_"><!----> <!----> <span class="gl-dropdown-button-text">Beauftragte</span> <svg data-testid="chevron-down-icon" role="img"
aria-hidden="true" class="gl-button-icon dropdown-chevron gl-icon s16 gl-fill-current">
<use href="/assets/icons-8791a66659d025e0a4c801978c79a1fbd82db1d27d85f044a35728ea7cf0ae80.svg#chevron-down"></use>
</svg></button>
<ul role="menu" tabindex="-1" class="dropdown-menu" aria-labelledby="__BVID__888__BV_toggle_">
<div class="gl-dropdown-inner">
<div class="gl-dropdown-header !gl-border-b-0"><!---->
<p class="gl-mb-4 gl-mt-2 gl-text-center gl-font-bold">Beauftragte auswählen</p>
<li role="presentation" class="gl-dropdown-divider">
<hr role="separator" aria-orientation="horizontal" class="dropdown-divider">
</li>
<div class="gl-search-box-by-type"><svg data-testid="search-icon" role="img" aria-hidden="true" class="gl-search-box-by-type-search-icon gl-icon s16 gl-fill-icon-subtle">
<use href="/assets/icons-8791a66659d025e0a4c801978c79a1fbd82db1d27d85f044a35728ea7cf0ae80.svg#search"></use>
</svg> <input type="search" placeholder="Suchen" class="gl-form-input form-control gl-search-box-by-type-input" data-testid="user-search-input" aria-label="Suchen" id="__BVID__894"> <!----></div>
</div> <!---->
<div class="gl-dropdown-contents"><!---->
<li role="presentation" class="gl-relative gl-min-h-7"></li>
<li role="presentation" class="gl-dropdown-item"><button data-testid="unassign" role="menuitem" type="button" class="dropdown-item"><svg data-testid="dropdown-item-checkbox" role="img" aria-hidden="true"
class="gl-icon s16 gl-fill-current gl-dropdown-item-check-icon">
<use href="/assets/icons-8791a66659d025e0a4c801978c79a1fbd82db1d27d85f044a35728ea7cf0ae80.svg#mobile-issue-close"></use>
</svg> <!----> <!---->
<div class="gl-dropdown-item-text-wrapper">
<p class="gl-dropdown-item-text-primary"><span class="gl-font-bold gl-pl-0">Nicht zugewiesen</span></p> <!---->
</div> <!---->
</button></li> <!----> <!---->
<li role="presentation" class="gl-dropdown-item"><button data-testid="issuable-author" role="menuitem" type="button" class="dropdown-item"><!----> <!----> <!---->
<div class="gl-dropdown-item-text-wrapper">
<p class="gl-dropdown-item-text-primary"></p>
<div class="gl-avatar-labeled sidebar-participant gl-relative gl-items-center !gl-pl-6" size="32" src="/uploads/-/system/user/avatar/8796052/avatar.png"><img src="/uploads/-/system/user/avatar/8796052/avatar.png" alt=""
class="gl-avatar gl-avatar-circle gl-avatar-s32">
<div class="gl-avatar-labeled-labels !gl-text-left">
<div class="-gl-mx-1 -gl-my-1 gl-flex gl-flex-wrap gl-items-center !gl-text-left"><span class="gl-avatar-labeled-label">Qemu Janitor</span> <!----> <!----></div> <span
class="gl-avatar-labeled-sublabel">@qemu-janitor</span>
</div>
</div>
<p></p> <!---->
</div> <!---->
</button></li> <!---->
</div> <!---->
</div>
</ul>
</div>
</div>
</div>
</div>
</div>
</form><form class="">
<div role="group" class="form-group gl-form-group" id="__BVID__410">
<div><label for="weight-input" class="sr-only" id="__BVID__410__BV_label_"> Gewichtung <!----> <!----></label></div>
<div><input id="weight-input" type="number" placeholder="Gib eine Zahl ein" min="0" class="gl-form-input form-control"><!----><!----><!----></div>
</div>
</form>Text Content
Skip to content
GitLab
* Menü
* Gründe, die für GitLab sprechen
* Preise
* Vertrieb kontaktieren
* Erkunden
* Gründe, die für GitLab sprechen
* Preise
* Vertrieb kontaktieren
* Erkunden
* Anmelden
* Kostenlose Testversion anfordern
PRIMÄRNAVIGATION
Suchen oder aufrufen …
Projekt
* QEMU
* Verwalten
* Aktivität
* Mitglieder
* Labels
* Planen
* Tickets
* Ticketübersichten
* Meilensteine
* Iterationen
* Anforderungen
* Externes Wiki
* Code
* Repository
* Branch
* Commits
* Tags
* Repository-Diagramm
* Revisionen vergleichen
* Gesperrte Dateien
* Build
* Pipelines
* Aufgaben
* Pipeline-Zeitpläne
* Testfälle
* Artefakte
* Bereitstellung
* Releases
* Paket-Registry
* Container-Registry
* Modell-Registry
* Betreiben
* Umgebungen
* Terraform-Module
* Überwachen
* Vorfälle
* Service-Desk
* Analysieren
* Wertschöpfungskettenanalyse
* Mitwirkenden-Analyse
* CI/CD-Analyse
* Repository-Analysen
* Ticketanalysen
* Einblicke
* Modellexperimente
Hilfe
* * Hilfe
* Support
* GitLab-Dokumentation
* GitLab-Pläne vergleichen
* Community-Forum
* Zu GitLab beitragen
* Feedback geben
* Datenschutzerklärung
* * Tastenkürzel ?
* Was ist neu? 3
Code-Schnipsel Gruppen Projekte
1. QEMU
2. QEMU
3. Tickets
4. #103
9PFS DOES NOT HONOR OPEN FILE HANDLES ON UNLINKED FILES
Ticket-Aktionen
* Neues zugehöriges Ticket
* Referenz kopieren
Ticket-Aktionen
Referenz kopieren
--------------------------------------------------------------------------------
Geschlossen Ticket erstellt vor 3 Jahren von Qemu Janitor @qemu-janitor
This bug has been copied automatically from:
https://bugs.launchpad.net/qemu/+bug/1336794
This was originally filed over here:
https://bugzilla.redhat.com/show_bug.cgi?id=1114221
The open-unlink-fstat idiom used in some places to create an anonymous
private temporary file does not work in a QEMU guest over a virtio-9p
filesystem.
Version-Release number of selected component (if applicable):
qemu-kvm-1.6.2-6.fc20.x86_64
qemu-system-x86-1.6.2-6.fc20.x86_64
(those are fedora RPMs)
How reproducible:
Always. See this example C program:
https://bugzilla.redhat.com/attachment.cgi?id=913069
Steps to Reproduce:
1. Export a filesystem with virt-manager for the guest.
(type: mount, driver: default, mode: passthrough)
2. Start guest and mount that filesystem
(mount -t 9p -o trans=virtio,version=9p2000.L ...)
3. Run a program that uses open-unlink-fstat
(in my case it was trying to compile Perl 5.20)
Actual results:
fstat fails:
open("/home/tst/filename", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
unlink("/home/tst/filename") = 0
fstat(3, 0x23aa1a8) = -1 ENOENT (No such file or
directory)
close(3)
Expected results:
open("/home/tst/filename", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
unlink("/home/tst/filename") = 0
fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
fcntl(3, F_SETFD, FD_CLOEXEC) = 0
close(3)
Additional info:
There was a patch put into the kernel back in '07 to handle this very
problem for other filesystems; maybe its helpful:
http://lwn.net/Articles/251228/
There is also a thread on LKML from last December specifically about this
very problem:
https://lkml.org/lkml/2013/12/31/163
There was a discussion on the QEMU list back in '11 that doesn't seem to
have come to a conclusion, but did provide the test program that i've
attached to this report:
http://marc.info/?l=qemu-devel&m=130443605720648&w=2
👍 0 👎 0
Um Designs hochzuladen, musst du LFS aktivieren und ein(e) Administrator(in)
muss den gehashten Speicher aktiviert haben. Weitere Informationen
UNTERGEORDNETE ELEMENTE 0
Weitere Aktionen
* Labels anzeigen
* Show closed items
Derzeit sind keine untergeordneten Elemente zugewiesen. Verwende untergeordnete
Elemente, um dieses Ticket in kleinere Teile zu zerlegen.
VERKNÜPFTE ELEMENTE 0
Verknüpfe issue miteinander, um zu zeigen, dass sie verwandt sind oder andere
blockieren. Mehr erfahren.
AKTIVITÄT
Sortieren oder Filtern
* * Neueste zuerst
* Älteste zuerst
* * Alle Aktivitäten anzeigen
* Nur Kommentare anzeigen
* Nur Verlauf anzeigen
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'infinoid' on Launchpad (2015-04-10):
This bug makes it difficult to run a Debian Jessie guest with a 9pfs root
filesystem, because "apt-get update" uses the open-unlink-fstat idiom.
With this bug present, apt dies with the following error:
E: Unable to determine file size for fd 7 - fstat (2: No such file or
directory)
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'ericvh' on Launchpad (2015-04-12):
I've done some digging from the client side. As is mentioned in Miklos'
original patch (which appears to have been shot down) the higher level
implementation appear to be broken for this. Here's what the code looks
like for fstat in fs/stat.c:
int vfs_fstat(unsigned int fd, struct kstat *stat)
{
struct fd f = fdget_raw(fd);
int error = -EBADF;
if (f.file) {
error = vfs_getattr(&f.file->f_path, stat);
fdput(f);
}
return error;
}
In other words, it only uses the open fd to derrive a path and then
executes the getattr off of that path. If that path no longer exists
(because of unlink or remove) then you are hosed. In my understanding,
the
"work around" I suppose is the so-called 'silly renaming' where
remove/unlink simply does a rename until all open instances are closed.
The frustrating thing is that the 9p protocol is setup to work off of the
fid, which maps to the fd -- so its perfectly capable of the original
semantic but the high level code in fs/stat.c only wants to give a path.
I can do a work around on the client where a getattr "favors" open fids
for
the operation or I can do the sillyrename hack that NFS and CIFS has used
but both of those look god-awful.
-eric
On Fri, Apr 10, 2015 at 7:30 AM, Mark Glines <mark@glines.org> wrote:
> This bug makes it difficult to run a Debian Jessie guest with a 9pfs
> root filesystem, because "apt-get update" uses the open-unlink-fstat
> idiom. With this bug present, apt dies with the following error:
>
> E: Unable to determine file size for fd 7 - fstat (2: No such file or
> directory)
>
> --
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
> https://bugs.launchpad.net/bugs/1336794
>
> Title:
> 9pfs does not honor open file handles on unlinked files
>
> Status in QEMU:
> New
>
> Bug description:
> This was originally filed over here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1114221
>
> The open-unlink-fstat idiom used in some places to create an anonymous
> private temporary file does not work in a QEMU guest over a virtio-9p
> filesystem.
>
> Version-Release number of selected component (if applicable):
>
> qemu-kvm-1.6.2-6.fc20.x86_64
> qemu-system-x86-1.6.2-6.fc20.x86_64
> (those are fedora RPMs)
>
> How reproducible:
>
> Always. See this example C program:
>
> https://bugzilla.redhat.com/attachment.cgi?id=913069
>
> Steps to Reproduce:
> 1. Export a filesystem with virt-manager for the guest.
> (type: mount, driver: default, mode: passthrough)
> 2. Start guest and mount that filesystem
> (mount -t 9p -o trans=virtio,version=9p2000.L ...)
> 3. Run a program that uses open-unlink-fstat
> (in my case it was trying to compile Perl 5.20)
>
> Actual results:
>
> fstat fails:
>
> open("/home/tst/filename", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
> unlink("/home/tst/filename") = 0
> fstat(3, 0x23aa1a8) = -1 ENOENT (No such file or
> directory)
> close(3)
>
> Expected results:
>
> open("/home/tst/filename", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
> unlink("/home/tst/filename") = 0
> fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
> fcntl(3, F_SETFD, FD_CLOEXEC) = 0
> close(3)
>
> Additional info:
>
> There was a patch put into the kernel back in '07 to handle this very
> problem for other filesystems; maybe its helpful:
>
> http://lwn.net/Articles/251228/
>
> There is also a thread on LKML from last December specifically about
> this very problem:
>
> https://lkml.org/lkml/2013/12/31/163
>
> There was a discussion on the QEMU list back in '11 that doesn't seem
> to have come to a conclusion, but did provide the test program that
> i've attached to this report:
>
> http://marc.info/?l=qemu-devel&m=130443605720648&w=2
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1336794/+subscriptions
>
>
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'ericvh' on Launchpad (2015-04-12):
On Sun, Apr 12, 2015 at 9:09 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Apr 12, 2015 at 12:42:35PM -0000, Eric Van Hensbergen wrote:
>
> > In other words, it only uses the open fd to derrive a path and then
> > executes the getattr off of that path. If that path no longer exists
> > (because of unlink or remove) then you are hosed. In my
understanding,
> the
> > "work around" I suppose is the so-called 'silly renaming' where
> > remove/unlink simply does a rename until all open instances are
closed.
>
> What do you mean, "no longer exists"? Don't confuse path with pathname
-
> it's a mount,dentry pair. And dentry in question bloody well ought to
> still
> have the FID associated with it - you shouldn't use the same FID for
> TREMOVE and for TREAD/TWRITE.
Quite right, the fid is still there, I just don't have an easy way to get
at it. vfs_file operations have a direct notion of the fid because they
cache it in the struct file private data. dentries have several fids
associated with them and stored in thier private data, so I've got to
"guess" the right one. In most cases this probably won't cause a problem,
but it just feels a bit off.
There was a thread a few years back where Miklos was arguing for fstat to
pass through struct file information since the the fd given the fstat had
a
file structure associated with it (which in 9p's case has a direct pointer
to the "right" fid):
http://lwn.net/Articles/251228/
In any case, I've drafted a quick patch which takes the approach of
searching the dentry fid list for the fid, but it doesn't feel like the
right answer and I'm fairly certain I need to iterate on it a few times to
make sure I haven't hosed something up.
> TREMOVE clunks the FID passed to it; on
> some servers you really have no choice - server discards the file
> completely
> and on any FID that used to refer to it you get an error from that point
> on.
> On those you'd really have to do something like sillyrename - the only
> way to keep IO going for a file sitting on such server is to have it
> visible somewhere. Normal fs(4) is that way; e.g. u9fs(4) isn't - there
> FID maps to opened file descriptor on host and TREMOVE on another FID
> doesn't break it, as long as host supports IO on opened-but-unlinked
files.
> I don't remember where qemu 9pfs falls in that respect, but I'd expect
it
> to be more like u9fs...
>
>
Sort of, the servers in kvmtool and qemu (and diod) have a fid with the
open handle. However, all of them seem to implement getattr assuming they
have to re-walk to the file. In order to test my aforementioned changes
to
the client, I also did a quick patch to kvmtool which checks and sees if
the fid it receives has an fd and just uses fstat instead of lstat. Patch
is here at the moment, I'll send upstream once I'm happy with the client
side changes and look into creating a patch for qemu/diod:
https://github.com/ericvh/linux-
kvm/commit/2fa2f7e728ac08a7d9006516870db1a986aa6acc
> Which FID are you passing to server on unlink()?
>
>
Unlink/remove look to be getting a proper fid (in other words, not using
the one that is open). The problem is that getattr is using a reference
fid (an open fid that's already walked to the name). From a protocol
semantics perspective the fixes mentioned above probably don't help that
we
may have some dangling unopen fids pointing at a name that is no longer
valid, but that is just a consequence of the imperfect nature of the
mapping of dentries to fids and I'm not sure it does any harm. A trace
from the original problem on diod (which appears to not implement unlink
and is falling back to remove).
diod: P9_TWALK tag 1 fid 1 newfid 2 nwname 1 'foobar'
diod: P9_RLERROR tag 1 ecode 2
diod: P9_TWALK tag 1 fid 1 newfid 2 nwname 0
diod: P9_RWALK tag 1 nwqid 0e
diod: P9_TLCREATE tag 1 fid 2 name 'foobar' flags 0x8042 mode 0100644 gid
0
diod: P9_RLCREATE tag 1 qid (000000000012524b 0 '') iounit 0
diod: P9_TWALK tag 1 fid 1 newfid 3 nwname 1 'foobar'
diod: P9_RWALK tag 1 nwqid 1 (000000000012524b 0 '')
diod: P9_TGETATTR tag 1 fid 3 request_mask 0x17ff
diod: P9_RGETATTR tag 1 valid 0x17ff qid (000000000012524b 0 '') mode
0100644 uid 0 gid 0 nlink 1 rdev 0 size 0 blksize 4096 blocks 0 atime Mon
Apr 6 11:11:08 2015 mtime Mon Apr 6 11:11:08 2015 ctime Mon Apr 6
11:11:08 2015 btime X gen 0 data_version X
diod: P9_TUNLINKAT tag 1 dirfid 1 name 'foobar' flags 0
diod: P9_RLERROR tag 1 ecode 95
diod: P9_TWALK tag 1 fid 3 newfid 4 nwname 0
diod: P9_RWALK tag 1 nwqid 0
diod: P9_TREMOVE tag 1 fid 4
diod: P9_RREMOVE tag 1
diod: P9_TGETATTR tag 1 fid 3 request_mask 0x3fff
diod: P9_RLERROR tag 1 ecode 2
diod: P9_TCLUNK tag 1 fid 2
diod: P9_RCLUNK tag 1
diod: P9_TCLUNK tag 1 fid 3
diod: P9_RCLUNK tag 1
The client cloning 3 to 4 before the remove seems a bit unnecessary, but
is
probably done in the case that the remove fails on the server so that we
still have a dentry reference.
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'ericvh' on Launchpad (2015-04-13):
Well, technically fid 3 isn't 'open', only fid 2 is open - at least
according to the protocol. fid 3 and fid 2 are both clones of fid 1.
However, thanks for the alternative workaround. If you get a chance, can
you check that my change to the client to partially fix this for the other
servers doesn't break nfs-ganesha:
https://github.com/ericvh/linux/commit/fddc7721d6d19e4e6be4905f37ade5b0521f4ed5
On Mon, Apr 13, 2015 at 3:27 AM, Dominique Martinet <
dominique.martinet@cea.fr> wrote:
> Hi,
>
> for what it's worth, the sample code given works with nfs-ganesha
> server, so I'm not sure what's not working here.
>
> Here's the server traces:
> TATTACH: tag=1 fid=0 afid=-1 uname='nobody' aname='/export' n_uname=-1
> RATTACH: tag=1 fid=0 qid=(type=128,version=0,path=1)
> TGETATTR: tag=1 fid=0 request_mask=0x7ff
> RGETATTR: tag=1 valid=0x7ff qid=(type=128,version=0,path=1) mode=040755
> uid=0 gid=0 nlink=3 rdev=192 size=4096 blksize=4096 blocks=8
> atime=(1428909387,0) mtime=(1428909389,0) ctime=(1428909389,0)
btime=(0,0)
> gen=0, data_version=0
> TATTACH: tag=1 fid=1 afid=-1 uname='' aname='/export' n_uname=0
> RATTACH: tag=1 fid=1 qid=(type=128,version=0,path=1)
> TGETATTR: tag=1 fid=1 request_mask=0x3fff
> RGETATTR: tag=1 valid=0x7ff qid=(type=128,version=0,path=1) mode=040755
> uid=0 gid=0 nlink=3 rdev=192 size=4096 blksize=4096 blocks=8
> atime=(1428909387,0) mtime=(1428909389,0) ctime=(1428909389,0)
btime=(0,0)
> gen=0, data_version=0
> TWALK: tag=1 fid=1 newfid=2 nwname=1 (component 1/1 :test.txt)
> RERROR(_9P_TWALK) tag=1 err=(2|No such file or directory)
> TWALK: tag=1 fid=1 newfid=2 nwname=0
> RWALK: tag=1 fid=1 newfid=2 nwqid=0 fileid=1 pentry=0x8278c0 refcount=4
> TLCREATE: tag=1 fid=2 name=test.txt flags=0100102 mode=0100644 gid=0
> RLCREATE: tag=1 fid=2 name=test.txt
> qid=(type=0,version=0,path=144115205255725065) iounit=0
> pentry=0x7fffc0000d00
> TWALK: tag=1 fid=1 newfid=3 nwname=1 (component 1/1 :test.txt)
> RWALK: tag=1 fid=1 newfid=3 nwqid=1 fileid=144115205255725065
> pentry=0x7fffc0000d00 refcount=3
> TGETATTR: tag=1 fid=3 request_mask=0x17ff
> RGETATTR: tag=1 valid=0x7ff
qid=(type=0,version=0,path=144115205255725065)
> mode=0100644 uid=0 gid=0 nlink=1 rdev=192 size=0 blksize=4096 blocks=0
> atime=(1428909674,0) mtime=(1428909674,0) ctime=(1428909674,0)
btime=(0,0)
> gen=0, data_version=0
> TWRITE: tag=1 fid=2 offset=0 count=6
> RWRITE: tag=1 fid=2 offset=0 input count=6 output count=6
> TUNLINKAT: tag=1 dfid=1 name=test.txt
> TUNLINKAT: tag=1 dfid=1 name=test.txt
> TGETATTR: tag=1 fid=3 request_mask=0x3fff
> RGETATTR: tag=1 valid=0x7ff
qid=(type=0,version=0,path=144115205255725065)
> mode=0100644 uid=0 gid=0 nlink=0 rdev=192 size=6 blksize=4096 blocks=0
> atime=(1428909674,0) mtime=(1428909674,0) ctime=(1428909674,0)
btime=(0,0)
> gen=0, data_version=0
> TCLUNK: tag=1 fid=2
> RCLUNK: tag=1 fid=2
> TCLUNK: tag=1 fid=3
> RCLUNK: tag=1 fid=3
>
> (if you're not familiar with 9P, ATTACH = mount, WALK = create a new
> 'fid' either clone of current file (nwname = 0) or lookup, CLUNK ~=
> close. Rest is obvious enough)
>
>
> There's no lookup between the unlink and the getattr, so what I think is
> missing is that both qemu and diod do not understand that fids 2 and 3
> refer to the same open file ?
> It's a bit of a weird behavior that the client will open a new fid
> through lookup walk for a first stat, but I'm mounting with cache=none
> here so you should be having the same.
>
> --
> Dominique
>
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'ericvh' on Launchpad (2015-04-14):
That patch looks fine by me. Happy to put it in the queue. Thanks Al.
On Tue, Apr 14, 2015 at 11:07 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Apr 13, 2015 at 04:05:28PM +0000, Eric Van Hensbergen wrote:
> > Well, technically fid 3 isn't 'open', only fid 2 is open - at least
> > according to the protocol. fid 3 and fid 2 are both clones of fid 1.
> > However, thanks for the alternative workaround. If you get a chance,
can
> > you check that my change to the client to partially fix this for the
> other
> > servers doesn't break nfs-ganesha:
> >
> >
>
https://github.com/ericvh/linux/commit/fddc7721d6d19e4e6be4905f37ade5b0521f4ed5
>
> BTW, what the hell is going on in v9fs_vfs_mknod() and v9fs_vfs_link()?
> You allocate 4Kb buffer, sprintf into it ("b %u %u", "c %u %u", or
"%d\n")
> feed it to v9fs_vfs_mkspecial() and immediately free it. What's wrong
with
> a local array of char? In the worst case it needs to be char name[24] -
> surely, we are not so tight on stack that extra 16 bytes (that array
> instead
> of a pointer) would drive us over the cliff?
>
> IOW, do you have any problem with this:
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 703342e..cda68f7 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -1370,6 +1370,8 @@ v9fs_vfs_symlink(struct inode *dir, struct dentry
> *dentry, const char *symname)
> return v9fs_vfs_mkspecial(dir, dentry, P9_DMSYMLINK, symname);
> }
>
> +#define U32_MAX_DIGITS 10
> +
> /**
> * v9fs_vfs_link - create a hardlink
> * @old_dentry: dentry for file to link to
> @@ -1383,7 +1385,7 @@ v9fs_vfs_link(struct dentry *old_dentry, struct
> inode *dir,
> struct dentry *dentry)
> {
> int retval;
> - char *name;
> + char name[1 + U32_MAX_DIGITS + 2]; /* sign + number + \n + \0 */
> struct p9_fid *oldfid;
>
> p9_debug(P9_DEBUG_VFS, " %lu,%pd,%pd\n",
> @@ -1393,20 +1395,12 @@ v9fs_vfs_link(struct dentry *old_dentry, struct
> inode *dir,
> if (IS_ERR(oldfid))
> return PTR_ERR(oldfid);
>
> - name = __getname();
> - if (unlikely(!name)) {
> - retval = -ENOMEM;
> - goto clunk_fid;
> - }
> -
> sprintf(name, "%d\n", oldfid->fid);
> retval = v9fs_vfs_mkspecial(dir, dentry, P9_DMLINK, name);
> - __putname(name);
> if (!retval) {
> v9fs_refresh_inode(oldfid, d_inode(old_dentry));
> v9fs_invalidate_inode_attr(dir);
> }
> -clunk_fid:
> p9_client_clunk(oldfid);
> return retval;
> }
> @@ -1425,7 +1419,7 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry
> *dentry, umode_t mode, dev_t rde
> {
> struct v9fs_session_info *v9ses = v9fs_inode2v9ses(dir);
> int retval;
> - char *name;
> + char name[2 + U32_MAX_DIGITS + 1 + U32_MAX_DIGITS + 1];
> u32 perm;
>
> p9_debug(P9_DEBUG_VFS, " %lu,%pd mode: %hx MAJOR: %u MINOR:
%u\n",
> @@ -1435,26 +1429,16 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry
> *dentry, umode_t mode, dev_t rde
> if (!new_valid_dev(rdev))
> return -EINVAL;
>
> - name = __getname();
> - if (!name)
> - return -ENOMEM;
> /* build extension */
> if (S_ISBLK(mode))
> sprintf(name, "b %u %u", MAJOR(rdev), MINOR(rdev));
> else if (S_ISCHR(mode))
> sprintf(name, "c %u %u", MAJOR(rdev), MINOR(rdev));
> - else if (S_ISFIFO(mode))
> - *name = 0;
> - else if (S_ISSOCK(mode))
> + else
> *name = 0;
> - else {
> - __putname(name);
> - return -EINVAL;
> - }
>
> perm = unixmode2p9mode(v9ses, mode);
> retval = v9fs_vfs_mkspecial(dir, dentry, perm, name);
> - __putname(name);
>
> return retval;
> }
>
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'ericvh' on Launchpad (2015-04-15):
good to know, thanks dominique. I gave it a sniff test with FSX and a few
other benchmarks, but I need to hit it with some multithreaded
regressions. Any pointers to reproducible failure cases would be
beneficial.
On Wed, Apr 15, 2015 at 6:28 AM Dominique Martinet <
dominique.martinet@cea.fr> wrote:
> Eric Van Hensbergen wrote on Mon, Apr 13, 2015 at 04:05:28PM +0000:
> > Well, technically fid 3 isn't 'open', only fid 2 is open - at least
> > according to the protocol. fid 3 and fid 2 are both clones of fid 1.
>
> Right, they're clone fids, but nothing in the protocol says what should
> happen to non-open fids when you unlink the file either - I guess both
> behaviours are OK as long as the client can handle it, so it would make
> sense to at least call fstat() on the fid matching the fd, but while
> I think this is how the kernel currently behaves the kernel doesn't HAVE
> to make one open, separate fid per open file descriptor either.
>
> > However, thanks for the alternative workaround. If you get a chance,
can
> > you check that my change to the client to partially fix this for the
> other
> > servers doesn't break nfs-ganesha:
> >
> >
>
https://github.com/ericvh/linux/commit/fddc7721d6d19e4e6be4905f37ade5b0521f4ed5
>
> I'm afraid I haven't had much time lately, but while fstat-after-unlink
> still works I'm getting some Oops with my basic test suite (create empty
> files and rm -rf, kernel compilation, etc - nothing fancy) to the point
> of locking myself out of my test box pretty quickly.
>
> I'll try to debug patches a bit more individually (trying everything at
> once isn't helping), but at the very least something is fishy
>
> --
> Dominique Martinet
>
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'l-admin-o' on Launchpad (2016-05-05):
I would appreciate this patch being committed as I *think* it's affecting
a system i'm building now.
I have a backup host with 2 VMs. For business reasons they need to be
network isolated from each other and the host, so each is passed through a
physical NIC. Each VM does need access to a variable size datastore on the
host so I am using virtfs /9p to expose a mountpoint to each VM.
The VMs each backup servers to their respective mountpoint to this virtfs
mount using rsync. Just backing up one server with ~4000 files and 3 large
sparse VM images saw the open files on the backup host increase to over
*800000* and the rsync progressively get slower. Shutting down these VMs
then takes hours as it can't unlock the files it has open on the backup
host.
I understand rsync does use open-unlink-fstat extensively, hence why I
think this is the issue.
This is a deal breaker for any production use of virtfs. Does anybody know
if this is fixed in other builds of qemu?
tl;dr - to recreate this on 16.04 - create a VM with a virtfs/9p mount to
the host. Do lots of rsyncs to this mount within the VM, watch 'lsof | wc
-l' go higher and higher on the host.
Thanks,
/Sean
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'janitor' on Launchpad (2016-05-24):
Status changed to 'Confirmed' because the bug affects multiple users.
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'gkurz' on Launchpad (2016-06-02):
Latest work on the subject seems to be:
https://github.com/ericvh/linux/commit/eaf70223eac094291169f5a6de580351890162a2
I could verify that this patch still applies to the upstream kernel tree
and fixes the issue.
The fix was verified with upstream QEMU + the following patch:
http://patchwork.ozlabs.org/patch/626194/
I have pinged kernel v9fs maintainers but I have not received any answer
yet.
I intend to push the QEMU patch to upstream soon.
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'maxim-kuvyrkov' on Launchpad (2017-08-04):
Hi Greg,
Did you push the qemu patch upstream, and now it is a matter of fixing the
kernel?
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'gkurz' on Launchpad (2017-08-04):
Hi Maxim,
No I didn't...
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'agretha' on Launchpad (2018-11-26):
hi,
i am probably trying to ride a dead horse here, but is there any chance
this patch will make its way into master?
thanks,
alex
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'gkurz' on Launchpad (2018-11-27):
Hi Alex,
Well... it's slightly more than one patch in QEMU, and this also requires
some linux kernel side changes. And I really can't^W^Wdon't want to invest
more time there if no one helps. This being said, I see more and more
activity on 9p since Dominique Martinet has taken maintainership. Maybe
worth to resurrect the discussion on v9fs-developer@lists.sourceforge.net
? If it gets enough momentum there, I'll be happy to go forward with the
QEMU changes.
Cheers,
--
Greg
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'agretha' on Launchpad (2018-11-28):
hi greg,
thanks very much for you answer. i saw the proposed kernel patch from eric
van hensbergen - even tried to build my own kernel with the patch applied,
i was ready to run this on a custom kernel with a custom built qemu, but
although the patch can be applied, there have been too many changes in the
surrounding code for it to be able to work.
the idea of the 9p file sharing in qemu is really nice (and fast). i am
(was) trying to use it as a persistent storage on a kubernetes cluster and
it is much better than nfs (performance wise) locking works just dandy.
with 9p i thought i was golden, unfortunately no cigar.
since there are different parties involved (and to get something into the
linux kernel requires - from what i have read - the patience of a buddhist
monk) i think it will be very hard to get this picked up.
because of the time frame this will probably not be a solution for me, but
i am nonetheless willing to invest some time to bringing this forward.
how is a good way to proceed? (sorry, this question might sound dumb, but
despite being a software developer for most of my working life the ways of
the open source community have never revealed themselves to me).
-alex
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'gkurz' on Launchpad (2020-05-26):
I haven't worked on this topic in years.
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'gkurz' on Launchpad (2020-05-27):
For the records.
QEMU patches:
https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg07586.html
Linux patches:
https://sourceforge.net/p/v9fs/mailman/message/35175775/
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'th-huth' on Launchpad (2020-12-10):
Released with QEMU v5.2.0.
* Qemu Janitor @qemu-janitor · vor 3 Jahren
Autor(in) Reporter
Comment from 'th-huth' on Launchpad (2020-12-10):
Closed by accident, Christian just told me that this is not fixed yet.
Sorry for the inconvenience.
* Qemu Janitor added Launchpad label vor 3 Jahren
added Launchpad label
* John Snow added Storage kindBug labels vor 3 Jahren
added Storage kindBug labels
* John Snow added workflowTriaged label vor 3 Jahren
added workflowTriaged label
* Christian Schoenebeck @schoenebeck · vor 3 Jahren
Reporter
As I was not involved in 9p yet when this issue was created and discussed, I
just gave this issue a spin now, first tested whether this is actually still
an issue - yes it is unfortunately - and reviewed the discussions so far as
far as I could. However as discussions on this issue, along with various
different patch set versions, are so sparsely spread over multiple places and
time, it is quite tedious for me to acquire all the info on this adequately.
If somebody is still interested in fixing this overall issue, then I would
suggest splitting this issue up into smaller problems and patch sets
accordingly to be able to bring this forward at all.
As the current version of the 9p protocol is apparently not a show stopper
for this issue, I would start by fixing the individual requests types to
behave correctly after unlink() on QEMU side, one by one, not all together.
We already do have a decent test case framework for 9p on QEMU side which can
be used to tackle these issues down, without need of any 9p client or any
guest OS installation, which is very convenient and efficient to use:
https://wiki.qemu.org/Documentation/9p#Test_Cases
* Christian Schoenebeck @schoenebeck · vor 2 Jahren
Reporter
Workaround: as I pointed out in this 9p rootfs HOWTO, enabling tmpfs on
guest's /tmp directory seems to be sufficient to prevent this issue from
happening in practice, either by doing that inside guest or on host level, it
does not matter. Because so far I have not seen any software using an
use-after-unlink pattern outside of /tmp.
I tested this workaround for various distros and versions, and it worked
reliably as described in the linked HOWTO no matter which software I
additionally installed.
* Christian Schoenebeck mentioned in issue #1012 (closed) vor 2 Jahren
mentioned in issue #1012 (closed)
* Vsevolod Kozlov @vsevolodkozlovcloudronics · vor 2 Jahren
It looks like localedef under Debian (bullseye, glibc 2.31-13+deb11u3) is
using a similarly problematic file system access pattern when generating the
locale archive, as seen in the trace below:
openat(AT_FDCWD, "/usr/lib/locale/locale-archive.oCbmyz", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
write(3, "\t\1\2\336\0\0\0\08\0\0\0\0\0\0\0\213\3\0\0\274*\0\0\0\0\0\0L\35\0\0"..., 56) = 56
ftruncate(3, 103860) = 0
mmap(NULL, 536870912, PROT_NONE, MAP_SHARED, 3, 0) = 0xffff7c945000
mmap(0xffff7c945000, 103860, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 3, 0) = 0xffff7c945000
linkat(AT_FDCWD, "/usr/lib/locale/locale-archive.oCbmyz", AT_FDCWD, "/usr/lib/locale/locale-archive", 0) = 0
unlinkat(AT_FDCWD, "/usr/lib/locale/locale-archive.oCbmyz", 0) = 0
fchmod(3, 0644) = -1 ENOENT (No such file or directory)
unlinkat(AT_FDCWD, "/usr/lib/locale/locale-archive", 0) = 0
The fchmod call fails after unlinkat in a way that looks like this issue. And
since this is happening under /usr, unfortunately the tmpfs workaround is not
applicable.
(This is a version of QEMU from git commit 1fba9dc7 running on a macOS host,
where 9P has only very recently become sufficiently functional to even try
something like this at all, though, so it may as well be an issue related to
that, but the symptoms look very much like this issue…)
Bearbeitet vor 2 Jahren bei Vsevolod Kozlov
* * Christian Schoenebeck @schoenebeck · vor 2 Jahren
Reporter
@vsevolodkozlovcloudronics confirmed, when I run "dpkg-reconfigure
locales" on latest Debian Bullseye (on a Linux host BTW), select a new
locale to be generated, then I get the same error now. Maybe there was
some change on Bullseye, not sure.
Anyway, like I said before, I can help with reviewing patches to bring
this issue forward, both on QEMU and on Linux kernel side, but it still
needs somebody willing to send patches. I will definitely not make this a
one-man-show.
I'm actually surprised that you got that far on a macOS host. Because
there is still the unresolved issue with case-insensitive filesystems with
macOS hosts: https://lore.kernel.org/qemu-devel/1757498.AyhHxzoH2B@silver/
* Antworten reduzieren
* Vsevolod Kozlov @vsevolodkozlovcloudronics · vor 2 Jahren
I have used a separate case-sensitive APFS volume on the host side. It
seemed pretty hopeless (and pointless) to try to get a normal Linux system
running on a case-insensitive file system.
* Bitte registriere oder melde dich an um zu antworten
* Christian Schoenebeck @schoenebeck · vor 2 Jahren
Reporter
@gkurz : JFYI
* Christian Schoenebeck @schoenebeck · vor 2 Jahren
Reporter
I just realized that the Linux kernel patches regarding this use-after-unlink
issue were already merged:
https://github.com/torvalds/linux/commits/478ba09edc1f2f2ee27180a06150cb2d1a686f9c
It would make sense to reassess what's still to be done to resolve this
issue.
* Christian Schoenebeck @schoenebeck · vor 5 Monaten
Reporter
So some scenarios seem to work already, e.g. doing I/O on unlinked files:
https://lore.kernel.org/all/E1rcnYJ-0004KK-LV@lizzy.crudebyte.com/
So current task would be finding the scenarios that do not work yet. Feedback
appreciated.
* Christian Schoenebeck mentioned in commit c81e7219 vor 3 Tagen
mentioned in commit c81e7219
* Christian Schoenebeck closed with commit c81e7219 vor 3 Tagen
closed with commit c81e7219
* Christian Schoenebeck @schoenebeck · vor 3 Tagen
Reporter
Commit c81e7219 fixed this open-unlink-fstat scenario, hence considering this
issue as resolved.
Note however that other use-after-unlink scenarios might still exist. Please
open a separate bug report for those.
* Christian Schoenebeck mentioned in commit mjt0k/qemu@1e417913 vor 2 Tagen
mentioned in commit mjt0k/qemu@1e417913
* Christian Schoenebeck mentioned in commit mjt0k/qemu@37adcdb9 vor 2 Tagen
mentioned in commit mjt0k/qemu@37adcdb9
* Christian Schoenebeck mentioned in commit mjt0k/qemu@313f59d6 vor 2 Tagen
mentioned in commit mjt0k/qemu@313f59d6
* Michael Tokarev added Stableto backport label vor 2 Tagen
added Stableto backport label
Bitte registriere oder melde dich an um zu antworten
0 Beauftragte
Keine
Beauftragte
Beauftragte auswählen
--------------------------------------------------------------------------------
Nicht zugewiesen
Qemu Janitor
@qemu-janitor
Epic
Keine
Keine
Epic
Labels
5
Stable to backport kind Bug workflow Triaged Launchpad Storage
5
Stable to backport kind Bug workflow Triaged Launchpad Storage
Launchpad +4 weitere
Etiketts auswählen
Keine passenden Ergebnisse
Projektlabel verwalten
Meilenstein
Keine
Keine
Meilenstein
Iteration
Keine
Keine
Iteration
Gewichtung
Keine
Keine
Gewichtung
Fälligkeitsdatum
Keine
Keine
Keine
Zeiterfassung
Weder Schätzung noch Zeitaufwand eingetragen
Gesundheitsstatus
Keine
Keine
Kein Status
Integritätsprobleme auswählen
*
* Kein Status
* Gesundheit Zustand
* Planmäßig
* Erfordert deine Aufmerksamkeit
* In Gefahr
Vertraulichkeit
Nicht vertraulich
Nicht vertraulich
Hiermit wird die Geheimhaltung eingeschaltet. Nur Projekt Mitglieder mit at
least the Planner role, the author, and assignees könnten diesen Ticket sehen
oder darüber benachrichtigt werden.
Abbrechen Einschalten
4
4 Teilnehmer(innen)