Added umount command and unmount() API

fs/build-tests.xml

@@ -41,6 +41,7 @@ help         Output these messages
             <test name="org.jnode.test.fs.filesystem.FSTestSuite" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.driver.tests.BlockDeviceAPITest" todir="${basedir}/build/reports/junit"/>
             <test name="org.jnode.test.fs.command.SyncCommandTest" todir="${basedir}/build/reports/junit"/>
+            <test name="org.jnode.fs.service.def.FileSystemAPIImplTest" todir="${basedir}/build/reports/junit"/>
         </junit>
 	</target>
 

fs/descriptors/org.jnode.fs.command.xml

@@ -43,6 +43,7 @@
     <alias name="eject" class="org.jnode.fs.command.EjectCommand"/>
     <alias name="mount" class="org.jnode.fs.command.MountCommand"/>
     <alias name="sync" class="org.jnode.fs.command.SyncCommand"/>
+    <alias name="umount" class="org.jnode.fs.command.UnmountCommand"/>
   </extension>
 
   <extension point="org.jnode.shell.syntaxes">
@@ -66,6 +67,11 @@
         <argument argLabel="path" description="the path of the filesystem to flush"/>
       </sequence>
     </syntax>
+    <syntax alias="umount">
+      <sequence description="Unmount the filesystem at the given path">
+        <argument argLabel="directory" description="the mount point to unmount"/>
+      </sequence>
+    </syntax>
   </extension>
 
   <extension point="org.jnode.security.permissions">

fs/src/commands/org/jnode/fs/command/UnmountCommand.java

@@ -0,0 +1,69 @@
+/*
+ * $Id$
+ *
+ * Copyright (C) 2003-2015 JNode.org
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but 
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public 
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library; If not, write to the Free Software Foundation, Inc., 
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+package org.jnode.fs.command;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import org.jnode.fs.service.FileSystemService;
+import org.jnode.naming.InitialNaming;
+import org.jnode.shell.AbstractCommand;
+import org.jnode.shell.syntax.Argument;
+import org.jnode.shell.syntax.FileArgument;
+
+/**
+ * Unmount a filesystem.
+ */
+public class UnmountCommand extends AbstractCommand {
+
+    private final FileArgument argDir =
+        new FileArgument("directory", Argument.MANDATORY,
+            "the mount point to unmount");
+
+    public UnmountCommand() {
+        super("Unmount a filesystem");
+        registerArguments(argDir);
+    }
+
+    public static void main(String[] args) throws Exception {
+        new UnmountCommand().execute(args);
+    }
+
+    public void execute() throws Exception {
+        FileSystemService fss = InitialNaming.lookup(FileSystemService.NAME);
+        PrintWriter out = getOutput().getPrintWriter();
+        PrintWriter err = getError().getPrintWriter();
+        String path = argDir.getValue().getCanonicalPath();
+
+        if (!fss.isMount(path)) {
+            err.println("Not a mount point: " + path);
+            err.println("Use 'mount' to list mounted filesystems.");
+            exit(1);
+        }
+
+        try {
+            fss.unmount(path);
+            out.println("Unmounted " + path);
+        } catch (IOException ex) {
+            err.println("Failed to unmount " + path + ": " + ex.getMessage());
+            exit(1);
+        }
+    }
+}

fs/src/fs/org/jnode/fs/service/FileSystemService.java

@@ -115,6 +115,16 @@ public <T extends FileSystemType<?>> T getFileSystemType(Class<T> name)
      */
     public boolean isMount(String fullPath);
 
+    /**
+     * Unmount the filesystem at the given path.
+     * Flushes the filesystem, closes it, and removes the mount point.
+     *
+     * @param fullPath the mount point path
+     * @throws IOException if flush or close fails
+     * @throws IllegalArgumentException if path is not a mount point
+     */
+    public void unmount(String fullPath) throws IOException;
+
     /**
      * Gets the filesystem API.
      */

fs/src/fs/org/jnode/fs/service/def/FileSystemAPIImpl.java

@@ -37,6 +37,7 @@
 import org.jnode.fs.FSDirectory;
 import org.jnode.fs.FSEntry;
 import org.jnode.fs.FileSystem;
+import org.jnode.fs.spi.AbstractFileSystem;
 import org.jnode.java.io.VMFileHandle;
 
 /**
@@ -460,8 +461,35 @@ void mount(String fullPath, FileSystem<?> fs, String fsPath) throws IOException
             fullPath = File.separatorChar + fullPath;
         }
 
-        mountPoints.put(fullPath, fs); // TODO handle removal (+ add unmount
-                                        // method) of filesystems
+        mountPoints.put(fullPath, fs);
+    }
+
+    /**
+     * Unmount the filesystem at the given path.
+     * Flushes the filesystem, closes it, and removes the mount point.
+     * If close fails, the mount point is restored for rollback.
+     *
+     * @param fullPath the mount point path
+     * @throws IOException if close fails
+     * @throws IllegalArgumentException if path is not a mount point
+     */
+    void unmount(String fullPath) throws IOException {
+        if (fullPath.charAt(0) != File.separatorChar) {
+            fullPath = File.separatorChar + fullPath;
+        }
+
+        FileSystem<?> fs = mountPoints.remove(fullPath);
+        if (fs == null) {
+            throw new IllegalArgumentException("Not a mount point: " + fullPath);
+        }
+
+        try {
+            fs.close();
+            vfs.unregisterFileSystem(fs.getDevice());
+        } catch (IOException ex) {
+            mountPoints.put(fullPath, fs);
+            throw ex;
+        }
     }
 
     /**

fs/src/fs/org/jnode/fs/service/def/FileSystemPlugin.java

@@ -147,6 +147,18 @@ public void mount(String fullPath, FileSystem<?> fs, String fsPath) throws IOExc
         api.mount(VMFile.getNormalizedPath(fullPath), fs, fsPath);
     }
 
+    /**
+     * Unmount the filesystem at the given path.
+     * Flushes the filesystem, closes it, and removes the mount point.
+     *
+     * @param fullPath the mount point path
+     * @throws IOException if flush or close fails
+     * @throws IllegalArgumentException if path is not a mount point
+     */
+    public void unmount(String fullPath) throws IOException {
+        api.unmount(VMFile.getNormalizedPath(fullPath));
+    }
+
     /**
      * Return a map (fullPath -> FileSystem) of mount points
      * @return a copy of the internal map, sorted by fullPath

fs/src/test/org/jnode/fs/service/def/FileSystemAPIImplTest.java

@@ -0,0 +1,167 @@
+/*
+ * $Id$
+ *
+ * Copyright (C) 2003-2015 JNode.org
+ *
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but 
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public 
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library; If not, write to the Free Software Foundation, Inc., 
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+package org.jnode.fs.service.def;
+
+import java.io.IOException;
+import java.util.Map;
+
+import org.jnode.driver.Device;
+import org.jnode.fs.FileSystem;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
+import static junit.framework.Assert.fail;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for FileSystemAPIImpl.unmount().
+ */
+public class FileSystemAPIImplTest {
+
+    private FileSystemAPIImpl api;
+    private VirtualFS vfs;
+
+    @Before
+    public void setUp() throws Exception {
+        FileSystemManager fsm = new FileSystemManager();
+        Device device = mock(Device.class);
+        when(device.getShortDescription()).thenReturn("mock-device");
+        vfs = new VirtualFS(device);
+        api = new FileSystemAPIImpl(fsm, vfs);
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        api = null;
+        vfs = null;
+    }
+
+    @Test
+    public void testUnmountNonExistentMountPointThrowsIllegalArgument() {
+        try {
+            api.unmount("/nonexistent");
+            fail("Expected IllegalArgumentException");
+        } catch (IllegalArgumentException e) {
+            assertTrue(e.getMessage().contains("Not a mount point"));
+        } catch (IOException e) {
+            fail("Expected IllegalArgumentException, got IOException");
+        }
+    }
+
+    @Test
+    public void testUnmountWithoutLeadingSlash() throws Exception {
+        Device device = mock(Device.class);
+        FileSystem<?> fs = mock(FileSystem.class);
+        when(fs.getDevice()).thenReturn(device);
+        when(fs.isReadOnly()).thenReturn(true);
+
+        api.mount("/mnt", fs, null);
+        assertTrue(api.getMountPoints().containsKey("/mnt"));
+
+        api.unmount("mnt");
+        assertFalse(api.getMountPoints().containsKey("/mnt"));
+    }
+
+    @Test
+    public void testUnmountReadOnlyFsClosesAndUnregisters() throws Exception {
+        Device device = mock(Device.class);
+        FileSystem<?> fs = mock(FileSystem.class);
+        when(fs.getDevice()).thenReturn(device);
+        when(fs.isReadOnly()).thenReturn(true);
+
+        api.mount("/mnt", fs, null);
+        api.unmount("/mnt");
+
+        verify(fs).close();
+        assertFalse(api.getMountPoints().containsKey("/mnt"));
+    }
+
+    @Test
+    public void testUnmountRollbackOnCloseFailure() throws Exception {
+        Device device = mock(Device.class);
+        FileSystem<?> fs = mock(FileSystem.class);
+        when(fs.getDevice()).thenReturn(device);
+        when(fs.isReadOnly()).thenReturn(false);
+
+        api.mount("/mnt", fs, null);
+        assertTrue(api.getMountPoints().containsKey("/mnt"));
+
+        IOException closeException = new IOException("Close failed");
+        doThrow(closeException).when(fs).close();
+
+        try {
+            api.unmount("/mnt");
+            fail("Expected IOException");
+        } catch (IOException e) {
+            assertEquals("Close failed", e.getMessage());
+        }
+
+        Map<String, FileSystem<?>> mountPoints = api.getMountPoints();
+        assertTrue("Mount point should be restored after rollback",
+            mountPoints.containsKey("/mnt"));
+        assertEquals(fs, mountPoints.get("/mnt"));
+    }
+
+    @Test
+    public void testUnmountRemovesMountPoint() throws Exception {
+        Device device = mock(Device.class);
+        FileSystem<?> fs = mock(FileSystem.class);
+        when(fs.getDevice()).thenReturn(device);
+        when(fs.isReadOnly()).thenReturn(true);
+
+        api.mount("/mnt", fs, null);
+        assertTrue(api.getMountPoints().containsKey("/mnt"));
+
+        api.unmount("/mnt");
+        assertFalse(api.getMountPoints().containsKey("/mnt"));
+        assertEquals(0, api.getMountPoints().size());
+    }
+
+    @Test
+    public void testUnmountMultipleMountPoints() throws Exception {
+        Device device1 = mock(Device.class);
+        Device device2 = mock(Device.class);
+        FileSystem<?> fs1 = mock(FileSystem.class);
+        FileSystem<?> fs2 = mock(FileSystem.class);
+        when(fs1.getDevice()).thenReturn(device1);
+        when(fs2.getDevice()).thenReturn(device2);
+        when(fs1.isReadOnly()).thenReturn(true);
+        when(fs2.isReadOnly()).thenReturn(true);
+
+        api.mount("/mnt1", fs1, null);
+        api.mount("/mnt2", fs2, null);
+        assertEquals(2, api.getMountPoints().size());
+
+        api.unmount("/mnt1");
+        assertEquals(1, api.getMountPoints().size());
+        assertTrue(api.getMountPoints().containsKey("/mnt2"));
+
+        api.unmount("/mnt2");
+        assertEquals(0, api.getMountPoints().size());
+    }
+}