mpalmer / lvmsync

Synchronise LVM LVs across a network by sending only snapshotted changes
http://theshed.hezmatt.org/lvmsync
GNU General Public License v3.0
382 stars 61 forks source link

lvmsync fail if lv contains "-" #13

Closed dpeddi closed 10 years ago

dpeddi commented 10 years ago

lvmsync fail if contains "-" since parser fail. By adding the - to pattern, lvmsync fail again since devmapper duplicate "-"

plivox commented 10 years ago

Hello everybody,

I have the same problem, here is my distribution:

$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 7.4 (wheezy)
Release:    7.4
Codename:   wheezy

And my Ruby version:

$ ruby -v
ruby 1.9.3p194 (2012-04-20 revision 35410) [x86_64-linux]

My first problem relates with Open3 and the "exitstatus" function:

/home/vincent/lvmsync/lib/lvm/vg_config.rb:78:in `block in vgcfgbackup_output': undefined method `exitstatus' for nil:NilClass (NoMethodError)
    from /usr/lib/ruby/1.9.1/tempfile.rb:320:in `open'
    from /home/vincent/lvmsync/lib/lvm/vg_config.rb:59:in `vgcfgbackup_output'
    from /home/vincent/lvmsync/lib/lvm/vg_config.rb:22:in `initialize'
    from ./lvmsync2:157:in `new'
    from ./lvmsync2:157:in `run_client'
    from ./lvmsync2:96:in `main'
    from ./lvmsync2:259:in `<main>'

I corrected like this:

diff --git a/lib/lvm/vg_config.rb b/lib/lvm/vg_config.rb
index dac4edf..85aa35d 100644
--- a/lib/lvm/vg_config.rb
+++ b/lib/lvm/vg_config.rb
@@ -57,15 +57,15 @@ class LVM::VGConfig
                                cmd = "#{@vgcfgbackup_cmd} -f #{tmpf.path} #{@vg_name}"
                                stdout = nil
                                stderr = nil
-                               Open3.popen3(cmd) do |stdin_fd, stdout_fd, stderr_fd|
+                               Open3.popen3(cmd) do |stdin_fd, stdout_fd, stderr_fd, wait_thr|
                                        stdin_fd.close
                                        stdout = stdout_fd.read
                                        stderr = stderr_fd.read
-                               end

-                               if $?.exitstatus != 0
-                                       raise RuntimeError,
-                                             "Failed to run vgcfgbackup: #{stdout}\n#{stderr}"
+          if !wait_thr.value.success?
+            raise RuntimeError,
+                  "Failed to run vgcfgbackup: #{stdout}\n#{stderr}"
+          end
                                end

                                out = File.read(tmpf.path)

Then I had problems with logical volumes including dash:

$ lvmsync --stdout /dev/vg/vm-disk-snap
vg_config.rb:25:in `initialize': Cannot parse vgcfgbackup output: Expected one of [a-z0-9\_],  = , [\s] at line 95, column 5 (byte 1825) after peterpan { (RuntimeError)

Change lib/vgcfgbackup.treetop syntax:

diff --git a/lib/vgcfgbackup.treetop b/lib/vgcfgbackup.treetop
index 8e7a22b..9ca0b8e 100644
--- a/lib/vgcfgbackup.treetop
+++ b/lib/vgcfgbackup.treetop
@@ -11,9 +11,9 @@ grammar VgCfgBackup
                [a-z0-9_]+ <VariableName>
        end

-       rule group
-               variable_name space "{" (variable / group / space / comment)+ "}" <Group>
-       end
+    rule group
+        [a-z0-9\_\-]+ space "{" (variable / group / space / comment)+ "}" <Group>
+    end

        rule integer
                [1-9] [0-9]* <Integer> / "0" <Integer>

And with device mapper:

diff --git a/lib/lvm/snapshot.rb b/lib/lvm/snapshot.rb
index 6189e66..07b3f4f 100644
--- a/lib/lvm/snapshot.rb
+++ b/lib/lvm/snapshot.rb
@@ -108,6 +108,10 @@ class LVM::Snapshot
        end

        def metadata_device
-               "/dev/mapper/#{@vg}-#{@lv}-cow"
+    if @lv =~ /-/
+      "/dev/mapper/#{@vg}-#{@lv.gsub(/-/, '--')}-cow"
+    else
+      "/dev/mapper/#{@vg}-#{@lv}-cow"
+    end
        end
 end

After some tests it seems to work.

Vincent

dassams commented 10 years ago

I can confirm the problem with the undefined method `exitstatus'.

Yet the patch doesn't work for me either:

/var/lib/gems/1.9.1/gems/lvmsync-1.0.1/lib/lvm/vg_config.rb:66:in block in vgcfgbackup_output': undefined local variable or methodwait_thr' for #LVM::VGConfig:0x00000000613780 (NameError) from /usr/lib/ruby/1.9.1/tempfile.rb:320:in open' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/lib/lvm/vg_config.rb:56:invgcfgbackup_output' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/lib/lvm/vg_config.rb:20:in initialize' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/bin/lvmsync:154:innew' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/bin/lvmsync:154:in run_client' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/bin/lvmsync:93:inmain' from /var/lib/gems/1.9.1/gems/lvmsync-1.0.1/bin/lvmsync:256:in <top (required)>' from /usr/local/bin/lvmsync:23:inload' from /usr/local/bin/lvmsync:23:in `

'

rralcala commented 10 years ago

The following diff worked for me in: root@:/home/admin/lvmsync# lsb_release -a No LSB modules are available. Distributor ID: Debian Description: Debian GNU/Linux 7.4 (wheezy) Release: 7.4 Codename: wheezy root@:/home/admin/lvmsync# ruby -v ruby 1.9.3p194 (2012-04-20 revision 35410) [x86_64-linux]

Even using - in the vg name as well, is basically the same diff that vincentlauria posted but with dashes on vg and cleaner diff output.

diff --git lib/lvm/snapshot.rb lib/lvm/snapshot.rb
index 6189e66..72e3f35 100644
--- lib/lvm/snapshot.rb
+++ lib/lvm/snapshot.rb
@@ -108,6 +108,6 @@ class LVM::Snapshot
    end

    def metadata_device
-       "/dev/mapper/#{@vg}-#{@lv}-cow"
+       "/dev/mapper/#{@vg.gsub(/-/, '--')}-#{@lv.gsub(/-/, '--')}-cow"
    end
 end
diff --git lib/lvm/vg_config.rb lib/lvm/vg_config.rb
index dac4edf..21e08b3 100644
--- lib/lvm/vg_config.rb
+++ lib/lvm/vg_config.rb
@@ -54,16 +54,18 @@ class LVM::VGConfig
            out = nil

            Tempfile.open('vg_config') do |tmpf|
+               exit_status = nil
                cmd = "#{@vgcfgbackup_cmd} -f #{tmpf.path} #{@vg_name}"
                stdout = nil
                stderr = nil
-               Open3.popen3(cmd) do |stdin_fd, stdout_fd, stderr_fd|
+               Open3.popen3(cmd) do |stdin_fd, stdout_fd, stderr_fd, wait_thr|
                    stdin_fd.close
                    stdout = stdout_fd.read
                    stderr = stderr_fd.read
+                   exit_status = wait_thr.value.exitstatus
                end

-               if $?.exitstatus != 0
+               if exit_status != 0
                    raise RuntimeError,
                          "Failed to run vgcfgbackup: #{stdout}\n#{stderr}"
                end
diff --git lib/vgcfgbackup.treetop lib/vgcfgbackup.treetop
index 8e7a22b..f076af4 100644
--- lib/vgcfgbackup.treetop
+++ lib/vgcfgbackup.treetop
@@ -8,7 +8,7 @@ grammar VgCfgBackup
    end

    rule variable_name
-       [a-z0-9_]+ <VariableName>
+       [-a-z0-9_]+ <VariableName>
    end

    rule group
mpalmer commented 10 years ago

Both the exitstatus and hyphen parsing problems were fixed in my tree a while ago, but I don't think I pushed them up. The v1.0.2 tag should fix these issues.