Tests for diod (#5)

* Register scheme "p9" in fsspec.

* Fix test for Python 3.11

String representation of Enum value includes Enum class in 3.11.
Use `value` to get a string value of the Enum.

* Release 0.0.3

* Tests for diod

* Install for root

* Release 0.0.4

Author: pbchekin

.github/workflows/test.yaml

@@ -42,6 +42,11 @@ jobs:
           cache: pip
           cache-dependency-path: pyproject.toml
 
+      - name: Install diod
+        run: |
+          sudo apt update -y
+          sudo apt install -y diod 
+
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
@@ -50,3 +55,8 @@ jobs:
       - name: Run tests
         run: ./scripts/test.sh
 
+      - name: Run tests for diod
+        run: |
+          sudo python -m pip install --upgrade pip
+          sudo python -m pip install -e .[tests]
+          sudo ./scripts/test.sh --diod

README.md

@@ -8,10 +8,11 @@ Supported protocols:
 * [9P2000.u](https://ericvh.github.io/9p-rfc/rfc9p2000.u.html)
 * [9P2000.L](https://github.com/chaos/diod/blob/master/protocol.md)
 
-Supported servers:
+Tested with the following servers:
 
 * [py9p](https://github.com/pbchekin/p9fs-py/blob/main/src/py9p/__main__.py)
 * [unpfs](https://github.com/pfpacket/rust-9p/blob/master/README.md#unpfs)
+* [diod](https://github.com/chaos/diod)
 
 ## Examples
 

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "p9fs"
-version = "0.0.3"
+version = "0.0.4"
 description = "9P implementation of Python fsspec"
 license = {file = "LICENSE"}
 readme = "README.md"

scripts/test.sh

@@ -13,6 +13,9 @@ PY9P_LOG=/tmp/py9p.log
 UNPFS_PID=/tmp/unpfs.pid
 UNPFS_LOG=/tmp/unpfs.log
 
+DIOD_PID=/tmp/diod.pid
+DIOD_LOG=/tmp/diod.log
+
 export PYTHONUNBUFFERED=1
 
 function mymktempdir {
@@ -78,12 +81,29 @@ function stop_unpfs {
   fi
 }
 
+function run_diod {
+  local exported_dir="$1"
+  ${DIOD:-diod} -f -l 0.0.0.0:1234 -e "$exported_dir" -d3 -n &> $DIOD_LOG &
+  echo $! > $DIOD_PID
+  wait_for_server
+}
+
+function stop_diod {
+  if [[ -f $DIOD_PID ]]; then
+    kill "$(<$DIOD_PID)" || true
+    rm -f "$DIOD_PID"
+  fi
+}
+
 function user_tests {
-  cd "$PROJECT_ROOT/src"
-  python -m pytest -v -rA ../tests/user "$@"
+  cd "$PROJECT_ROOT"
+  python -m pytest -v -rA tests/user "$@"
 }
 
 function cleanup {
+  stop_py9p
+  stop_unpfs
+  stop_diod
   if [[ -f $TMP_DIRS_FILE ]]; then
     xargs -n1 -r rm -rf < $TMP_DIRS_FILE
     rm -f $TMP_DIRS_FILE
@@ -96,20 +116,29 @@ rm -f "$TMP_DIRS_FILE"
 
 echo "PROJECT_ROOT: $PROJECT_ROOT"
 
-echo "Testing py9p server with 9P2000"
-EXPORTED_DIR=""$(mymktempdir)""
-run_py9p "$EXPORTED_DIR" 9P2000
-user_tests --exported "$EXPORTED_DIR" --9p 9P2000
-stop_py9p
-
-echo "Testing py9p server with 9P2000.u"
-EXPORTED_DIR=""$(mymktempdir)""
-run_py9p "$EXPORTED_DIR" 9P2000.u
-user_tests --exported "$EXPORTED_DIR" --9p 9P2000.u
-stop_py9p
-
-echo "Testing unpfs server with 9P2000.L"
-EXPORTED_DIR=""$(mymktempdir)""
-run_unpfs "$EXPORTED_DIR"
-user_tests --exported "$EXPORTED_DIR" --9p 9P2000.L
-stop_unpfs
+# Diod requires root, so running it conditionally
+if [[ " $@ " =~ " --diod " ]]; then
+  echo "Testing diod server with 9P2000.L"
+  EXPORTED_DIR=""$(mymktempdir)""
+  run_diod "$EXPORTED_DIR"
+  user_tests --exported "$EXPORTED_DIR" --9p 9P2000.L --aname "$EXPORTED_DIR"
+  stop_diod
+else
+  echo "Testing py9p server with 9P2000"
+  EXPORTED_DIR=""$(mymktempdir)""
+  run_py9p "$EXPORTED_DIR" 9P2000
+  user_tests --exported "$EXPORTED_DIR" --9p 9P2000
+  stop_py9p
+
+  echo "Testing py9p server with 9P2000.u"
+  EXPORTED_DIR=""$(mymktempdir)""
+  run_py9p "$EXPORTED_DIR" 9P2000.u
+  user_tests --exported "$EXPORTED_DIR" --9p 9P2000.u
+  stop_py9p
+
+  echo "Testing unpfs server with 9P2000.L"
+  EXPORTED_DIR=""$(mymktempdir)""
+  run_unpfs "$EXPORTED_DIR"
+  user_tests --exported "$EXPORTED_DIR" --9p 9P2000.L
+  stop_unpfs
+fi

src/p9fs/__init__.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import errno
+import io
 import os
 import pathlib
 import socket
@@ -44,13 +45,6 @@ class P9FileSystem(fsspec.AbstractFileSystem):
 
     protocol = '9p'
 
-    host: str
-    port: int
-    username: str
-    password: Optional[str]
-    version: Version
-    verbose: bool
-
     def __init__(
         self,
         host: str,
@@ -59,6 +53,7 @@ def __init__(
         password: Optional[str] = None,
         version: Union[str, Version] = Version.v9P2000L,
         verbose: bool = False,
+        aname: str = '',
         **kwargs,
     ):
         """9P implementation of fsspec.
@@ -80,6 +75,7 @@ def __init__(
         else:
             self.version = version
         self.verbose = verbose
+        self.aname = aname
         self.fids = fid.FidCache()
         self._rlock = threading.RLock()
         super().__init__(**kwargs)
@@ -107,7 +103,13 @@ def _connect(self):
         s = socket.socket(socket.AF_INET)
         s.connect((self.host, self.port))
         credentials = py9p.Credentials(user=self.username, passwd=self.password)
-        self.client = py9p.Client(s, credentials=credentials, ver=self.version, chatty=self.verbose)
+        self.client = py9p.Client(
+            s,
+            credentials=credentials,
+            ver=self.version,
+            chatty=self.verbose,
+            aname=self.aname,
+        )
 
     def _mkdir(self, path, mode: int = 0o755):
         self._mknod(path, mode | stat.S_IFDIR)
@@ -128,7 +130,12 @@ def _mknod(self, tfid, path, mode):
     @with_fid
     def _info(self, tfid, path):
         parts = pathlib.Path(path).parts
-        self.client._walk(self.client.ROOT, tfid, parts)
+        response = self.client._walk(self.client.ROOT, tfid, parts)
+        # canonical 9p implementation and specifically diod do not return an error if walk is
+        # unsuccessful. Instead, they return all qids up to last existing component in the path.
+        # The only way to check if path exists is compare the number of qids in the response.
+        if len(response.wqid) != len(parts):
+            raise P9FileNotFound(path)
         if self.version == Version.v9P2000L:
             response = self.client._getattr(tfid)
             self.client._clunk(tfid)
@@ -180,7 +187,9 @@ def _info_from_rstat(self, parent: str, response) -> List[Dict]:
 
     @with_fid
     def _unlink(self, tfid, path):
-        if self.version == Version.v9P2000L:
+        # TODO: diod does not support unlinkat, remove should be used instead.
+        # Currently the only way to identify diod is to check if aname is set.
+        if self.version == Version.v9P2000L and not self.aname:
             p = pathlib.Path(path)
             self.client._walk(self.client.ROOT, tfid, p.parent.parts)
             self.client._unlinkat(tfid, p.name)
@@ -227,7 +236,7 @@ def _readdir(self, tfid, path):
             items = []
             offset = 0
             while True:
-                response = self.client._readdir(tfid, offset, self.client.msize)
+                response = self.client._readdir(tfid, offset, self.client.msize - py9p.IOHDRSZ)
                 if response.count == 0:
                     break
                 for entry in response.stat:
@@ -433,7 +442,14 @@ def _upload_chunk(self, final=False):
                 self.closed = True
                 raise
 
-        self.fs._write(self.buffer.getvalue(), self.offset, self._f)
+        data = self.buffer.getvalue()
+        size = len(data)
+        offset = 0
+        msize = self.fs.client.msize - py9p.IOHDRSZ
+        while offset < size - 1:
+            asize = min(msize, size - offset)
+            self.fs._write(data[offset:offset + asize], self.offset + offset, self._f)
+            offset += asize
         return True
 
     def _initiate_upload(self):
@@ -446,7 +462,13 @@ def _fetch_range(self, start, end):
         """Get the specified set of bytes from remote"""
         if self._f is None:
             self._f = self.fs._open_fid(self.path, os.O_RDONLY)
-        return self.fs._read(end - start + 1, start, self._f)
+        data = io.BytesIO()
+        offset = start
+        msize = self.fs.client.msize - py9p.IOHDRSZ
+        while offset < end:
+            asize = min(msize, end - offset + 1)
+            offset += data.write(self.fs._read(asize, offset, self._f))
+        return data.getvalue()
 
     def close(self):
         """Close file"""

src/py9p/py9p.py

@@ -1584,12 +1584,13 @@ def __init__(
             chatty=0,
             ver: Version = Version.v9P2000,
             msize=8192,
+            aname='',
     ):
         self.credentials = credentials
         self.version = ver
         self.msize = msize
         self.fd = Sock(fd, self.dotu, chatty)
-        self.login(authsrv, credentials)
+        self.login(authsrv, credentials, aname)
 
     @property
     def dotu(self):
@@ -1758,7 +1759,7 @@ def _fullclose(self):
         self._clunk(self.CWD)
         self.fd.close()
 
-    def login(self, authsrv, credentials):
+    def login(self, authsrv, credentials, aname=''):
         ver = self.version.to_bytes()
         fcall = self._version(self.msize, ver)
         self.msize = fcall.msize
@@ -1785,7 +1786,9 @@ def login(self, authsrv, credentials):
             else:
                 raise ClientError('unknown authentication method: %s' % credentials.authmode)
 
-        self._attach(self.ROOT, fcall.afid, credentials.user, b'')
+        if isinstance(aname, str):
+            aname = aname.encode('utf-8')
+        self._attach(self.ROOT, fcall.afid, credentials.user, aname)
         if fcall.afid != NOFID:
             self._clunk(fcall.afid)
         self._walk(self.ROOT, self.CWD, [])

tests/user/conftest.py

@@ -43,3 +43,10 @@ def pytest_addoption(parser):
         default=False,
         help="Verbose 9P client",
     )
+    parser.addoption(
+        "--aname",
+        action="store",
+        default="",
+        type=str,
+        help="Name to attach to, required for diod",
+    )

tests/user/test_p9fs.py

@@ -7,6 +7,7 @@
 Examples:
     unpfs 'tcp!0.0.0.0!1234' $TMPDIR
     python -m py9p -p 1234 -w -r $TMPDIR
+    sudo diod -f -l 0.0.0.0:1234 -d3 -n -e $TMPDIR
 
 Pass this TMPDIR to the test suite with `--exported`:
     python -m pytest tests/user --exported $TMPDIR
@@ -29,6 +30,7 @@ def fs(pytestconfig):
         username=pytestconfig.getoption('--user'),
         version=pytestconfig.getoption('--9p'),
         verbose=pytestconfig.getoption('--chatty'),
+        aname=pytestconfig.getoption('--aname')
     )
 
 
@@ -156,7 +158,7 @@ def test_copy(fs, exported_path):
 
 
 def test_registration(fs, exported_path):
-    url = f'p9://{fs.username}@{fs.host}:{fs.port}/xxx?version={fs.version.value}'
+    url = f'p9://{fs.username}@{fs.host}:{fs.port}/xxx?version={fs.version.value}&aname={fs.aname}'
     with fsspec.open(url, 'r') as f:
         data = f.read()
         assert data == 'This is a test content'