From 2df36892dd25f10d25ee3266021d4ced0a01b4cd Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Fri, 25 Apr 2014 15:00:12 -0400 Subject: [PATCH] synced_folders/rsync: only chown when necessary [GH-3525] Run remote rsync as root to guarantee that rsync can write to guestpath. This obviates the need to chown the guestpath to the SSH user prior to sync. This brings a substantial speedup (2x on a moderately-sized shared folder) and properly triggers filesystem notifications on only the files changed by a given sync. --- plugins/guests/darwin/cap/rsync.rb | 13 ++++++------ plugins/guests/darwin/plugin.rb | 7 ++++++- plugins/guests/freebsd/cap/rsync.rb | 13 ++++++------ plugins/guests/freebsd/plugin.rb | 7 ++++++- plugins/guests/linux/cap/rsync.rb | 17 +++++----------- plugins/guests/linux/plugin.rb | 4 ++-- plugins/guests/netbsd/cap/rsync.rb | 13 ++++++------ plugins/guests/netbsd/plugin.rb | 7 ++++++- plugins/guests/openbsd/cap/rsync.rb | 13 ++++++------ plugins/guests/openbsd/plugin.rb | 7 ++++++- plugins/guests/smartos/cap/rsync.rb | 13 ++++++------ plugins/guests/smartos/plugin.rb | 7 ++++++- plugins/guests/solaris/cap/rsync.rb | 14 +++++++------ plugins/guests/solaris/plugin.rb | 7 ++++++- plugins/guests/solaris11/cap/rsync.rb | 14 +++++++------ plugins/synced_folders/rsync/helper.rb | 11 ++++++++++ .../plugins/guests/smartos/cap/rsync_test.rb | 20 +------------------ 17 files changed, 105 insertions(+), 82 deletions(-) diff --git a/plugins/guests/darwin/cap/rsync.rb b/plugins/guests/darwin/cap/rsync.rb index 3ddbe485e..e69439af4 100644 --- a/plugins/guests/darwin/cap/rsync.rb +++ b/plugins/guests/darwin/cap/rsync.rb @@ -6,13 +6,14 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "sudo rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + machine.communicate.sudo( + "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/darwin/plugin.rb b/plugins/guests/darwin/plugin.rb index a2fb0b2db..3effee16a 100644 --- a/plugins/guests/darwin/plugin.rb +++ b/plugins/guests/darwin/plugin.rb @@ -41,7 +41,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("darwin", "rsync_pre") do + guest_capability("darwin", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("darwin", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/freebsd/cap/rsync.rb b/plugins/guests/freebsd/cap/rsync.rb index 3dc6d6718..05cec8b72 100644 --- a/plugins/guests/freebsd/cap/rsync.rb +++ b/plugins/guests/freebsd/cap/rsync.rb @@ -12,13 +12,14 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "sudo rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'", shell: "sh") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'", shell: "sh") - end + def self.rsync_post(machine, opts) + machine.communicate.sudo( + "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/freebsd/plugin.rb b/plugins/guests/freebsd/plugin.rb index 8136adfdf..524b57329 100644 --- a/plugins/guests/freebsd/plugin.rb +++ b/plugins/guests/freebsd/plugin.rb @@ -46,7 +46,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("freebsd", "rsync_pre") do + guest_capability("freebsd", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("freebsd", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/linux/cap/rsync.rb b/plugins/guests/linux/cap/rsync.rb index 79e68469f..6df67167a 100644 --- a/plugins/guests/linux/cap/rsync.rb +++ b/plugins/guests/linux/cap/rsync.rb @@ -6,21 +6,14 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, opts) - username = machine.ssh_info[:username] - - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{opts[:guestpath]}'") - comm.sudo("find '#{opts[:guestpath]}' ! -user #{username} -print0 | " + - "xargs -0 -r chown -v #{username}:") - end + def self.rsync_command(machine) + "sudo rsync" end def self.rsync_post(machine, opts) - machine.communicate.tap do |comm| - comm.sudo("find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + - "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") - end + machine.communicate.sudo( + "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/linux/plugin.rb b/plugins/guests/linux/plugin.rb index ab083d2be..2a222b6cc 100644 --- a/plugins/guests/linux/plugin.rb +++ b/plugins/guests/linux/plugin.rb @@ -67,12 +67,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("linux", "rsync_post") do + guest_capability("linux", "rsync_command") do require_relative "cap/rsync" Cap::RSync end - guest_capability("linux", "rsync_pre") do + guest_capability("linux", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/netbsd/cap/rsync.rb b/plugins/guests/netbsd/cap/rsync.rb index 7f8550017..3d2f9cfcd 100644 --- a/plugins/guests/netbsd/cap/rsync.rb +++ b/plugins/guests/netbsd/cap/rsync.rb @@ -13,13 +13,14 @@ module VagrantPlugins 'pkg_add rsync') end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "sudo rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + machine.communicate.sudo( + "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/netbsd/plugin.rb b/plugins/guests/netbsd/plugin.rb index ef822e4d4..6c13658ea 100644 --- a/plugins/guests/netbsd/plugin.rb +++ b/plugins/guests/netbsd/plugin.rb @@ -46,7 +46,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("netbsd", "rsync_pre") do + guest_capability("netbsd", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("netbsd", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/openbsd/cap/rsync.rb b/plugins/guests/openbsd/cap/rsync.rb index 0e591ad62..61a942f30 100644 --- a/plugins/guests/openbsd/cap/rsync.rb +++ b/plugins/guests/openbsd/cap/rsync.rb @@ -13,13 +13,14 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "sudo rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + machine.communicate.sudo( + "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/openbsd/plugin.rb b/plugins/guests/openbsd/plugin.rb index e47c9f65c..5bdba96eb 100644 --- a/plugins/guests/openbsd/plugin.rb +++ b/plugins/guests/openbsd/plugin.rb @@ -46,7 +46,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("openbsd", "rsync_pre") do + guest_capability("openbsd", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("openbsd", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/smartos/cap/rsync.rb b/plugins/guests/smartos/cap/rsync.rb index 0217c4dcc..5c8cbab97 100644 --- a/plugins/guests/smartos/cap/rsync.rb +++ b/plugins/guests/smartos/cap/rsync.rb @@ -6,14 +6,13 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] - sudo = machine.config.smartos.suexec_cmd + def self.rsync_command(machine) + "#{machine.config.smartos.suexec_cmd} rsync" + end - machine.communicate.tap do |comm| - comm.execute("#{sudo} mkdir -p '#{folder_opts[:guestpath]}'") - comm.execute("#{sudo} chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + machine.communicate.sudo("find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/smartos/plugin.rb b/plugins/guests/smartos/plugin.rb index f99f7e498..9d27d80c0 100644 --- a/plugins/guests/smartos/plugin.rb +++ b/plugins/guests/smartos/plugin.rb @@ -41,7 +41,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("smartos", "rsync_pre") do + guest_capability("smartos", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("smartos", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/solaris/cap/rsync.rb b/plugins/guests/solaris/cap/rsync.rb index a470011b2..96851b03c 100644 --- a/plugins/guests/solaris/cap/rsync.rb +++ b/plugins/guests/solaris/cap/rsync.rb @@ -6,13 +6,15 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "#{machine.config.solaris.suexec_cmd} rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + su_cmd = machine.config.solaris.su_cmd + machine.communicate.execute( + "#{su_cmd} find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/guests/solaris/plugin.rb b/plugins/guests/solaris/plugin.rb index d7bb5e474..330410f1f 100644 --- a/plugins/guests/solaris/plugin.rb +++ b/plugins/guests/solaris/plugin.rb @@ -41,7 +41,12 @@ module VagrantPlugins Cap::RSync end - guest_capability("solaris", "rsync_pre") do + guest_capability("solaris", "rsync_command") do + require_relative "cap/rsync" + Cap::RSync + end + + guest_capability("solaris", "rsync_post") do require_relative "cap/rsync" Cap::RSync end diff --git a/plugins/guests/solaris11/cap/rsync.rb b/plugins/guests/solaris11/cap/rsync.rb index 0175bf86d..1b48ddc56 100644 --- a/plugins/guests/solaris11/cap/rsync.rb +++ b/plugins/guests/solaris11/cap/rsync.rb @@ -6,13 +6,15 @@ module VagrantPlugins machine.communicate.test("which rsync") end - def self.rsync_pre(machine, folder_opts) - username = machine.ssh_info[:username] + def self.rsync_command(machine) + "#{machine.config.solaris11.suexec_cmd} rsync" + end - machine.communicate.tap do |comm| - comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'") - comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'") - end + def self.rsync_post(machine, opts) + su_cmd = machine.config.solaris11.su_cmd + machine.communicate.execute( + "#{su_cmd} '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " + + "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}") end end end diff --git a/plugins/synced_folders/rsync/helper.rb b/plugins/synced_folders/rsync/helper.rb index c5c9e7da7..0e8373502 100644 --- a/plugins/synced_folders/rsync/helper.rb +++ b/plugins/synced_folders/rsync/helper.rb @@ -84,6 +84,17 @@ module VagrantPlugins args << "--no-perms" if args.include?("--archive") || args.include?("-a") end + # Disable rsync's owner/group preservation (implied by --archive) unless + # specifically requested, since we adjust owner/group to match shared + # folder setting ourselves. + args << "--no-owner" unless args.include?("--owner") || args.include?("-o") + args << "--no-group" unless args.include?("--group") || args.include?("-g") + + # Tell local rsync how to invoke remote rsync with sudo + if machine.guest.capability?(:rsync_command) + args << "--rsync-path"<< machine.guest.capability(:rsync_command) + end + # Build up the actual command to execute command = [ "rsync", diff --git a/test/unit/plugins/guests/smartos/cap/rsync_test.rb b/test/unit/plugins/guests/smartos/cap/rsync_test.rb index d95ae49a7..e8ff0c8c6 100644 --- a/test/unit/plugins/guests/smartos/cap/rsync_test.rb +++ b/test/unit/plugins/guests/smartos/cap/rsync_test.rb @@ -1,7 +1,7 @@ require File.expand_path("../../../../../base", __FILE__) describe "VagrantPlugins::VagrantPlugins::Cap::Rsync" do - let(:plugin) { VagrantPlugins::GuestSmartos::Plugin.components.guest_capabilities[:smartos].get(:rsync_pre) } + let(:plugin) { VagrantPlugins::GuestSmartos::Plugin.components.guest_capabilities[:smartos].get(:rsync_installed) } let(:machine) { double("machine") } let(:config) { double("config", smartos: VagrantPlugins::GuestSmartos::Config.new) } let(:communicator) { VagrantTests::DummyCommunicator::Communicator.new(machine) } @@ -30,23 +30,5 @@ describe "VagrantPlugins::VagrantPlugins::Cap::Rsync" do end end end - - describe ".rsync_pre" do - let(:username) { "some_user" } - - before do - machine.stub(:ssh_info).and_return({username: username}) - end - - it "creates a local directory" do - communicator.expect_command(%Q(pfexec mkdir -p '/mountpoint')) - plugin.rsync_pre(machine, {guestpath: '/mountpoint'}) - end - - it "chowns local directory to ssh user" do - communicator.expect_command(%Q(pfexec chown -R #{username} '/mountpoint')) - plugin.rsync_pre(machine, {guestpath: '/mountpoint'}) - end - end end