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

improve backup and restore procedure #320

Closed
wants to merge 22 commits into from

Conversation

Blaubart
Copy link
Contributor

@Blaubart Blaubart commented Jul 21, 2022

This backup script stores all XCSoar settings and relevant OpenVario settings like:

  • brightness of the display
  • rotation
  • touch screen calibration
  • language settings
  • dropbear settings
  • SSH, variod and sensord status

backups are stored at USB stick at:

  • openvario/backup/"MAC address of eth0"/
    So you can store more than one backup on the same stick!

We add system info script, too

7lima and others added 10 commits July 11, 2022 00:59
…re procedure" Openvario#315

New backup scripts added as an example
Use eth0 MAC ID in backup path
Make sure backup path exists
Added new scripts to URI list
Simplify recursive copy operation by use of smart rsync features
Replace colon in MAC address by dash
Set --recursive rsync option
Set --recursive rsync option: no need for trailing slash
Use head instead of tail. Comments!
Don't use shell here document, provide input more conservatively
make sure MAC address is used by eth0
improve the backup/restore scripts
backup ssh settings
Straightlined and refactored backup/restore with Blaubart's changes
Put user message before first backup command
Put user message before first backup command
Typo
Typo
including backup of variod and sensord status
Update restore-system.sh
Made header comments more clear for system backup scripts
Cleaner structure for backup path
Made evaluation of path variables whitespace-safe
Take values from files after they have been restored to home directory
Display rsync error code to the user
Removed unnecessary syntactic garbage
Always call systemctl with --quiet option
Mentioned contribution from 7lima & Blaubart
Display rsync error code to the user
Issue status messages only after settings done
In case of error exit prematurely before restoring settings
Fix "case/esac" syntax violation introduced two commits earlier
Use loop for daemons
Use loop for daemons
Comments fixed
Code formatting
add some explanation of the features as comment
Fixed some non-obvious defects
add a script that only uploads XCSoar files
Update ovmenu-ng.sh
Update menu
remove transfer-xcsoar
more compatible to the new menu ovmenu-x from Max
add a new transfer scriot
fixing bugs at transfer-xcsoar
Update openvario-image-testing.bb
fixing bugs
removed scripts, they are no longer necessary
Ignore Eclipse project files
Error fix and general refactoring:
File list delivered in a way so tabs aren't hurting.
Syntactic cleanup.
Bourne Shell compatibility restored.
Code formatting and messages edits.
Moved common code out of case/esac.
case/esac default case message fixed.
Made sure sync is always called.
Variable XCSOAR_PATH reintroduced.
Update transfer-xcsoar.sh
Update transfer-xcsoar.sh
Update transfer-xcsoar.sh
fixed syntax error
system infos and improve transfer script
- removed /etc/opkg from backup. It makes problems with the backup from openvario.shell
- add system info script
add info script to the menu
Update system-info.sh
Don't duplicate if's anymore
adding kernel version and hostname
Revert "fixed syntax error"
This reverts commit 99c6c36.
Revert "Update transfer-xcsoar.sh"
This reverts commit 4c5f0dd.
Revert "Update transfer-xcsoar.sh"
This reverts commit 9bc92c3.
Removing /etc/opkg backup needs no if/fi
Set --progress option for rsync
Formatting
Restore operation has been refactored into a Shell Function
Send error output to STDERR
Show message before first file operation
Visibility of yearned-for final message improved
Provident background system buffer sync to help later syncs finish quicker
Show virtual progress bar
Comments
Message improved
Message integrated that user should wait a moment
…sions

System info script gives an overview which packages and which versions are installed. It also shows IP addresses, kernel, host, system daemon status and other info.
Moved functional code from menu ovmenu-ng.sh into file system-info.sh.
A bunch of new system information was added
- renamed functions and menu items download* to backup*
- renamed functions and menu items upload*   to restore*
- (see transfer-xcsoar.sh for name correspondances)
- split functions for XCSoar restore and XCSoar upload
- gentle reordering
- Provident background system buffer sync to help later syncs finish quicker
- Use eth0 MAC ID in backup path
- Simplify recursive copy operation by use of smart rsync features
- More compatible to the new menu ovmenu-x from Max
- Display rsync error codes to the user
- Propagate rsync error codes to the invoking shell
- Visibility of yearned-for final message improved
- Message integrated that user should wait a moment
- Show virtual progress bar. better user guidance
- Bourne Shell compatibility restored
- Avoid trouble when copying symlinks
- Included into the backup the statuses of:
  - brightness of the display
  - rotation
  - touch screen calibration
  - language settings
  - dropbear settings
  - SSH, variod and sensord status
Corresponds to what was done in commit
  fac62be,
but it was in point of fact part of squashed commit
  83936e9
which is part of the effective ancestry
- Corrected function names so they're now matching
- Prevented redirection from overwriting itself or adding up
- Hints about that same problem in some older code parts
- Whitespace formatting
Copy link
Collaborator

@MaxKellermann MaxKellermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still confused by this.

@@ -23,6 +23,7 @@ SRC_URI = "\
file://update-system.sh \
file://download-igc.sh \
file://transfer-xcsoar.sh \
file://system-info.sh \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has nothing to do with rearranging menu items, and probably this fails the build because this script doesn't exist (yet).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right!

@@ -41,12 +42,14 @@ do_install() {
${S}/update-system.sh \
${S}/download-igc.sh \
${S}/transfer-xcsoar.sh \
${S}/system-info.sh \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

${S}/ov-calibrate-ts.sh \
${D}${bindir}/
cd ${D}${bindir}
ln -s -r transfer-xcsoar.sh restore-system.sh
ln -s -r transfer-xcsoar.sh restore-xcsoar.sh
ln -s -r transfer-xcsoar.sh backup-system.sh
ln -s -r transfer-xcsoar.sh upload-xcsoar.sh
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this script?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is to upload files like XCSoar profile, Flarmnet database etc. to /home/root/.xcsoar. We still have to add this part to transfer-xcsoar.sh

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was: this part of the commit doesn't belong in this commit!

Comment on lines 345 to 350
# Copy usb/usbstick/openvario/upload/xcsoar to /home/root/.xcsoar
function upload_xcsoar() {
/usr/bin/upload-xcsoar.sh >> /tmp/tail.$$ &
dialog --backtitle "OpenVario" --title "Result" --tailbox /tmp/tail.$$ 30 50
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit not only rearranges menu items, but also adds a reference to this script, which doesn't (yet) exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we still have to add it to transfer-xcsoar.sh

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's wrong - this commit is broken!

Comment on lines 23 to 24
echo ' [..........] Starting'
echo ' [#.........] Wait until "DONE !!" appears before you exit!'
echo ' [==========] Starting'
echo ' [#=========] Wait until "DONE !!" appears before you exit!'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this .. an improvement? How? Replace on kind of ASCII art with another? I think the dots are a better "background" graphic, but that's a question of taste.
I think any effort here is useless, because I expect all of this to be replaced with a GUI.

Copy link
Contributor Author

@Blaubart Blaubart Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an improvement, because the menu-x uses proportional font, so we have to change the characters for the bar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this is about displaying a progress bar after the fact from a log file? It doesn't make sense to display a progress bar when the process has already finished, does it?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The echo displays line by line in the console during the script's execution. So it really gives an impression of the ongoing progress.

Comment on lines +22 to +29
# collect status of SSH, variod and sensord
if /bin/systemctl --quiet is-enabled dropbear.socket
then
SSH_STATUS=enabled
else
SSH_STATUS=disabled
fi

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit does three things:

  • move code to a new script (why?)
  • reformat the code
  • add new features

Doing 3 distinct things in 1 commit obscures the code changes, because one cannot see what was changed; the commitdiff is "remove 20 lines here, add 100 lines over there", but no diff possible.

Comment on lines 48 to 52
Download "Download XCSoar to USB" \
Upload "Upload files from USB to XCSoar" \
Update_Maps "Update Maps" \
Upload_XCSoar "Update or upload XCSoar files" \
Backup "Backup XCSoar and OV settings" \
Restore "Restore XCSoar and OV settings" \
Restore_XCSoar "Restore only XCSoar settings" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to grasp this diff. This removes 2 menu items and adds 4 different menu items. Then I read the commit message, but none of this appears to makes sense.

including function upload-xcsoar.sh
Added the function to check whether the profiles stored in .xcsoar are protected or not. When restoring, the protection of the profile may also be restored.
File extension of the backup file removed to avoid misunderstandings
The message after the upload was specified as restore. Upload and restore process have been separated
Copy link
Contributor

@lordfolken lordfolken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think all this configuration should be in a separate partition, and nothing should change on the root partition.

done

# Store if profiles are protected or not
if opkg list-installed | grep "e2fsprogs" > /dev/null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not so portable. Please do a stat command to the executable.

Copy link

@7lima 7lima Apr 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Querying the version of the daemons by a "opkg list-installed" query is not as portable as desired, yes.
But this is an inherited flaw from earlier system-info versions.
It should be improved separately from this pull request.

Comment on lines +68 to +73
if /bin/systemctl --quiet is-enabled dropbear.socket
then echo enabled
elif /bin/systemctl --quiet is-active dropbear.socket
then echo temporary
else echo disabled
fi > /home/root/ssh-status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indenting

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a different compact indentation style deliberately used here.
Futhermore the echo commands' output is redirected by a single if ... fi output redirection.
I welcome any counterproposal to make it even clearer.

# Sync the buffer to be sure data is on disk
# Copy brightness setting
cat /sys/class/backlight/lcd/brightness > /home/root/brightness

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we ever implement some dynamic brightness setting, this would write the current (adapted) brightness into the backup, instead of a configured value. Where is the configured value saved?

# Restore SSH status
case `cat /home/root/ssh-status` in
enabled)
/bin/systemctl enable --quiet --now dropbear.socket
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabling does not start the dropbear daemon. This needs a start in addtion.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right! I'll add it.

Comment on lines +68 to +73
if /bin/systemctl --quiet is-enabled dropbear.socket
then echo enabled
elif /bin/systemctl --quiet is-active dropbear.socket
then echo temporary
else echo disabled
fi > /home/root/ssh-status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indenting

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a different compact indentation style deliberately used here.
Futhermore the echo commands' output is redirected by a single if ... fi output redirection.
I welcome any counterproposal to make it even clearer.

7lima added a commit to 7lima/meta-openvario that referenced this pull request Apr 8, 2023
The system info script gives an overview which packages and which versions are installed.
It also shows IP addresses, kernel, host, system daemon status and other info.

A bunch of new system information was added:
- KERNEL_VERSION
- MAC address of eth0 (also relevant for backups)
- HOSTNAME
- versions of:
  - I2C_TOOLS
  - E2FSPROGS
  - USB_MODESWITCH
- daemon status of:
  - SSH
  - variod
  - sensord

The output is no more done with a "dialog" message box but directly echoed to the console.

Yes, querying the version of the daemons by a "opkg list-installed" is not as portable as desired:
Openvario#320 (comment)
But this is an inherited flaw from earlier system-info versions.
It should be improved separately from this commit.
@@ -89,4 +89,4 @@ echo -e '\n'
echo ' Status of SSH, variod and sensord:'
echo ' SSH is '$SSH_STATUS
echo ' variod is '$VARIOD_STATUS
echo ' sensord is '$SENSORD_STATUS
echo ' sensord is '$SENSORD_STATUS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing new line at end of file

echo " [#######===] brightness setting has been restored."

# Restore rotation setting
grep "rotation" /boot/config.uEnv | cut -d '=' -f 2 | tr -d '"' > /sys/class/graphics/fbcon/rotate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm horrified by this line. And I'm horrified by this whole PR which adds shitloads of fragile shell script code.

@7lima
Copy link

7lima commented Apr 23, 2023

I still think all this configuration should be in a separate partition, and nothing should change on the root partition.

Can you give some more details about that, please?

7lima added a commit to 7lima/meta-openvario that referenced this pull request Apr 23, 2023
- Provident background system buffer sync to help later syncs finish quicker
- Use eth0 MAC ID in backup path
- Simplify recursive copy operation by use of smart rsync features
- More compatible to the new menu ovmenu-x from Max
- Display rsync error codes to the user
- Propagate rsync error codes to the invoking shell
- Visibility of yearned-for final message improved
- Message integrated that user should wait a moment
- Show virtual progress bar. better user guidance
- Bourne Shell compatibility restored
- Avoid trouble when copying symlinks
- Included into the backup the statuses of:
  - brightness of the display
  - rotation
  - touch screen calibration
  - language settings
  - dropbear settings
  - SSH, variod and sensord status
- 3 backup/restore functions added:
  - Backup         "Backup XCSoar and OV settings"
  - Restore        "Restore XCSoar and OV settings"
  - Restore_XCSoar "Restore only XCSoar settings"

This is on a branch to be proposed as a new PR in response to the PR Openvario#320 comments.
@lordfolken
Copy link
Contributor

lordfolken commented Apr 25, 2023

I still think all this configuration should be in a separate partition, and nothing should change on the root partition.

Can you give some more details about that, please?

IMO there should be a root parition and a data partition.
The root contains the os + applications.
The data partition is bascially /root/ or preferred /home/xcsoaruser
in there are all the files:

  • for xcsoar
  • persisitent settings for (rotation, brightness etc.)
  • network configurations such as SIDs for wlan
  • settings fo bluetooth

The root partition can be upgraded by replacing the disk image. That way upgrades are consistent and the user doesn't loose their data between upgrades.

@7lima
Copy link

7lima commented Apr 29, 2023

I still think all this configuration should be in a separate partition, and nothing should change on the root partition.
Can you give some more details about that, please?
IMO there should be a root parition and a data partition.
...
The root partition can be upgraded by replacing the disk image. That way upgrades are consistent and the user doesn't loose their data between upgrades.
That's a very good point, thank you. I agree. So let's finish this pull request first and put this feature into a new one.

@linuxianer99
Copy link
Member

How should we handle to ovmenu-ng vs. x-menu ?

I would suggest putting all effort to x-menu ...

@Blaubart
Copy link
Contributor Author

I fully agree. I think we can close this PR because of PR #352. But in general the script can be used in the X menu without any problems. I already use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants