[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] guess_fstype applet
From: Tanguy Pruvot <tanguy.pruvot () gmail ! com>
Date: 2015-04-26 11:57:16
Message-ID: CACOx48pfFAYEJQVBiWZvM20Wh9+Rtw4_SnBJXhF+nMZ+BEgKrQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
here is our (cm) latest version (squashed and applied to 1.23 branch) :
https://github.com/tpruvot/android_external_busybox/commit/783122b34e3647583c746908371b4121a7d249a6
2015-02-12 9:21 GMT+01:00 walter harms <wharms@bfs.de>:
> hi all,
> i was following the discussions a bit and now i have a minor question.
> so the idea guess_fstype is to be a subroutine of mount ?!
> why not make it a proper command ?
> Output could be something like a mount cmdline.
>
> I got this idea because in the past i had the problem of cloning a
> drive setup (keywords: size, raid, lvm). In the end I wrote a small script
> with the right setup but i guess it would be a boom for backup programms
> and guessing the filesystem parameters would be a nice first step.
>
> just my two cents,
> wh
>
>
> Am 12.02.2015 01:20, schrieb Tanguy Pruvot:
> > Thanks, i think it will be another divergence, the main purpose of this
> > patch is for the android recovery which should be able to mount badly set
> > fstype (exemple: for a backup)
> >
> > Even if in CM the recovery can be installed on each nightly, we have the
> > ability to keep a custom one (to handle different backup formats etc...)
> > and the fstab can be different of the recovery one.
> >
> > Now we have another problem about that, its the ability to use encrypted
> > file systems...
> >
> > 2015-02-11 20:37 GMT+01:00 Tanguy Pruvot <tanguy.pruvot@gmail.com>:
> >
> >> Thanks for the review... was merged too fast
> >>
> >> 2015-02-11 20:32 GMT+01:00 Isaac Dunham <ibid.ag@gmail.com>:
> >>
> >>> On Wed, Feb 11, 2015 at 07:50:49PM +0100, Tanguy Pruvot wrote:
> >>>> 2015-02-11 19:48 GMT+01:00 Denys Vlasenko <vda.linux@googlemail.com>:
> >>>>
> >>>>> On Fri, Feb 6, 2015 at 5:35 PM, Tanguy Pruvot <
> >>> tanguy.pruvot@gmail.com>
> >>>>> wrote:
> >>>>>> From CyanogenMod :
> >>>>>>
> >>>>>> http://review.cyanogenmod.org/#/c/87995/
> >>>>>>
> >>>>>> 2013-08-23 6:20 GMT+02:00 James B <jamesbond3142@gmail.com>:
> >>>>>>>
> >>>>>>> If the interface can be simplifed to detect filesystem only
> >>> (instead
> >>>>>>> of also looking for labels and UUID) I think I think it would get
> >>> an
> >>>>>>> order of magnitude speed-up (taking the cues from Puppy Linux's
> >>>>>>> original guess_fstype which basically does just that). I'm not so
> >>> sure
> >>>>>>> about code-size reduction, though.
> >>>>>>>
> >>>>>>> It is called guess_fstype for historical reason (because that's
> >>> how it
> >>>>>>> was used and called in Puppy Linux), I'm happy to change it to
> >>>>>>> "fstype" if that helps everybody else :)
> >>>>>
> >>>>> I'm confused.
> >>>>> Is here something proposed for inclusion into busybox,
> >>>>> or you are now talking about hacks for a particular project only?
> >>>>
> >>>> its a feature to enhance the auto mount with blkid code... we are
> about
> >>> to
> >>>> merge this commit soon in CM12
> >>>
> >>> Comments:
> >>> - do NOT let code override a user specification.
> >>> If I specify -t (ntfs|ntfs-3g) or (msdos|vfat) manually, it should
> force
> >>> mount to try mounting using that specific driver and then *fail* if
> >>> it's impossible.
> >>> If fstype overrides user specifications, you may end up breaking future
> >>> filesystems.
> >>> I would think that this would be better:
> >>> // Treat fstype "auto" as unspecified
> >>> if (mp->mnt_type && strcmp(mp->mnt_type, "auto") == 0)
> >>> mp->mnt_type = NULL;
> >>> +#if ENABLE_FEATURE_BLKID_TYPE
> >>> + // try to to autodetect type
> >>> + if (!mp->mnt_type)
> >>> + mp->mnt_type = get_fstype_from_devname(mp->mnt_fsname);
> >>> #endif
> >>>
> >>> OTOH, if I specify -t auto and userspace code selects the wrong fs type
> >>> (as it surely will, because bugs and unexpected inputs happen...),
> >>> it would be proper for mount to keep retrying, as would be done if
> >>> mp->mnt_type is unset after the new code.
> >>>
> >>> - I'm guessing that the use of != to compare two strings is a mistake?
> >>> + || (detected_fstype != NULL && detected_fstype !=
> mp->mnt_type))
> >>>
> >>> - guess_fstype, fstype, blkid commands:
> >>> guess_fstype is the name that Puppy Linux has used for a command that
> >>> outputs filesystem type without label/uuid.
> >>> fstype is the name that it goes by in Toybox, which is now in Android.
> >>> However, upstream Android turns it off and just uses blkid.
> >>>
> >>> HTH,
> >>> Isaac Dunham
> >>>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
>
[Attachment #5 (text/html)]
<div dir="ltr"><span style="font-size:12.8000001907349px">here is our (cm) latest \
version (squashed and applied to 1.23 branch) :</span><div \
style="font-size:12.8000001907349px"><a \
href="https://github.com/tpruvot/android_external_busybox/commit/783122b34e3647583c746908371b4121a7d249a6" \
target="_blank">https://github.com/tpruvot/android_external_busybox/commit/783122b34e3647583c746908371b4121a7d249a6</a></div></div><div \
class="gmail_extra"><br><div class="gmail_quote">2015-02-12 9:21 GMT+01:00 walter \
harms <span dir="ltr"><<a href="mailto:wharms@bfs.de" \
target="_blank">wharms@bfs.de</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hi all,<br> i \
was following the discussions a bit and now i have a minor question.<br> so the idea \
guess_fstype is to be a subroutine of mount ?!<br> why not make it a proper command \
?<br> Output could be something like a mount cmdline.<br>
<br>
I got this idea because in the past i had the problem of cloning a<br>
drive setup (keywords: size, raid, lvm). In the end I wrote a small script<br>
with the right setup but i guess it would be a boom for backup programms<br>
and guessing the filesystem parameters would be a nice first step.<br>
<br>
just my two cents,<br>
wh<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Am 12.02.2015 01:20, schrieb Tanguy Pruvot:<br>
> Thanks, i think it will be another divergence, the main purpose of this<br>
> patch is for the android recovery which should be able to mount badly set<br>
> fstype (exemple: for a backup)<br>
><br>
> Even if in CM the recovery can be installed on each nightly, we have the<br>
> ability to keep a custom one (to handle different backup formats etc...)<br>
> and the fstab can be different of the recovery one.<br>
><br>
> Now we have another problem about that, its the ability to use encrypted<br>
> file systems...<br>
><br>
> 2015-02-11 20:37 GMT+01:00 Tanguy Pruvot <<a \
href="mailto:tanguy.pruvot@gmail.com">tanguy.pruvot@gmail.com</a>>:<br> ><br>
>> Thanks for the review... was merged too fast<br>
>><br>
>> 2015-02-11 20:32 GMT+01:00 Isaac Dunham <<a \
href="mailto:ibid.ag@gmail.com">ibid.ag@gmail.com</a>>:<br> >><br>
>>> On Wed, Feb 11, 2015 at 07:50:49PM +0100, Tanguy Pruvot wrote:<br>
>>>> 2015-02-11 19:48 GMT+01:00 Denys Vlasenko <<a \
href="mailto:vda.linux@googlemail.com">vda.linux@googlemail.com</a>>:<br> \
>>>><br> >>>>> On Fri, Feb 6, 2015 at 5:35 PM, Tanguy \
Pruvot <<br> >>> <a \
href="mailto:tanguy.pruvot@gmail.com">tanguy.pruvot@gmail.com</a>><br> \
>>>>> wrote:<br> >>>>>> From CyanogenMod :<br>
>>>>>><br>
>>>>>> <a href="http://review.cyanogenmod.org/#/c/87995/" \
target="_blank">http://review.cyanogenmod.org/#/c/87995/</a><br> \
>>>>>><br> >>>>>> 2013-08-23 6:20 GMT+02:00 James \
B <<a href="mailto:jamesbond3142@gmail.com">jamesbond3142@gmail.com</a>>:<br> \
>>>>>>><br> >>>>>>> If the interface can be \
simplifed to detect filesystem only<br> >>> (instead<br>
>>>>>>> of also looking for labels and UUID) I think I think it \
would get<br> >>> an<br>
>>>>>>> order of magnitude speed-up (taking the cues from Puppy \
Linux's<br> >>>>>>> original guess_fstype which basically \
does just that). I'm not so<br> >>> sure<br>
>>>>>>> about code-size reduction, though.<br>
>>>>>>><br>
>>>>>>> It is called guess_fstype for historical reason (because \
that's<br> >>> how it<br>
>>>>>>> was used and called in Puppy Linux), I'm happy to \
change it to<br> >>>>>>> "fstype" if that helps \
everybody else :)<br> >>>>><br>
>>>>> I'm confused.<br>
>>>>> Is here something proposed for inclusion into busybox,<br>
>>>>> or you are now talking about hacks for a particular project \
only?<br> >>>><br>
>>>> its a feature to enhance the auto mount with blkid code... we are \
about<br> >>> to<br>
>>>> merge this commit soon in CM12<br>
>>><br>
>>> Comments:<br>
>>> - do NOT let code override a user specification.<br>
>>> If I specify -t (ntfs|ntfs-3g) or (msdos|vfat) manually, it should \
force<br> >>> mount to try mounting using that specific driver and then \
*fail* if<br> >>> it's impossible.<br>
>>> If fstype overrides user specifications, you may end up breaking \
future<br> >>> filesystems.<br>
>>> I would think that this would be better:<br>
>>> // Treat fstype "auto" as unspecified<br>
>>> if (mp->mnt_type && strcmp(mp->mnt_type, \
"auto") == 0)<br> >>> mp->mnt_type = \
NULL;<br> >>> +#if ENABLE_FEATURE_BLKID_TYPE<br>
>>> + // try to to autodetect type<br>
>>> + if (!mp->mnt_type)<br>
>>> + mp->mnt_type = \
get_fstype_from_devname(mp->mnt_fsname);<br> >>> #endif<br>
>>><br>
>>> OTOH, if I specify -t auto and userspace code selects the wrong fs \
type<br> >>> (as it surely will, because bugs and unexpected inputs \
happen...),<br> >>> it would be proper for mount to keep retrying, as would \
be done if<br> >>> mp->mnt_type is unset after the new code.<br>
>>><br>
>>> - I'm guessing that the use of != to compare two strings is a \
mistake?<br> >>> + || (detected_fstype != NULL && \
detected_fstype != mp->mnt_type))<br> >>><br>
>>> - guess_fstype, fstype, blkid commands:<br>
>>> guess_fstype is the name that Puppy Linux has used for a command \
that<br> >>> outputs filesystem type without label/uuid.<br>
>>> fstype is the name that it goes by in Toybox, which is now in \
Android.<br> >>> However, upstream Android turns it off and just uses \
blkid.<br> >>><br>
>>> HTH,<br>
>>> Isaac Dunham<br>
>>><br>
</div></div><div class="HOEnZb"><div \
class="h5">_______________________________________________<br> busybox mailing \
list<br> <a href="mailto:busybox@busybox.net">busybox@busybox.net</a><br>
<a href="http://lists.busybox.net/mailman/listinfo/busybox" \
target="_blank">http://lists.busybox.net/mailman/listinfo/busybox</a><br> \
</div></div></blockquote></div><br></div>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic