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

Added option to specify target mount point #85

Closed
wants to merge 1 commit into from

Conversation

psychopenguin
Copy link

Hi there,

This PR implements the option --mountpoint where one can specify the target mount point to be used when the volume is mounted in more that one location. Fix for #71

UUIDS=$($ssh findmnt -n -v -t btrfs -o UUID --list)
fi

if [[ "$mountpoint_cmdline" == "none" ]]; then
TARGETS=$DETECTED_TARGETS
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

The indention is wrong here

TARGETS=$DETECTED_TARGETS
else
if grep -q "$mountpoint_cmdline" <<< "$DETECTED_TARGETS" ;then
TARGETS=$mountpoint_cmdline
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldnt it make sense to escape the variable here when the path contains spaces?

if [[ "$mountpoint_cmdline" == "none" ]]; then
TARGETS=$DETECTED_TARGETS
else
if grep -q "$mountpoint_cmdline" <<< "$DETECTED_TARGETS" ;then
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if this single grep is satisfying enough. Or at least it could be improved. I did not test this, but what if detected targets contains /mnt/test but you specified /mnt via the command line? An exact match would be better here.

@wesbarnett
Copy link
Collaborator

Closing since no response to requests for more info

@wesbarnett wesbarnett closed this Jan 23, 2021
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.

3 participants