Fix issue with missing "size=XXG" in properties. Bump to 0.0.7.

CHANGES.rst

@@ -1,6 +1,15 @@
 Changes
 -------
 
+* **v0.0.7** - 2016-10-07
+
+  Bugs fixed:
+
+  - Instead of trusting on the "size=XXG" which may or may not be
+    present in the storage volume config, it reads the QCOW header or
+    ZFS volume size directly. Also checks that the values are available
+    before attempting a move.
+
 * **v0.0.6** - 2016-09-21
 
   New features:

README.rst

@@ -54,8 +54,8 @@ When configured, you can do something like this:
     $ proxmove banana-cluster the-new-cluster node2 node2-ssd the-vm-to-move
     12:12:27: Attempt moving banana-cluster<e1400248> => the-new-cluster<6669ad2c> (node 'node2'): the-vm-to-move
     12:12:27: - source VM the-vm-to-move@node1<qemu/565/running>
-    12:12:27: - storage 'ide2': None,media=cdrom (blobsize=None)
-    12:12:27: - storage 'virtio0': sharedsan:565/vm-565-disk-1.qcow2,format=qcow2,iops_rd=4000,iops_wr=500,size=50G (blobsize=50GiB)
+    12:12:27: - storage 'ide2': None,media=cdrom (host=<unknown>, guest=<unknown>)
+    12:12:27: - storage 'virtio0': sharedsan:565/vm-565-disk-1.qcow2,format=qcow2,iops_rd=4000,iops_wr=500,size=50G (host=37.7GiB, guest=50.0GiB)
     12:12:27: Creating new VM 'the-vm-to-move' on 'the-new-cluster', node 'node2'
     12:12:27: - created new VM 'the-vm-to-move--CREATING' as UPID:node2:00005977:1F4D78F4:57C55C0B:qmcreate:126:user@pve:; waiting for it to show up
     12:12:34: - created new VM 'the-vm-to-move--CREATING': the-vm-to-move--CREATING@node2<qemu/126/stopped>

proxmove

@@ -30,7 +30,7 @@ from urllib.parse import urlparse
 __author__ = 'Walter Doekes'
 __copyright__ = 'Copyright (C) Walter Doekes, OSSO B.V. 2016'
 __licence__ = 'GPLv3+'
-__version__ = '0.0.6'
+__version__ = '0.0.7'
 
 # TODO: see NotImplementedErrors
 # TODO: split up into modules (storage, etc.)
@@ -454,7 +454,7 @@ class ProxmoxStorage(object):
                 'temp dir {!r} of storage {!r} does not exist; '
                 'please create it!'.format(self.temp, self.name))
 
-    def copy(self, data_size, disk_size, src_location, src_format,
+    def copy(self, image_size, disk_size, src_location, src_format,
              dst_storage, dst_id, dst_name):
         dst_temp = self.copy_to_temp(src_location, dst_storage, dst_name)
 
@@ -465,7 +465,7 @@ class ProxmoxStorage(object):
         else:
             assert not src_format, src_format
             new_path, new_format = self.copy_direct(
-                data_size, src_location, dst_storage, dst_name)
+                image_size, src_location, dst_storage, dst_name)
 
         return new_path, new_format
 
@@ -476,7 +476,7 @@ class ProxmoxStorage(object):
                        dst_name):
         raise NotImplementedError('subclasses need to implemented this')
 
-    def copy_direct(self, data_size, src_location, dst_storage, dst_name):
+    def copy_direct(self, image_size, src_location, dst_storage, dst_name):
         raise NotImplementedError('subclasses need to implemented this')
 
     def run_command(self, command, hide_stderr=False, tty=False):
@@ -592,6 +592,50 @@ class ProxmoxStoragePlain(ProxmoxStorage):
         exact_size = data.split()[4].decode('ascii', 'replace')
         return int(exact_size)
 
+    def get_disk_size(self, image_location):
+        path = os.path.join(self.path, image_location)
+        # Extract the filesystem size from the qcow/qcow2 image.
+        # For other formats we have no support yet.
+        if not path.endswith(('.qcow', '.qcow2')):
+            # Non-qcow2 image not supported at this time...
+            return None
+
+        try:
+            data = self.ssh_command(
+                ['hd', path, '|', 'head', '-n2'], hide_stderr=True)
+        except subprocess.CalledProcessError:
+            return None
+
+        # Extract qcow2 data. Example:
+        # 00000000  51 46 49 fb 00 00 00 03  00 00 00 00 00 00 00 00  |...
+        # 00000010  00 00 00 00 00 00 00 10  00 00 00 05 00 00 00 00  |...
+        hexdata = (
+            b''.join([i[10:58] for i in data.split(b'\n')])
+            .decode('ascii', 'replace').replace(' ', ''))
+        try:
+            # uint32_t magic; /* 'Q', 'F', 'I' followed by 0xfb. */
+            if int(hexdata[0:8], 16) != 0x514649fb:
+                raise ValueError('magic', hexdata[0:8])
+            # uint32_t version;
+            if int(hexdata[8:16], 16) > 3:  # up to version 3 is known
+                raise ValueError('version', hexdata[8:16])
+            # uint64_t backing_file_offset;
+            if int(hexdata[16:32], 16) != 0:
+                raise ValueError('backing_file_offset', hexdata[16:32])
+            # uint32_t backing_file_size;
+            if int(hexdata[32:40], 16) != 0:
+                raise ValueError('backing_file_size', hexdata[32:40])
+            # VERSION1: uint32_t mtime;
+            # VERSION2: uint32_t cluster_bits;
+            # uint64_t size; /* in bytes */
+            size = int(hexdata[48:64], 16)
+        except ValueError as e:
+            raise ValueError(
+                'bad QCOW{{,2}} header ({} = {}), path: {}, data: {}'.format(
+                    e.args[0], e.args[1], path, hexdata))
+
+        return size
+
     def check_prerequisite_commands(self):
         if not self.has_commands(['ssh', 'scp']):
             raise PrepareError(
@@ -680,6 +724,16 @@ class ProxmoxStorageZfs(ProxmoxStorage):
         # see the transfer go over 100%.
         return int(human_size_scan(human_size) * 1.02)
 
+    def get_disk_size(self, image_location):
+        pool = '{}/{}'.format(self.path, image_location)
+        data = self.ssh_command(
+            ['zfs', 'list', '-H', '-o', 'volsize', pool], hide_stderr=True)
+        number = data.decode('ascii', 'replace').strip()
+        if not number:
+            return None
+
+        return int(human_size_scan(number))  # hope this is accurate
+
     def copy_to_temp(self, src_location, dst_storage, dst_name):
         if isinstance(dst_storage, ProxmoxStorageZfs):
             # ZFS->ZFS copying requires no temp files.
@@ -691,6 +745,7 @@ class ProxmoxStorageZfs(ProxmoxStorage):
 
     def copy_from_temp(self, disk_size, src_temp, src_format, dst_id,
                        dst_name):
+        assert disk_size, disk_size
         dst_zfs = '{}/{}'.format(self.path, dst_name)
         self.ssh_command(['zfs', 'create', '-V', disk_size, dst_zfs])
 
@@ -717,12 +772,12 @@ class ProxmoxStorageZfs(ProxmoxStorage):
         # ZFS pool known to belong to dst_storage).
         return dst_name, None
 
-    def copy_direct(self, data_size, src_location, dst_storage, dst_name):
+    def copy_direct(self, image_size, src_location, dst_storage, dst_name):
         src_zfs = '{}/{}@proxmove-{}'.format(
             self.path, src_location, datetime.now().strftime('%y%m%d-%H%M%S'))
         dst_zfs = '{}/{}'.format(dst_storage.path, dst_name)
         log.info('zfs(1) send/recv {} data from {!r} to {!r} (on {})'.format(
-            human_size_fmt(data_size), src_zfs, dst_zfs, dst_storage))
+            human_size_fmt(image_size), src_zfs, dst_zfs, dst_storage))
 
         self.ssh_command(['zfs', 'snapshot', src_zfs])
 
@@ -733,13 +788,13 @@ class ProxmoxStorageZfs(ProxmoxStorage):
 
         # pv shows a nice progress bar. It's optional.
         optional_pv_pipe = ''
-        if dst_storage.has_commands(['pv']) and data_size:
+        if dst_storage.has_commands(['pv']):
             log.debug(
                 "Using pv(1) for progress bar because it's available")
             optional_pv_pipe = (
                 # --fineta does not exist on all versions..
                 # --force is required to make it display anything..
-                'pv --force --eta --progress -s {} | '.format(data_size))
+                'pv --force --eta --progress -s {} | '.format(image_size))
 
         # Older mbuffer(1) [v20140310-3] on the receiving end
         # may say: "mbuffer: warning: No controlling terminal
@@ -985,16 +1040,33 @@ class ProxmoxVolume(object):
 
         return ','.join(ret)
 
-    def get_size(self):
-        if not hasattr(self, '_get_size'):
+    def get_size(self, observer):
+        """
+        Get size of volume, either 'guest' size (filesystem size,
+        visible to the guest OS) or 'host' size (image/blob size,
+        observed when transferring data between storage nodes).
+        """
+        # filesystem size vs image/blob size
+        assert observer in ('host', 'guest'), observer
+        cache_key = '_get_size__{}'.format(observer)
+
+        if not hasattr(self, cache_key):
             if self.storage:
-                self._get_size = self.storage.get_image_size(self.location)
+                if observer == 'host':
+                    setattr(self, cache_key, self.storage.get_image_size(
+                        self.location))
+                elif observer == 'guest':
+                    setattr(self, cache_key, self.storage.get_disk_size(
+                        self.location))
+                else:
+                    raise NotImplementedError(observer)
             else:
-                self._get_size = None
-        return self._get_size
+                setattr(self, cache_key, None)
+
+        return getattr(self, cache_key)
 
-    def get_human_size(self):
-        size = self.get_size()
+    def get_human_size(self, observer):
+        size = self.get_size(observer)
         if not size:
             return '<unknown>'
         return human_size_fmt(size)
@@ -1006,7 +1078,7 @@ class ProxmoxVolume(object):
         # Create new name and let the storage backend do the copying.
         new_name = 'vm-{}-{}'.format(new_vmid, new_key)
         new_path, new_format = self.storage.copy(
-            self.get_size(), self.get_property('size'),
+            self.get_size('host'), self.get_size('guest'),
             self.location, self.get_format(),
             new_storage, new_vmid, new_name)
 
@@ -1074,7 +1146,7 @@ class VmMover(object):
     def prepare(self):
         log.debug('Sanity checks and preparation')
         self.prepare_vm_list()
-        self.prepare_vm_checks()
+        self.prepare_vm_config()
         self.prepare_storage_list()
         self.prepare_storage_prerequisites()
         self.prepare_storage_settings()
@@ -1116,12 +1188,31 @@ class VmMover(object):
                         'VM {!r} exists on destination already'.format(
                             vm_name_with_suffix))
 
-    def prepare_vm_checks(self):
-        log.debug('Checking for pending changes in {} VMs to move'.format(
+    def prepare_vm_config(self):
+        log.debug('Checking for troublesome config in {} VMs to move'.format(
             len(self.vms_to_move)))
 
         for vm in self.vms_to_move:
+            # Checks whether the internal config is okay.
             vm.check_config()
+            # Check whether volume sizes can be determined. We may need
+            # both host and guest size.
+            self.prepare_vm_config_volumes(vm)
+
+    def prepare_vm_config_volumes(self, vm):
+        for volume in vm.get_volumes().values():
+            if volume.get_size('host') is None:
+                # If we have no size of the host, then we most
+                # definately won't have any size of the guest
+                pass
+
+            elif volume.get_size('guest') is None:
+                # This could become a problem: if we don't know the disk
+                # size, we can probably not convert from QCOW2 to ZFS
+                # safely.
+                raise PrepareError(
+                    'unknown guest disk-size of volume {} on VM {}'.format(
+                        volume, vm))
 
     def prepare_storage_list(self):
         storages = set([self.dst_storage])
@@ -1182,8 +1273,9 @@ class VmMover(object):
         for key, volume in src_vm.get_volumes().items():
             # TODO: Check whether volume is accessible? Must be combined
             # with the is_removable() and skip_disks options below.
-            log.info('- storage {!r}: {} (blobsize={})'.format(
-                key, volume, volume.get_human_size()))
+            log.info('- storage {!r}: {} (host={}, guest={})'.format(
+                key, volume, volume.get_human_size('host'),
+                volume.get_human_size('guest')))
 
         if not dry_run:
             # Translate config, create new VM.