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

Fix store path copying in Google Compute Engine image build #24264

Merged
merged 1 commit into from
Mar 24, 2017

Conversation

8573
Copy link
Contributor

@8573 8573 commented Mar 24, 2017

In nixos/modules/virtualisation/google-compute-image.nix, copy store
paths with rsync -a rather than cp -prd, because rsync seems
better able to handle the hard-links that may be present in the store,
whereas cp may fail to copy them.

I have tested that the Google Compute Engine image builds successfully
for me with this patch, whereas it did not without this patch.

This is the same fix applied for Azure images in commit
097ef6e.

Fixes #23973.

In `nixos/modules/virtualisation/google-compute-image.nix`, copy store
paths with `rsync -a` rather than `cp -prd`, because `rsync` seems
better able to handle the hard-links that may be present in the store,
whereas `cp` may fail to copy them.

I have tested that the Google Compute Engine image builds successfully
for me with this patch, whereas it did not without this patch.

This is the same fix applied for Azure images in commit
097ef6e.

Fixes NixOS#23973.
@mention-bot
Copy link

@8573, thanks for your PR! By analyzing the history of the files in this pull request, we identified @oconnorr, @rbvermaa and @Phreedom to be potential reviewers.

@8573 8573 changed the title Fix store path copying in Google Compute Image build Fix store path copying in Google Compute Engine image build Mar 24, 2017
@globin globin merged commit e0e520a into NixOS:master Mar 24, 2017
8573 added a commit to 8573/nixpkgs that referenced this pull request Mar 24, 2017
Having fixed the Google Compute Engine image build process's copying
of store paths in PR NixOS#24264, I ran `nixos-rebuild --upgrade switch`...
and the GCE image broke again, because it sets the NixOS configuration
option for the sysctl variable `kernel.yama.ptrace_scope` to
`mkDefault "1"`, i.e., with override priority 1000, and now the
`sysctl` module sets the same option to `mkDefault "0"` (this was
changed in commit 86721a5).

This patch raises the override priority of the Google Compute Engine
image configuration's definition of the Yama sysctl option to 500
(still lower than the priority of an unmodified option definition).

I have tested that this patch allows the Google Compute Engine image
to again build successfully for me.
fpletz pushed a commit that referenced this pull request Mar 26, 2017
Having fixed the Google Compute Engine image build process's copying
of store paths in PR #24264, I ran `nixos-rebuild --upgrade switch`...
and the GCE image broke again, because it sets the NixOS configuration
option for the sysctl variable `kernel.yama.ptrace_scope` to
`mkDefault "1"`, i.e., with override priority 1000, and now the
`sysctl` module sets the same option to `mkDefault "0"` (this was
changed in commit 86721a5).

This patch raises the override priority of the Google Compute Engine
image configuration's definition of the Yama sysctl option to 500
(still lower than the priority of an unmodified option definition).

I have tested that this patch allows the Google Compute Engine image
to again build successfully for me.
@8573
Copy link
Contributor Author

8573 commented Mar 26, 2017

Given @clefru's comment here

[…] the kernel used during the QEMU build phase will corrupt data when talking to /nix/store via 9p. You suffer from this bug if your "cp" command complains about hard-links in /nix/store.

— it seems my patch here was only fixing a symptom without addressing the underlying problem?

@clefru
Copy link
Contributor

clefru commented Mar 27, 2017

@8573: I'd also suspect that rsync just works around the 9p corruption bug. When applying the 9p client fixes, cp doesn't complain anymore, so that's proof enough for me that there are on actual hard-links.

I am not sure on whether I'd keep rsync over cp. Eventually it doesn't matter that much. What I hoped to do was to unify all the image building scripts for GCE/Azure/AWS as they all look very similar to each other. But that's the matter of a different PR.

globin pushed a commit that referenced this pull request Mar 27, 2017
Having fixed the Google Compute Engine image build process's copying
of store paths in PR #24264, I ran `nixos-rebuild --upgrade switch`...
and the GCE image broke again, because it sets the NixOS configuration
option for the sysctl variable `kernel.yama.ptrace_scope` to
`mkDefault "1"`, i.e., with override priority 1000, and now the
`sysctl` module sets the same option to `mkDefault "0"` (this was
changed in commit 86721a5).

This patch raises the override priority of the Google Compute Engine
image configuration's definition of the Yama sysctl option to 500
(still lower than the priority of an unmodified option definition).

I have tested that this patch allows the Google Compute Engine image
to again build successfully for me.

(cherry picked from commit a4ac550)
@8573
Copy link
Contributor Author

8573 commented Apr 5, 2017

@globin, @clefru: Now that the 9p fix has been applied (if I
understand #24143 correctly), do you think that this patch and the
corresponding patch for the Azure images (commit
097ef6e) should be reverted, as they
only masked the problem, and will do so again if the 9p issue
reappears?

adrianpk added a commit to adrianpk/nixpkgs that referenced this pull request May 31, 2024
Having fixed the Google Compute Engine image build process's copying
of store paths in PR NixOS#24264, I ran `nixos-rebuild --upgrade switch`...
and the GCE image broke again, because it sets the NixOS configuration
option for the sysctl variable `kernel.yama.ptrace_scope` to
`mkDefault "1"`, i.e., with override priority 1000, and now the
`sysctl` module sets the same option to `mkDefault "0"` (this was
changed in commit 86721a5).

This patch raises the override priority of the Google Compute Engine
image configuration's definition of the Yama sysctl option to 500
(still lower than the priority of an unmodified option definition).

I have tested that this patch allows the Google Compute Engine image
to again build successfully for me.

(cherry picked from commit a4ac550)
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.

NixOS image for Google Compute Engine fails to build
4 participants