Hi Eric, Please see http://jp-andre.pagesperso-orange.fr/systcomp.tar.gz with (most of) your comments taken into account.
For creating a new compressed file, the procedure would be : - create a new void file - "truncate" it to the desired size (hence a void sparse file) - set reparse data for the desired compression mode - feed the data sequentially. Only the last step would go through the plugin, and the plugin knows how much space has to be reserved for the pointers to compression blocks. Jean-Pierre Eric Biggers wrote: > On Mon, Nov 23, 2015 at 08:49:44AM +0100, Jean-Pierre André wrote: >> See proposal on >> http://jp-andre.pagesperso-orange.fr/systcomp.patches.gz > > Hi, > > Great work! It looks cleaner than I had expected. I have a few questions and > comments: > > Should plugin_operations_t really be declared in reparse.h? reparse.h is > supposed to be a header for the library, but the plugins are specific to the > FUSE driver. > > I think the plugins will need to always be loaded from a fixed location set > during the build. The 'launch_path' logic effectively allows users to load > arbitrary plugins into ntfs-3g, which is a problem if someone has ntfs-3g > installed setuid root. > > Naming: should the word "reparse" be included? For example: > "ntfs_reparse_plugin_operations_t", "ntfs_plugin_reparse_80000017.so". In the > code, just calling them "plugins" seems too vague. Of course, the way the > plugins are presented to users can still be feature-oriented, e.g. "this > NTFS-3g > plugin allows you to read system-compressed files". > > With a reparse plugin for tag 0x80000017 I will get sent all the WOF reparse > points, including WIM-backed files ("WIMBoot" files) and anything Microsoft > may > add to WOF in the future, not just system-compressed files. On unsupported > files, should I just fail operations with EOPNOTSUPP? Or should I pretend the > file is a symlink to "unsupported reparse point"? > > There is an edge case in ntfs_get_reparse_tag(). If the reparse tag is 0, the > function returns 0 but doesn't set errno. This could be fixed by having > valid_reparse_data() consider reparse tags of 0 to be invalid. layout.h > already > defines the constant IO_REPARSE_TAG_RESERVED_ZERO, and Windows considers this > value to be invalid as well. > > In the FUSE driver, there could be a reusable function which combines > ntfs_get_reparse_tag() and get_reparse_plugin(). > > reparse_getattr() and reparse_readlink() sound overly generic, since they > don't > handle all types of reparse points. Maybe they should be renamed to > symlink_getattr() and symlink_readlink(). Or perhaps > link_reparse_point_getattr() and link_reparse_point_readlink()? > > The reparse plugin operations need to be clearly documented so that plugin > authors know what to do. > > It probably makes the most sense to pass the reparse data to the plugin > operations. Otherwise, almost every operation would end up immediately > reading > the reparse data anyway. There would, however, need to be a new version of > ntfs_make_symlink() that takes in the reparse data directly. > > With the proposed hook placement in ntfs_fuse_read(), there is no way for the > plugin to support reading named data streams. For system compressed files, > only > the default behavior is needed. So to fix this, it would be sufficient to > only > call the plugin if the unnamed data stream is being read. That may be fine, > and > ntfs_fuse_getattr() already has similar behavior. But I don't know whether > that > behavior is necessarily what all plugins will want. > > There is an open() plugin operation, but isn't called from anywhere. > > Do plugins need to export a "cleanup" or "deinit" function to go along with > "init"? Or should it be assumed that either plugins won't allocate global > resources or will use library destructors (__attribute__(cleanup) with gcc)? > > Eric ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140 _______________________________________________ ntfs-3g-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel
