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

log2ram for x86? #868

Open
cwiggs opened this issue Jul 27, 2024 · 4 comments
Open

log2ram for x86? #868

cwiggs opened this issue Jul 27, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@cwiggs
Copy link
Contributor

cwiggs commented Jul 27, 2024

Describe the bug
This might be a question more than anything. I noticed in the setup_device.sh script here: https://github.com/mynodebtc/mynode/blob/master/setup/setup_device.sh#L457 it installs log2ram if the system is a RPI or x86 device.

When trying to run this on a Debian 12 VM in proxmox log2ram failed to start. I ended up just disabling it all together since I don't really want logs going to ram on this system. I was thinking it might be better to check if the system is an sdcard and if so setup log2ram.

I'd be willing to submit a PR if you think that is a good idea.

Expected behavior
log2ram should only be enabled if the system is using an sd card.

Screenshots
n/a

Desktop (please complete the following information):
n/a

MyNode hardware (please complete the following information):

  • Device: x86 Debian 12 VM on Proxmox
  • Version: latest / master branch

Additional context

@cwiggs cwiggs added the bug Something isn't working label Jul 27, 2024
@tehelsper
Copy link
Collaborator

Yeah, that change would be beneficial. The model two (which is x86) boots off of a SD card or USB flash drive, so this was added as an optimization for that platform.

It may be better to leave it installed and just disable log2ram if it's not on an SD card during startup or add an option to enable/disable on the settings page manually. The images are typically made using log2ram while they are on an USB drive, so any detection done during setup_device.sh may not represent the final drive the device is actually using. There's also one other install location in mynode_post_upgrade.sh.

@cwiggs
Copy link
Contributor Author

cwiggs commented Jul 29, 2024

The model two (which is x86) boots off of a SD card or USB flash drive, so this was added as an optimization for that platform.

Ah okay that makes sense.

It may be better to leave it installed and just disable log2ram if it's not on an SD card during startup or add an option to enable/disable on the settings page manually.

Yeah that makes sense. For the time being I'll do some work to see why it's failing on Debian 12 and either fix it or create an option to disable starting the service.

The images are typically made using log2ram while they are on an USB drive, so any detection done during setup_device.sh may not represent the final drive the device is actually using.

Okay. I'm just using the setup_device.sh script on a Debian 12 VM so I'm not sure how that merges with the building the image logic. Any suggestions and how to deal with this?

There's also one other install location in mynode_post_upgrade.sh.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh, are you okay if I create a libs directory and put some simple bash scripts in there with common functions that we can import (source) in both of the scripts? This would allow us to write the code once and just use it when we need it in multiple places.

@cwiggs
Copy link
Contributor Author

cwiggs commented Jul 30, 2024

For the time being I'll do some work to see why it's failing on Debian 12 and either fix it or create an option to disable starting the service.

I was able to fix the issue with Debian 12, PR is her: #869

I still think it's good to do a few things:

  • Detect if the system is on an Sdcard, USB, etc and install log2ram by default.
  • Have an option in the mynode webui to enable/disable log2ram.
  • Have a libs directory for common bash function that can just be imported into various scripts.

Let me know what you think about the above things, and the questions from my previous comment and I'd be happy to support PRs for all of this.

Thanks for the app, MyNode has been super helpful for me, I just switched to it from Umbrel.

@tehelsper
Copy link
Collaborator

Glad to hear you've been liking it!

Okay. I'm just using the setup_device.sh script on a Debian 12 VM so I'm not sure how that merges with the building the image logic. Any suggestions and how to deal with this?

OK, I see how you've been using setup_device. That's basically the use case, when running it on a base debian distro, it will add and install and the default apps and software. Your PR change was good.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh, are you okay if I create a libs directory and put some simple bash scripts in there with common functions that we can import (source) in both of the scripts? This would allow us to write the code once and just use it when we need it in multiple places.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh

There is a lot of duplication between the two, but they have different use cases and at this point, that code almost never needs to change. I'd prefer not to change it up at this point. A lot duplicate code that does change was pulled out into a separate script and all future apps use the SDK approach so they go into their own locations without needing to be in either setup_device.sh or mynode_post_upgrade.sh. You can see some of that in mynode_app_versions.sh and rootfs/standard/usr/share/mynode_apps/.

I like the second suggested option the most. The first could be nice, but I worry it could be error-prone. I'm not sure how to consistently detect the type of hardware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants