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

errors in prepare-dispvm, cleanup needed #2390

Closed
Rudd-O opened this Issue Oct 22, 2016 · 13 comments

Comments

Projects
None yet
5 participants
@Rudd-O

Rudd-O commented Oct 22, 2016

The dispvm-prerun.sh script in qubes-core-agent-linux does not work correctly:

fedora-23-dvm prepare-dvm.sh[433]: /usr/lib/qubes/dispvm-prerun.sh: line 10: /tmp/dispvm-dotfiles-errors.log: Permission denied

Please do not log junk to /tmp. Either use logger or simply cat to stdout / stderr and let the journal catch the output.

Also, many of these logging outputs are exercising the functions of forgotten debug printfs. There's no need to run these things.

Also, that script is redoing some of the work that mount-dirs.sh is doing. Boot ordering of unit files does not appear to indicate a race, but doing double work is unnecessary.

My recommended solution here (apart from the reasonable cleanup of that script), is as follows:

  • Delete the stuff to set up the DVM homedir from dispvm-prerun.sh. This does not need to be done at that point, because mount-dirs.sh already did it.
  • Split the mount-dirs.sh script and its corresponding unit file into two:
  1. before-mount-home.sh, which must have the exact same Before/After dependencies as the original unit file, but get a Before=rw.mount dependency and an After=dev-xvdb.device. This first script will be in charge of initializing and resizing the block device, as well as nuking the old /home symlink, but it will not attempt to mount anything (as systemd is doing that work in parallel currently, it is racy). Let fstab care of mounting.
  2. after-mount-home.sh, which must have the exact same Before/After dependencies as the original unit file, but also gains a After=rw.mount before-mount-home.service. This script then launches right after /rw has been mounted but before /rw/home is mounted to /home. At this point, the part of the script that goes and creates stuff in /rw can do the work it is doing right now, including unpacking the DVM stuff and mounting the necessary file systems.

That way, the job of preparing the block device is done at one stage, while the job of preparing the file system itself is done separately after /rw has been automounted during boot.

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Oct 22, 2016

Also, if the home user directory needs to be created, because it did not exist or something, just cp -R the stuff from /etc/skel instead of unpacking a buncha dotfiles!

Rudd-O commented Oct 22, 2016

Also, if the home user directory needs to be created, because it did not exist or something, just cp -R the stuff from /etc/skel instead of unpacking a buncha dotfiles!

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Oct 22, 2016

Member

The dispvm-prerun.sh script in qubes-core-agent-linux does not work correctly:

fedora-23-dvm prepare-dvm.sh[433]: /usr/lib/qubes/dispvm-prerun.sh: line 10: /tmp/dispvm-dotfiles-errors.log: Permission denied

Please do not log junk to /tmp. Either use logger or simply cat to stdout / stderr and let the journal catch the output.

Also, many of these logging outputs are exercising the functions of forgotten debug printfs. There's no need to run these things.

Also, that script is redoing some of the work that mount-dirs.sh is doing. Boot ordering of unit files does not appear to indicate a race, but doing double work is unnecessary.

Care to send a pull request?

This first script will be in charge of initializing and resizing the block device, as well as nuking the old /home symlink, but it will not attempt to mount anything (as systemd is doing that work in parallel currently, it is racy). Let fstab care of mounting.

There is no race, as both entries have noauto in /etc/fstab, so systemd do not mount them. But your approach seems like a good solution for #979

Member

marmarek commented Oct 22, 2016

The dispvm-prerun.sh script in qubes-core-agent-linux does not work correctly:

fedora-23-dvm prepare-dvm.sh[433]: /usr/lib/qubes/dispvm-prerun.sh: line 10: /tmp/dispvm-dotfiles-errors.log: Permission denied

Please do not log junk to /tmp. Either use logger or simply cat to stdout / stderr and let the journal catch the output.

Also, many of these logging outputs are exercising the functions of forgotten debug printfs. There's no need to run these things.

Also, that script is redoing some of the work that mount-dirs.sh is doing. Boot ordering of unit files does not appear to indicate a race, but doing double work is unnecessary.

Care to send a pull request?

This first script will be in charge of initializing and resizing the block device, as well as nuking the old /home symlink, but it will not attempt to mount anything (as systemd is doing that work in parallel currently, it is racy). Let fstab care of mounting.

There is no race, as both entries have noauto in /etc/fstab, so systemd do not mount them. But your approach seems like a good solution for #979

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Oct 22, 2016

Is there no support for DVMs on non-systemd templates?

The vm-init.d qubes-core script portion that is equivalent to mount-dirs.sh has no support whatsoever for it.

Rudd-O commented Oct 22, 2016

Is there no support for DVMs on non-systemd templates?

The vm-init.d qubes-core script portion that is equivalent to mount-dirs.sh has no support whatsoever for it.

@Rudd-O Rudd-O referenced this issue in QubesOS/qubes-core-agent-linux Oct 22, 2016

Merged

Clean up early initialization and setup of /rw #21

@andrewdavidwong andrewdavidwong added this to the Release 3.2 milestone Oct 22, 2016

@unman

This comment has been minimized.

Show comment
Hide comment
@unman

unman Oct 23, 2016

Member

@Rudd-O What are these non-systemd templates? Afaik all the current templates use systemd.

Member

unman commented Oct 23, 2016

@Rudd-O What are these non-systemd templates? Afaik all the current templates use systemd.

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Oct 23, 2016

Debian allegedly can use non-systemd boot. And some people may not want to use systemd.

Also, maybe the agent needs to preserve non-systemd support to continue making an agent for the BSDs.

Rudd-O commented Oct 23, 2016

Debian allegedly can use non-systemd boot. And some people may not want to use systemd.

Also, maybe the agent needs to preserve non-systemd support to continue making an agent for the BSDs.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Oct 25, 2016

Member

On Sat, Oct 22, 2016 at 08:17:20AM -0700, Rudd-O wrote:

Is there no support for DVMs on debian? The qubes-core script portion that is equivalent to mount-dirs.sh has no support whatsoever for it.

What do you mean? Exactly the same script should be run there.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Member

marmarek commented Oct 25, 2016

On Sat, Oct 22, 2016 at 08:17:20AM -0700, Rudd-O wrote:

Is there no support for DVMs on debian? The qubes-core script portion that is equivalent to mount-dirs.sh has no support whatsoever for it.

What do you mean? Exactly the same script should be run there.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Oct 27, 2016

Ignore that comment please. It was my ignorance talking.

Rudd-O commented Oct 27, 2016

Ignore that comment please. It was my ignorance talking.

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Nov 8, 2016

Member

After merging QubesOS/qubes-core-agent-linux#21 DispVMs have no .bash_profile, which means ugly bash prompt. This is because /home_volatile/user exists in the template, but is not populated from /etc/skel. Previously .bash_profile was provided in dispvm-dotfiles.tbz.
Any idea how to solve it without rebuilding the whole template (i.e. solve it during package update)? Or maybe ignore it?

Member

marmarek commented Nov 8, 2016

After merging QubesOS/qubes-core-agent-linux#21 DispVMs have no .bash_profile, which means ugly bash prompt. This is because /home_volatile/user exists in the template, but is not populated from /etc/skel. Previously .bash_profile was provided in dispvm-dotfiles.tbz.
Any idea how to solve it without rebuilding the whole template (i.e. solve it during package update)? Or maybe ignore it?

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Nov 13, 2016

The solution would be to copy all files from /etc/skel onto /home/user on boot, except for files that may already exist in /home/user.

Lemme dig into the code.

Rudd-O commented Nov 13, 2016

The solution would be to copy all files from /etc/skel onto /home/user on boot, except for files that may already exist in /home/user.

Lemme dig into the code.

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Nov 13, 2016

Actually, I think the cause you posit may not be the real reason why this happens. Reading code...

Rudd-O commented Nov 13, 2016

Actually, I think the cause you posit may not be the real reason why this happens. Reading code...

@Rudd-O

This comment has been minimized.

Show comment
Hide comment
@Rudd-O

Rudd-O Nov 13, 2016

Here is what we do in mount-dirs.sh when the DVM savefile is created. All this action happens from mount-dirs.sh.

  1. Invoke setup-rw.sh.
  2. Invoke setup-dvm-home.sh.
  3. Mount --bind /home_volatile onto /home.

In step 1, we create /rw and /rw/home/ (based on /etc/skel if /home.orig/ does not exist, else based on /home.orig/).

In step 2 we do nothing unless the user has explicitly demanded /home customization. Customization would involve creating /home_volatile/user and populating it.

I think we need to make step 2 such that the code starting at https://github.com/QubesOS/qubes-core-agent-linux/blob/master/init/setup-rw.sh#L58 executes for the DVM too. We do create /home_volatile/user/(.local/share...) in misc-post-stop.sh. So /home_volatile/user will already exist, and the condition cannot just be "hey, if it doesn't exist, then do the population". I think we may have to populate /home_volatile/user unconditionally based on skel or home.orig. Presumably, this would cause the DVM savefile to end up with a properly-populated /home_volatile/user and therefore a properly-populated /home/user.

I think the right place to do the population is in setup-dvm-home.sh.

What do you think?

Rudd-O commented Nov 13, 2016

Here is what we do in mount-dirs.sh when the DVM savefile is created. All this action happens from mount-dirs.sh.

  1. Invoke setup-rw.sh.
  2. Invoke setup-dvm-home.sh.
  3. Mount --bind /home_volatile onto /home.

In step 1, we create /rw and /rw/home/ (based on /etc/skel if /home.orig/ does not exist, else based on /home.orig/).

In step 2 we do nothing unless the user has explicitly demanded /home customization. Customization would involve creating /home_volatile/user and populating it.

I think we need to make step 2 such that the code starting at https://github.com/QubesOS/qubes-core-agent-linux/blob/master/init/setup-rw.sh#L58 executes for the DVM too. We do create /home_volatile/user/(.local/share...) in misc-post-stop.sh. So /home_volatile/user will already exist, and the condition cannot just be "hey, if it doesn't exist, then do the population". I think we may have to populate /home_volatile/user unconditionally based on skel or home.orig. Presumably, this would cause the DVM savefile to end up with a properly-populated /home_volatile/user and therefore a properly-populated /home/user.

I think the right place to do the population is in setup-dvm-home.sh.

What do you think?

@Rudd-O Rudd-O referenced this issue in QubesOS/qubes-core-agent-linux Nov 13, 2016

Merged

Initialize home_volatile for disposable VMs. #24

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Nov 18, 2016

Member

Package with both PR included is already in current-testing repository.

Member

marmarek commented Nov 18, 2016

Package with both PR included is already in current-testing repository.

@adrelanos

This comment has been minimized.

Show comment
Hide comment
@adrelanos

adrelanos Oct 31, 2017

Member

@marmarek

Package with both PR included is already in current-testing repository.

So I guess this ticket is solved?

Member

adrelanos commented Oct 31, 2017

@marmarek

Package with both PR included is already in current-testing repository.

So I guess this ticket is solved?

@marmarek marmarek closed this Oct 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment