Retrieve disk format and virtual size directly (#1274)

When uploading disks to libvirt storage it is unnecessary to require
that the virtual size or the format be provided as these can be
retrieved by calling qemu-img on the box files to retrieve the required
information.

Update the handle box image support to separate the handling of the two
different formats and remove the need to specify the additional settings
in the case of the V2 format for multi disk boxes.

lib/vagrant-libvirt/action/handle_box_image.rb

@@ -16,38 +16,49 @@ module VagrantPlugins
         end
 
         def call(env)
-          # Verify box metadata for mandatory values.
+          # Handle box formats converting between v1 => v2 and ensuring
-          #
+          # any obsolete settings are rejected.
-          # Verify disk number
+
           disks = env[:machine].box.metadata.fetch('disks', [])
           if disks.empty?
-            disks.push({
+            # Handle box v1 format
-              'path' => HandleBoxImage.get_default_box_image_path(0),
-              'name' => HandleBoxImage.get_volume_name(env, 0),
-              'virtual_size' => HandleBoxImage.get_virtual_size(env),
-            })
-          end
-          HandleBoxImage.verify_virtual_size_in_disks(disks)
 
-          # Support qcow2 format only for now, but other formats with backing
+            # Only qcow2 format is supported in v1, but other formats with backing
-          # store capability should be usable.
+            # store capability should be usable.
-          box_format = env[:machine].box.metadata['format']
+            box_format = env[:machine].box.metadata['format']
-          HandleBoxImage.verify_box_format(box_format)
+            HandleBoxImage.verify_box_format(box_format)
 
-          env[:box_volume_number] = disks.length()
+            env[:box_volume_number] = 1
-          env[:box_volumes] = Array.new(env[:box_volume_number]) {|i| {
+            env[:box_volumes] = [{
-              :path => HandleBoxImage.get_box_image_path(
+              :path => HandleBoxImage.get_box_image_path(env, HandleBoxImage.get_default_box_image_path(0)),
+              :name => HandleBoxImage.get_volume_name(env, 0),
+              :virtual_size => HandleBoxImage.get_virtual_size(env),
+              :format => box_format,
+            }]
+          else
+            # Handle box v2 format
+            # {
+            #   'path': '<path-of-file-box>', # can be inferred
+            #   'name': '<name-to-use-in-storage>'
+            # }
+            #
+
+            env[:box_volume_number] = disks.length()
+            env[:box_volumes] = Array.new(env[:box_volume_number]) { |i|
+              image_path = HandleBoxImage.get_box_image_path(
                 env,
                 disks[i].fetch('path', HandleBoxImage.get_default_box_image_path(i))
-              ),
-              :name => disks[i].fetch('name', HandleBoxImage.get_volume_name(env, i)),
-              :virtual_size => disks[i]['virtual_size'],
-              :format => HandleBoxImage.verify_box_format(
-                disks[i].fetch('format', box_format),
-                i
               )
+              format, virtual_size = HandleBoxImage.get_box_disk_settings(image_path)
+
+              {
+                :path => image_path,
+                :name => disks[i].fetch('name', HandleBoxImage.get_volume_name(env, i)),
+                :virtual_size => virtual_size.to_i,
+                :format => HandleBoxImage.verify_box_format(format)
+              }
             }
-          }
+          end
 
           # Get config options
           config = env[:machine].provider_config
@@ -134,10 +145,17 @@ module VagrantPlugins
           return box_format
         end
 
-        def self.verify_virtual_size_in_disks(disks)
+        def self.get_box_disk_settings(image_path)
-          disks.each_with_index do |disk, index|
+          stdout, stderr, status = Open3.capture3('qemu-img', 'info', image_path)
-            raise Errors::NoDiskVirtualSizeSet, disk_index:index if disk['virtual_size'].nil?
+          if !status.success?
+            raise Errors::BadBoxImage, image: image_path, out: stdout, err: stderr
           end
+
+          image_info_lines = stdout.split("\n")
+          format = image_info_lines.find { |l| l.start_with?('file format:') }.split(' ')[2]
+          virtual_size = image_info_lines.find { |l| l.start_with?('virtual size:') }.split(' ')[2]
+
+          return format, virtual_size
         end
 
         def send_box_image(env, config, box_image_file, box_volume)

lib/vagrant-libvirt/errors.rb

@@ -38,6 +38,10 @@ module VagrantPlugins
       end
 
       # Box exceptions
+      class BadBoxImage < VagrantLibvirtError
+        error_key(:bad_box_image)
+      end
+
       class NoBoxVolume < VagrantLibvirtError
         error_key(:no_box_volume)
       end

locales/en.yml

@@ -72,6 +72,10 @@ en:
       no_storage_pool: |-
         No usable storage pool found! Please check if storage pool is
         created and available.
+      bad_box_image: |-
+        Received error when query the box image details from '%{image}'.
+        Stdout: %{out}
+        Stderr: %{err}
       no_box_volume: |-
         Volume for box image is missing in storage pools. Try to run vagrant
         again, or check if storage volume is accessible.

spec/unit/action/handle_box_image_spec.rb

@@ -97,6 +97,8 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do
     end
 
     context 'when three disks in metadata.json' do
+      let(:status) { double }
+
       before do
         allow(all).to receive(:first).and_return(box_volume)
         allow(box_volume).to receive(:id).and_return(1)
@@ -106,22 +108,26 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do
           'disks' => [
             {
               'name'=>'send_box_name.img',
-              'virtual_size'=> 5,
             },
             {
               'path' => 'disk.qcow2',
-              'virtual_size'=> 10
-            },
-            {
-              'virtual_size'=> 20,
             },
+            { },
           ],
-          'format' => 'qcow2'
+        ]}
-          ]
-        }
         allow(env[:machine]).to receive_message_chain("box.directory.join") do |arg|
           '/test/'.concat(arg.to_s)
         end
+        allow(status).to receive(:success?).and_return(true)
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/box.img').and_return([
+            "image: /test/box.img\nfile format: qcow2\nvirtual size: 5 GiB (5368709120 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/disk.qcow2').and_return([
+          "image: /test/disk.qcow2\nfile format: qcow2\nvirtual size: 10 GiB (10737418240 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/box_2.img').and_return([
+          "image: /test/box_2.img\nfile format: qcow2\nvirtual size: 20 GiB (21474836480 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
       end
 
       it 'should have three disks in machine env' do
@@ -258,6 +264,8 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do
     end
 
     context 'when one of a multi disk definition has wrong disk format in metadata.json' do
+      let(:status) { double }
+
       before do
         allow(all).to receive(:first).and_return(box_volume)
         allow(box_volume).to receive(:id).and_return(1)
@@ -268,27 +276,32 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do
             'disks' => [
               {
                 'name'=>'send_box_name.img',
-                'virtual_size'=> 5,
                 'format'=> 'wrongFormat'
               },
               {
                 'path' => 'disk.qcow2',
-                'virtual_size'=> 10
-              },
-              {
-                'virtual_size'=> 20,
               },
+              { },
             ],
-            'format' => 'qcow2',
           ]
         }
         allow(env[:machine]).to receive_message_chain("box.directory.join") do |arg|
           '/test/'.concat(arg.to_s)
         end
+        allow(status).to receive(:success?).and_return(true)
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/box.img').and_return([
+            "image: /test/box.img\nfile format: qcow2\nvirtual size: 5 GiB (5368709120 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/disk.qcow2').and_return([
+          "image: /test/disk.qcow2\nfile format: qcow2\nvirtual size: 10 GiB (10737418240 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
+        allow(Open3).to receive(:capture3).with('qemu-img', 'info', '/test/box_2.img').and_return([
+          "image: /test/box_2.img\nfile format: qcow2\nvirtual size: 20 GiB (21474836480 bytes)\ndisk size: 1.45 GiB\n", "", status
+        ])
       end
 
-      it 'should raise WrongDiskFormatSet exception' do
+      it 'should be ignored' do
-        expect { subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::WrongDiskFormatSet)
+        expect(subject.call(env)).to be_nil
       end
     end
 

tools/create_box_with_two_disks.sh

@@ -17,15 +17,12 @@ then
     cat > "${NEW_PATH}/${BOX_VERSION}/libvirt/metadata.json" <<EOF
 {
   "provider": "libvirt",
-  "format": "qcow2",
   "disks" : [
-      {
+    {
-          "virtual_size": 2
+    },
-      },
+    {
-      {
           "path":"disk2.qcow2",
-          "virtual_size":10
+    }
-      }
   ]
 }
 EOF