Date: Sun, 14 Jan 2007 14:54:28 -0600 From: "Scot Hetzel" <swhetzel@gmail.com> To: "Divacky Roman" <xdivac02@stud.fit.vutbr.cz> Cc: Alexander Leidinger <Alexander@leidinger.net>, freebsd-emulation@freebsd.org Subject: Re: linuxolator: proc/filesystems and sysfs function implementations Message-ID: <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com> In-Reply-To: <20070114105239.GA6955@stud.fit.vutbr.cz> References: <790a9fff0701060012x40063f48pc842510b082df5a5@mail.gmail.com> <20070106130830.3c2e6d98@Magellan.Leidinger.net> <790a9fff0701132017g6c081567la4a759cea4618535@mail.gmail.com> <20070114105239.GA6955@stud.fit.vutbr.cz>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_34096_20425344.1168808068782
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On 1/14/07, Divacky Roman <xdivac02@stud.fit.vutbr.cz> wrote:
> a few notes..
>
> + switch (args->option) {
> + /*
> + * Translate the filesystem identifier string into a
> + * filesystem type index.
> + */
> + case 1:
>
> I'd use #define BAH 1. I know linux code uses 1,2,3 but we are not linux :)
>
Ok, I'll change them to GETFSIND, GETFSTYP, and GETNFSTYP, found them
defined in a couple of other online man pages (sgi, HP-UX, ..) for
sysfs.
http://www.docs.hp.com/en/B9106-90009/sysfs.2.html
> + if (error) {
> + LFREEPATH(name);
>
> use free() here. the LFREEPATH is meant to be used with the LCONVPATH...
> macros.
>
Wasn't sure, if I should use free or LFREEPATH, it's simple enough to change.
> + if (!vfsp)
> + return EINVAL;
>
> style - return (EINVAL);
>
Missed that one, will make the change.
> + buf = (char *)(uintptr_t)args->arg2;
:
> + bsd_to_linux_fs(vfsp->vfc_name, &fs);
> + len = strlen(fs.name) + 1;
> + error = copyout(fs.name, buf, len);
>
> no need to check if it fits in the userspace buffer? also you
> dont check return value
>
According to the linux man page, there isn't a way to check the size
of buf, as we are only passed the value of the pointer in args->arg2.
Should I replace buf with (char *)(uintptr_t)args->arg2 in the
copyout, since it is only used there.
http://www.die.net/doc/linux/man/man2/sysfs.2.html
I didn't check the return value, as error is going to be set to either
0 or EFAULT by the copyout function. Since it's the end of the case,
it falls thru to the return statement at the bottom of the function.
> + break;
> +
> + /*
> + * Return the total number of file system types currently present
> + * in the kernel.
> + */
> + case 3:
> + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list)
> + index++;
> + td->td_retval[0] = index;
>
> does this make sense? shouldnt we return just the number of filesystems
> that we share with linux?
>
There are only 7 filesystems that are freebsd specific (devfs,
fdescfs, mfs, nullfs, portalfs, procfs (bsd), unionfs). If we exclude
these filesystems, then we would need to ensure that we skip them in
the other two options.
The way that it is currently emulated, is that all freebsd filesystems
(kldloaded filesystems) are available to the linuxolator.
> + break;
> + default:
> + error = EINVAL;
> + }
> + return (error);
> +}
> Index: compat/linux/linux_util.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/compat/linux/linux_util.c,v
> retrieving revision 1.31
> diff -u -r1.31 linux_util.c
> --- compat/linux/linux_util.c 15 Aug 2006 12:54:29 -0000 1.31
> +++ compat/linux/linux_util.c 14 Jan 2007 02:11:08 -0000
> @@ -224,3 +224,82 @@
>
> return (EINVAL);
> }
> +
> +void
> +bsd_to_linux_fs(char *name, struct linux_file_system_type *fs)
> +{
> +#define L_NODEV(fname) \
> + if (strcmp(fname, name) == 0) \
> + fs->fs_flags = 0;
> +#define F2L_NAME(fname, lname) \
> + if (strcmp(fname, name) == 0) \
> + strcpy(fs->name, lname);
> +
> + L_NODEV("9p")
> + else L_NODEV("afs")
> + else L_NODEV("autofs")
> + else L_NODEV("cifs")
> + else L_NODEV("coda")
> + else L_NODEV("configfs")
> + else L_NODEV("debugfs")
> + else L_NODEV("devfs")
> + else L_NODEV("devpts")
> + else L_NODEV("fdescfs")
> + else L_NODEV("fuse")
> + else L_NODEV("hostfs")
> + else L_NODEV("hppfs")
> + else L_NODEV("hugetlbfs")
> + else L_NODEV("jffs2")
> + else L_NODEV("mfs")
> + else L_NODEV("ncpfs")
> + else L_NODEV("nfs")
> + else L_NODEV("nfs4")
> + else L_NODEV("nfsd")
> + else L_NODEV("nullfs")
> + else L_NODEV("openpromfs")
> + else L_NODEV("portalfs")
> + else L_NODEV("procfs")
> + else L_NODEV("linprocfs")
> + else L_NODEV("ramfs")
> + else L_NODEV("rootfs")
> + else L_NODEV("smbfs")
> + else L_NODEV("linsysfs")
> + else L_NODEV("unionfs")
> + else
> + fs->fs_flags = 1; /* FS_REQUIRES_DEV */
> +
> + F2L_NAME("ext2fs", "ext2")
> + else F2L_NAME("cd9660", "iso9660")
> + else F2L_NAME("msdosfs", "msdos")
> + else F2L_NAME("procfs", "bsdprocfs")
> + else F2L_NAME("linprocfs", "proc")
> + else F2L_NAME("linsysfs", "sysfs")
> + else F2L_NAME("ffs", "ufs")
> + else
> + strcpy(fs->name, name);
> +#undef L_NODEV
> +#undef F2L_NAME
>
> man, this is ugly :) cant you come up with some translation table or something?
>
I had created a table (see attached linux.fs), but wasn't sure how to
parse it, or how to keep it up todate as more filesystems are added.
> also... you strcpy string to another string, are you sure it always fits? how do you
> asure that?
>
name can't be larger than MFSNAMELEN, and sizeof(fs->name) == MFSNAMELEN.
> RCS file: /home/ncvs/src/sys/compat/linux/linux_util.h,v
> retrieving revision 1.28
> diff -u -r1.28 linux_util.h
> --- compat/linux/linux_util.h 27 Jun 2006 18:30:49 -0000 1.28
> +++ compat/linux/linux_util.h 14 Jan 2007 02:08:42 -0000
> @@ -101,4 +101,12 @@
> char *linux_get_char_devices(void);
> void linux_free_get_char_devices(char *string);
>
> +struct linux_file_system_type {
> + char name[16];
> + int fs_flags;
> +};
>
> 16? why 16? please comment or something
>
16 is the value of MFSNAMELEN and used by the vfsconf struct to define
the max size of vfc_name. I kept name (in the linux_file_system_type
struct) the same size, so that we didn't have to worry about the size
when using strcpy.
I also wasn't sure if I should add:
#include <sys/mount.h>
in linux_util.h so that MFSNAMELEN could be used to define the size of
name in the struct (it may also affect other files, that include
linux_util.h).
Scot
--
DISCLAIMER:
No electrons were mamed while sending this message. Only slightly bruised.
------=_Part_34096_20425344.1168808068782
Content-Type: text/plain; name=linux.fs; charset=ANSI_X3.4-1968
Content-Transfer-Encoding: base64
X-Attachment-Id: f_ewxubaua
Content-Disposition: attachment; filename="linux.fs"
CmxfbmFtZQkJZl9uYW1lCQlrX21vZHVsZSwJZmxhZ3MJCQlkZXNjcmlwdGlvbgoiOXAiLAkJTlVM
TCwJCU5VTEwsCQlOVUxMLAkJCSJQbGFuIDkgRmlsZXN5c3RlbSIsCiJhZGZzIiwJCU5VTEwsCQlO
VUxMLAkJRlNfUkVRVUlSRVNfREVWLAkiIiwKImFmZnMiLAkJTlVMTCwJCU5VTEwsCQlGU19SRVFV
SVJFU19ERVYsCSJBbWlnYSBGaWxlc3lzdGVtIiwKImFmcyIsCQlOVUxMLAkJTlVMTCwJCUZTX0JJ
TkFSWV9NT1VOVERBVEEsCSJBRlMgQ2xpZW50IEZpbGVzeXN0ZW0iLAoiYXV0b2ZzIiwJTlVMTCwJ
CU5VTEwsCQlOVUxMLAkJCSJBdXRvIEZpbGVzeXN0ZW0iLAoiYmVmcyIsCQlOVUxMLAkJTlVMTCwJ
CUZTX1JFUVVJUkVTX0RFViwJIkJlT1MgRmlsZXN5c3RlbSAoQmVGUykiLAoiYmZzIiwJCU5VTEws
CQlOVUxMLAkJRlNfUkVRVUlSRVNfREVWLAkiU0NPIFVuaXhXYXJlIEJGUyBGaWxlc3lzdGVtIiwK
ImNpZnMiCQlOVUxMLAkJTlVMTCwJCU5VTEwsCQkJIlNOSUEgQ0lGUyBGaWxlc3lzdGVtIiwKImNv
ZGEiLAkJTlVMTCwJCU5VTEwsCQlGU19CSU5BUllfTU9VTlREQVRBLAkiIiwKImNvbmZpZ2ZzIglO
VUxMLAkJTlVMTCwJCU5VTEwsCQkJIlNpbXBsZSBSQU0gRmlsZXN5c3RlbSBmb3IgdXNlciBkcml2
ZW4ga2VybmVsIHN1YnN5c3RlbSBjb25maWd1cmF0aW9uLiIsCiJjcmFtZnMiLAlOVUxMLAkJTlVM
TCwJCUZTX1JFUVVJUkVTX0RFViwJIkNvbXByZXNzZWQgUk9NIEZpbGVzeXN0ZW0iLAoiZGVidWdm
cyIsCU5VTEwsCQlOVUxMLAkJTlVMTCwJCQkiIiwKTlVMTCwJCSJkZXZmcyIsCSJkZXZmcyIsCU5V
TEwsCQkJIkZyZWVCU0QgRGV2aWNlIEZpbGVzeXN0ZW0iLAoiZGV2cHRzIiwJTlVMTCwJCU5VTEwJ
CU5VTEwsLAkJCSIiLAoiZWZzIiwJCU5VTEwsCQlOVUxMLAkJRlNfUkVRVUlSRVNfREVWLAkiIiwK
ImV4dDIiLAkJImV4dDJmcyIsCSJleHQyZnMiLAlGU19SRVFVSVJFU19ERVYsCSJTZWNvbmQgRXh0
ZW5kZWQgRmlsZXN5c3RlbSIsCiJleHQzIiwJCSJleHQyZnMiLAkiZXh0MmZzIiwJRlNfUkVRVUlS
RVNfREVWLAkiU2Vjb25kIEV4dGVuZGVkIEZpbGVzeXN0ZW0gd2l0aCBKb3VybmFsaW5nIiwKTlVM
TCwJCSJmZGVzY2ZzIiwJImZkZXNjZnMiLAlOVUxMLAkJCSJGcmVlQlNEIEZpbGUgRGVzY3JpcHRv
ciBGaWxlc3lzdGVtIiwKImZ1c2UiCQlOVUxMLAkJTlVMTCwJCU5VTEwsCQkJIkZpbGVzeXN0ZW0g
aW4gVXNlcnNwYWNlIiwKImhmcyIsCQlOVUxMLAkJTlVMTCwJCUZTX1JFUVVJUkVTX0RFViwJIk1h
Y2ludG9zaCBGaWxlc3lzdGVtIiwKImhmc3BsdXMiCU5VTEwsCQlOVUxMLAkJRlNfUkVRVUlSRVNf
REVWLAkiRXh0ZW5kZWQgTWFjaW50b3NoIEZpbGVzeXN0ZW0iLAoiaG9zdGZzIglOVUxMLAkJTlVM
TCwJCU5VTEwsCQkJIiIsCiJocGZzIiwJCU5VTEwsCQlOVUxMLAkJRlNfUkVRVUlSRVNfREVWLAki
IiwKImhwcGZzIiwJTlVMTCwJCU5VTEwsCQlOVUxMLAkJCSIiLAoiaHVnZXRsYmZzIiwJTlVMTCwJ
CU5VTEwsCQlOVUxMLAkJCSIiLAoiaXNvOTY2MCIsCSJjZDk2NjAiLAkiY2Q5NjYwIiwJRlNfUkVR
VUlSRVNfREVWLAkiIiwKImpmZnMiLAkJTlVMTCwJCU5VTEwsCQlGU19SRVFVSVJFU19ERVYsCSJK
b3VybmFsbGluZyBGbGFzaCBGaWxlc3lzdGVtIiwKImpmZnMyIiwJTlVMTCwJCU5VTEwsCQlOVUxM
LAkJCSJUaGUgSm91cm5hbGxpbmcgRmxhc2ggRmlsZXN5c3RlbSwgdjIiLAoiamZzIiwJCU5VTEws
CQlOVUxMLAkJRlNfUkVRVUlSRVNfREVWLAkiVGhlIEpvdXJuYWxlZCBGaWxlc3lzdGVtIChKRlMp
IiwKTlVMTCwJCSJtZnMiCQkiZ19tZCIsCQlOVUxMLAkJCSJGcmVlQlNEIE1lbW9yeSBGaWxlc3lz
dGVtIiwKIm1pbml4IiwJTlVMTCwJCU5VTEwsCQlGU19SRVFVSVJFU19ERVYsCSIiLAoibXNkb3Mi
LAkibXNkb3NmcyIsCSJtc2Rvc2ZzIiwJRlNfUkVRVUlSRVNfREVWLAkiTVMtRE9TIGZpbGVzeXN0
ZW0gc3VwcG9ydCIsCiJuY3BmcyIsCU5VTEwsCQlOVUxMLAkJTlVMTCwJCQkiIiwKIm5mcyIsCQki
bmZzIiwJCSJuZnMiLAkJRlNfT0REX1JFTkFNRXxGU19SRVZBTF9ET1R8RlNfQklOQVJZX01PVU5U
REFUQSwJIiIsCiJuZnM0IiwJCSJuZnM0IiwJCSJuZnM0IiwJCUZTX09ERF9SRU5BTUV8RlNfUkVW
QUxfRE9UfEZTX0JJTkFSWV9NT1VOVERBVEEsCSIiLAoibmZzZCIsCQlOVUxMLAkJTlVMTCwJCU5V
TEwsCQkJIiIsCiJudGZzIgkJIm50ZnMiLAkJIm50ZnMiLAkJRlNfUkVRVUlSRVNfREVWLAkiTlRG
UyAxLjIvMy54IGRyaXZlciAtIENvcHlyaWdodCAoYykgMjAwMS0yMDA2IEFudG9uIEFsdGFwYXJt
YWtvdiIsCk5VTEwsCQkibnVsbGZzIiwJIm51bGxmcyIJTlVMTCwJCQkiRnJlZUJTRCBOdWxsIEZp
bGVzeXN0ZW0iLAoib2NmczIiCQlOVUxMLAkJTlVMTCwJCUZTX1JFUVVJUkVTX0RFViwJIk9DRlMy
IDEuMy4zIiwKIm9wZW5wcm9tZnMiLAlOVUxMLAkJTlVMTCwJCU5VTEwsCQkJIiIsCk5VTEwsCQki
cG9ydGFsZnMiLAkicG9ydGFsZnMiLAlOVUxMLAkJCSJGcmVlQlNEIFBvcnRhbCBGaWxlc3lzdGVt
IiwKTlVMTCwJCSJwcm9jZnMiLAkicHJvY2ZzIiwJTlVMTCwJCQkiRnJlZUJTRCBQcm9jZXNzIEZp
bGVzeXN0ZW0iLAoicHJvYyIsCQkibGlucHJvY2ZzIiwJImxpbnByb2NmcyIsCU5VTEwsCQkJIkxp
bnV4IFByb2Nlc3MgRmlsZXN5c3RlbSIsCiJxbng0IiwJCU5VTEwsCQlOVUxMLAkJRlNfUkVRVUlS
RVNfREVWLAkiUU5YNCBmaWxlIHN5c3RlbSIsCiJyYW1mcyIsCU5VTEwsCQlOVUxMLAkJTlVMTCwJ
CQkiUmVzaXphYmxlIHNpbXBsZSBSQU0gRmlsZXN5c3RlbSIsCiJyb290ZnMiLAlOVUxMLAkJTlVM
TCwJCU5VTEwsCQkJIiIsCiJyZWlzZXJmcyIsCSJyZWlzZXJmcyIsCSJyZWlzZXJmcyIsCUZTX1JF
UVVJUkVTX0RFViwJIlJlaXNlckZTIEpvdXJuYWxlZCBGaWxlc3lzdGVtIiwKInJvbWZzIgkJTlVM
TCwJCU5VTEwsCQlGU19SRVFVSVJFU19ERVYsCSJST01GUyBGaWxlc3lzdGVtIiwKInNtYmZzIiwJ
InNtYmZzIiwJInNtYmZzIiwJRlNfQklOQVJZX01PVU5UREFUQSwJIiIsCiJzeXNmcyIsCSJsaW5z
eXNmcyIsCSJsaW5zeXNmcyIsCU5VTEwsCQkJIkxpbnV4IFN5c3RlbSBGaWxlc3lzdGVtIiwKInN5
c3YiLAkJTlVMTCwJCU5VTEwsCQlGU19SRVFVSVJFU19ERVYsCSIiLAoidjciLAkJTlVMTCwJCU5V
TEwsCQlGU19SRVFVSVJFU19ERVYsCSIiLAoidWRmIgkJInVkZiIsCQkidWRmIiwJCUZTX1JFUVVJ
UkVTX0RFViwJIlVuaXZlcnNhbCBEaXNrIEZvcm1hdCBGaWxlc3lzdGVtIiwKTlVMTCwJCSJmZnMi
LAkJInVmcyIsCQlGU19SRVFVSVJFU19ERVYsCSJCZXJrZWxleSBGYXN0IEZpbGVzeXN0ZW0iLAoi
dWZzIiwJCSJ1ZnMiLAkJInVmcyIsCQlGU19SRVFVSVJFU19ERVYsCSJCZXJrZWxleSBGYXN0IEZp
bGVzeXN0ZW0iLApOVUxMLAkJInVuaW9uZnMiLAkidW5pb25mcyIJTlVMTCwJCQkiRnJlZUJTRCBV
bmlvbiBGaWxlc3lzdGVtIiwKInZmYXQiLAkJIm1zZG9zZnMiLAkibXNkb3NmcyIsCUZTX1JFUVVJ
UkVTX0RFViwJIlZGQVQgRmlsZXN5c3RlbSIsCiJ2eGZzIiwJCU5VTEwsCQlOVUxMLAkJRlNfUkVR
VUlSRVNfREVWLAkiVmVyaXRhcyBGaWxlc3lzdGVtIChWeEZTKSBkcml2ZXIiLAoieGZzIiwJCSJ4
ZnMiLAkJInhmcyIsCQlGU19SRVFVSVJFU19ERVYsCSJTR0kgWEZTIiwKTlVMTCwJCSJ6ZnMiCQki
emZzIgkJRlNfUkVRVUlSRVNfREVWLAkiU29sYXJpcyBaZXR0YWJ5dGUgRmlsZSBTeXN0ZW0i
------=_Part_34096_20425344.1168808068782--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?790a9fff0701141254s2c92b10ag6b042a019bc283c>