Project

General

Profile

Actions

Bug #22162

closed

Support mknod() for regular files

Added by Peter Amstutz 26 days ago. Updated 19 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
FUSE
Story points:
-
Release:
Release relationship:
Auto

Description

mknod() can be used to create regular files without opening them, and appears to be used by Docker in setting up mounts in some circumstances. arv-mount should support this operation.


Subtasks 1 (0 open1 closed)

Task #22170: Review 22162-arv-mount-mknodResolvedPeter Amstutz10/09/2024Actions

Related issues

Related to Arvados - Bug #22163: determine why containerd 1.7.22 fails bind mounts with arv-mount output directoryResolvedLucas Di PentimaActions
Related to Arvados - Bug #22168: gofuse: support mknod() for regular filesResolvedTom CleggActions
Actions #1

Updated by Peter Amstutz 26 days ago

  • Description updated (diff)
Actions #2

Updated by Lucas Di Pentima 26 days ago

  • Related to Bug #22163: determine why containerd 1.7.22 fails bind mounts with arv-mount output directory added
Actions #3

Updated by Tom Clegg 25 days ago

  • Related to Bug #22168: gofuse: support mknod() for regular files added
Actions #4

Updated by Peter Amstutz 24 days ago

  • Target version changed from Development 2024-10-23 sprint to Development 2024-10-09 sprint
Actions #5

Updated by Peter Amstutz 24 days ago

  • Assigned To set to Peter Amstutz
  • Status changed from New to In Progress
Actions #6

Updated by Peter Amstutz 24 days ago

22162-arv-mount-mknod @ aa1c41182a554a0da533cb8e15a78b760534994d

developer-run-tests: #4487

  • All agreed upon points are implemented / addressed. Describe changes from pre-implementation design.
    • Adds support for mknod() of regular files
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • n/a
  • Code is tested and passing, both automated and manual, what manual testing was done is described.
    • Added a test that creates a file in a collection using mknod() and confirms that it can be read back.
  • Documentation has been updated.
    • n/a
  • Behaves appropriately at the intended scale (describe intended scale).
    • No change in scale
  • Considered backwards and forwards compatibility issues between client and server.
    • No change to Arvados API. Improves compatibility of our FUSE mount with programs that are accessing it.
  • Follows our coding standards and GUI style guidelines.
    • yes
Actions #7

Updated by Brett Smith 20 days ago

Peter Amstutz wrote in #note-6:

22162-arv-mount-mknod @ aa1c41182a554a0da533cb8e15a78b760534994d

None of this is a blocker:

  • It would be nice to have tests for the negative cases.
    • Can't mknod the other file types
    • Can't mknod in a read-only mount
    • Can't mknod in the other mount types besides Collection
  • Because fuseMknodTestHelperMknod doesn't do any actual testing, it seems to be needless wrapping. You should be able to delete the whole thing and just say self.pool.apply(os.mknod, (os.path.join(mounttmp, "file1.txt"),))

Thanks.

Actions #8

Updated by Peter Amstutz 20 days ago

  • Release set to 70
Actions #9

Updated by Peter Amstutz 20 days ago

Brett Smith wrote in #note-7:

Peter Amstutz wrote in #note-6:

22162-arv-mount-mknod @ aa1c41182a554a0da533cb8e15a78b760534994d

None of this is a blocker:

  • It would be nice to have tests for the negative cases.
    • Can't mknod the other file types
    • Can't mknod in a read-only mount
    • Can't mknod in the other mount types besides Collection
  • Because fuseMknodTestHelperMknod doesn't do any actual testing, it seems to be needless wrapping. You should be able to delete the whole thing and just say self.pool.apply(os.mknod, (os.path.join(mounttmp, "file1.txt"),))

Thanks.

22162-arv-mount-mknod @ 4150753d569e219c2ae7cc33817de3e5eab4d904

developer-run-tests: #4497

Actions #10

Updated by Brett Smith 19 days ago

Peter Amstutz wrote in #note-9:

22162-arv-mount-mknod @ 4150753d569e219c2ae7cc33817de3e5eab4d904

Thank you, these are great. Just one nit, this line:

self.assertTrue(m.writable() is False)

IMO this line isn't needed for the test at all: there should be other tests of this functionality. But even keeping it, it would be clearer written as self.assertFalse(m.writable()), to avoid the indirect negation and the unusual style of checking foo is Boolean.

Go ahead and merge, thanks.

Actions #11

Updated by Peter Amstutz 19 days ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF