search

aruhier / virt-backup

Backup your kvm guests managed by libvirt
Other
106 stars 23 forks source link

Cannot backup VM when using "xz" compression with a custom compresslevel #24

Closed sanminaben closed 5 years ago

sanminaben commented 5 years ago

When trying to backup a VM using the following config.yml options:

compression: xz
compression_lvl: 6

virt-backup fails with the following error message:

$ virt-backup -d backup
INFO:virt_backup:artifactory: Backup started
ERROR:virt_backup:Error with domain artifactory: __init__() got an unexpected keyword argument 'compresslevel'
ERROR:virt_backup:__init__() got an unexpected keyword argument 'compresslevel'
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/virt_backup/groups/pending.py", line 234, in start_multithread
    completed_backups[dom_name] = f.result()
  File "/usr/lib/python3.5/concurrent/futures/_base.py", line 398, in result
    return self.__get_result()
  File "/usr/lib/python3.5/concurrent/futures/_base.py", line 357, in __get_result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.5/dist-packages/virt_backup/groups/pending.py", line 268, in _start_backup
    return backup.start()
  File "/usr/local/lib/python3.5/dist-packages/virt_backup/backups/pending.py", line 165, in start
    else self.get_new_tar(self.target_dir, snapshot_date)
  File "/usr/local/lib/python3.5/dist-packages/virt_backup/backups/pending.py", line 255, in get_new_tar
    return tarfile.open(complete_path, mode, **extra_args)
  File "/usr/lib/python3.5/tarfile.py", line 1575, in open
    return func(name, filemode, fileobj, **kwargs)
  File "/usr/lib/python3.5/tarfile.py", line 1686, in xzopen
    t = cls.taropen(name, mode, fileobj, **kwargs)
  File "/usr/lib/python3.5/tarfile.py", line 1605, in taropen
    return cls(name, mode, fileobj, **kwargs)
TypeError: __init__() got an unexpected keyword argument 'compresslevel'
ERROR:virt_backup:backups failed for domains: artifactory

After looking through the code, it seems that the offending line is here:

in virt-backup/backups/pending.py:

         if self.compression not in (None, "tar"):
             mode = "w:{}".format(self.compression)
             extension = "tar.{}".format(self.compression)
             if self.compression is "xz":
                 extra_args["preset"] = self.compression_lvl
             else:
                 extra_args["compresslevel"] = self.compression_lvl

The problem is that if self.compression is "xz" can evaluate as False even if self.compression is a type 'str' with a value 'xz'. The "is" operator evaluates that two objects are the same (same instance, i.e. same id(object) value), and while this will always work with "None" (there is only one None object exists in the interpreter), it will not work for other types by intermittence. Worse, because the interpreter tends to reuse the same objects for identical immutable int or str values, the "is" operator will sometimes pass, sometimes fail. Replacing the "is" operator with "==" solves the problem.

aruhier commented 5 years ago

Hi, Thanks a lot for the bugreport and the fix for this bug in PR #25. You're totally right, I don't know why I used is here. It was already fixed in dev as I've rewritten a part of this code in the packagers, but I will do a quick release to avoid waiting for these changes to be merged.

sanminaben commented 5 years ago

Hi, no problem at all, I had not realized you had a dev branch, checked it out now. Thanks for merging my pull request!