Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] disable suspend if booting from removable storage #3038

Merged
merged 1 commit into from
Mar 24, 2014
Merged

[RFC] disable suspend if booting from removable storage #3038

merged 1 commit into from
Mar 24, 2014

Conversation

stefansaraev
Copy link
Contributor

suspend/resume is miserably broken (by design) when booting from removable storage (usb sticks). lets not allow xbmc to suspend/autosuspend

consoder the following scenario:

  • user boots from usb stick
  • user tries to suspend
  • here usb host got reset. bye bye /dev/sda
  • resume. /dev/sda is no more. bye bye /storage
  • as a side effect - udevil would mount /dev/sda2 as /var/media/Whatever

with this commit, there will be NO "suspend" entry in xbmc shutdown menu. shutdown function CAN NOT be set to "suspend" and is forced as "shutdown"

needs testing on RPi, also need to ensure if it doesnt break something with netboot (this I can test myself next monday).

also, it has been reported several times that resume with NBD/ISCSI boot is broken. let me know if we need to disable suspend also for netboot. EDIT: suspend with nfs is quite fine after we decreased default retrans,timeo options, just some noticeable 5-10 sec delay on resume.

@MilhouseVH
Copy link
Contributor

But when booting x86 from HDD (non-removable) suspend will still be available? I've no problems suspend/resuming a Revo 3700 and HDD (OE4b2). Can test the Pi a bit later today, suspend shouldn't be enabled anyway on the Pi.

@stefansaraev
Copy link
Contributor Author

@MilhouseVH yes, there is no change in behaviour when booting from fixed drive
hint: /sys/class/block/*/../removable

I know that suspend is not available (yet?) on PI. I just ask if someone who have RPi in hand can test for potential regressions

@piotrasd
Copy link
Contributor

where or when this commit was with this changes ?? ""nfs is quite fine after we decreased default retrans,timeo options, just some noticeable 5-10 sec delay on resume. "" ?? im ask because im mount manualy NFS share, over autostart.sh and maybe i should something change (sometimes i have some issue when go to sleep) Thx.

@stefansaraev
Copy link
Contributor Author

@piotrasd 9c1020f but it has nothing to do with manual mounts.

@MilhouseVH
Copy link
Contributor

Tested the patch with Raspberry Pi and no adverse effects.

@stefansaraev
Copy link
Contributor Author

thanks @MilhouseVH
just tested pxe/nfs here. all good.

stefansaraev added a commit that referenced this pull request Mar 24, 2014
[RFC] disable suspend if booting from removable storage
@stefansaraev stefansaraev merged commit 290fafc into OpenELEC:master Mar 24, 2014
@stefansaraev stefansaraev deleted the suspend branch March 24, 2014 11:01
@NedScott
Copy link

This is like fixing a leaky faucet by capping off the water pipes. This is going to have a huge negative impact on a ton of users, and I think you will find that impact will far outweigh what you are trying to avoid. Please reconsider this.

@fritsch
Copy link
Contributor

fritsch commented Mar 31, 2014

Not sure if we should program in 'possibly data loss by default', if that is the better choice. We had a real, real bad bug last time - where we wanted to give users the possibility to write on hfs+ volumes. So safety first.

Concerning the faucet example: If user has a broken / non working faucet - I would not attach those to the public water system and this is what this patch does. It saves users from themselves.

@NedScott
Copy link

Yes, but I'd rather put a bucket under the faucet than not be able to have water to drink and, you know, die.

I understand setting this as a default. I'm all for safety first. However, as mentioned in the forum thread by LossAngeles, there's total refusal to even allow this as some kind of advanced option. Let informed users make a choice. Otherwise the only choice a ton of people will have is to not use OE at all, or worse, keep using an outdated OE. Then which would be the bigger issue, a huge group of users who refuse to stop using a version with bugs and no future support, or letting advanced users make an informed choice?

@fritsch
Copy link
Contributor

fritsch commented Mar 31, 2014

So a "f**k your data" setting If I understand that correctly?

@stefansaraev: If that's easy doable, let's do it - we cannot save the world.

@stefansaraev
Copy link
Contributor Author

whats the problem here?

an informed user could do:

cat > /storage/.config/autostart.sh << EOF
#!/bin/sh
rm /dev/.suspend_disabled
EOF

so. why so much nonsense spam in my mailbox ?

@NedScott
Copy link

@fritsch If you need to name it that way to emphasize the danger, sure :)

@stefansaraev My apologies, for I had originally taken you at your word when you said "there will be NO work-around."

I believe this will be good enough for those users who really want this. Thank you for your time.

@fritsch
Copy link
Contributor

fritsch commented Mar 31, 2014

@stefansaraev
Copy link
Contributor Author

@NedScott there will be NO "official" workaround, like a setting in our settings addon or so. autostart.sh is like rc.local on ubuntu. it is for "informed people who know what they are doing", others should NOT boot from removable storage. hope all of you are happy now.

EDIT: and btw, the reasons are explained in PR description. feel free to send us patches (kernel?) to fix that broken-by-design behaviour.

@Jan1503
Copy link

Jan1503 commented Jan 12, 2015

Hi and sorry for this late comment.
I've just installed Kodi on my new cubox (iMX6 build) and would like to use the suspend-function as well but unfortunately it's greyed out.

I'm a newbie on linux, tried your instructions here, but there is no /dev/.suspend_disabled-file so I assume the option should be enabled but it's not :) Maybe the cpu isn't capable of suspending?

Please give me a quick hint on how to enable suspend on this little box if possible.
Thanks and regards, Jan

@stefansaraev
Copy link
Contributor Author

your platform does not support suspend at all. EDIT: and it has nothing to do with this PR.

@OpenELEC OpenELEC locked and limited conversation to collaborators Jan 12, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants