diff --git a/README.md b/README.md index 089424f..6cb9124 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,9 @@ can help a lot :-) * [Memory balloon](#memory-balloon) * [Libvirt communication channels](#libvirt-communication-channels) * [Custom command line arguments and environment variables](#custom-command-line-arguments-and-environment-variables) -* [Box Format](#box-format) +* [Box Formats](#box-formats) + * [Version 1](#version-1) + * [Version 2 (Experimental)](#version-2-experimental) * [Create Box](#create-box) * [Package Box from VM](#package-box-from-vm) * [Troubleshooting VMs](#troubleshooting-vms) @@ -1774,7 +1776,11 @@ Vagrant.configure("2") do |config| end ``` -## Box Format +## Box Formats + +### Version 1 + +This is the original format that most boxes currently use. You can view an example box in the [`example_box/directory`](https://github.com/vagrant-libvirt/vagrant-libvirt/tree/master/example_box). @@ -1788,6 +1794,45 @@ The box is a tarball containing: * `Vagrantfile` that does default settings for the provider-specific configuration for this provider + +### Version 2 (Experimental) + +Due to the limitation of only being able to handle a single disk with the version 1 format, a new +format was added to support boxes that need to specify multiple disks. This is still currently +experimental and as such support for packaging has yet to be added. There is a script in the tools +folder (tools/create_box_with_two_disks.sh) that should provide a guideline on how to create such +a box for those that wish to experiment and provide early feedback. + +At it's most basic, it expects an array of disks to allow a specific order to be presented. Disks +will be attached in this order and as such assume device names base on this within the VM. The +'path' attribute is required, and is expected to be relative to the base of the box. This should +allow placing the disk images within a nested directory within the box if it useful for those +with a larger number of disks. The name allows overriding the target volume name that will be +used in the libvirt storage pool. Note that vagrant-libvirt will still prefix the volume name +with `#{box_name}_vagrant_box_image_#{box_version}_` to avoid accidental clashes with other boxes. + +Format and virtual size need no longer be specified as they are now retrieved directly from the +provided image using `qemu-img info ...`. + +Example format: +```json +{ + 'disks': [ + { + 'path': 'disk1.img' + }, + { + 'path': 'disk2.img', + 'name': 'secondary_disk' + }, + { + 'path': 'disk3.img' + } + ], + 'provider': 'libvirt' +} +``` + ## Create Box If creating a box from a modified vagrant-libvirt machine, ensure that diff --git a/lib/vagrant-libvirt/action/handle_box_image.rb b/lib/vagrant-libvirt/action/handle_box_image.rb index d26a6c0..8e5fa49 100644 --- a/lib/vagrant-libvirt/action/handle_box_image.rb +++ b/lib/vagrant-libvirt/action/handle_box_image.rb @@ -30,30 +30,40 @@ module VagrantPlugins env[:box_volume_number] = 1 env[:box_volumes] = [{ - :path => HandleBoxImage.get_box_image_path(env, HandleBoxImage.get_default_box_image_path(0)), - :name => HandleBoxImage.get_volume_name(env, 0), + :path => HandleBoxImage.get_box_image_path(env[:machine].box, 'box.img'), + :name => HandleBoxImage.get_volume_name(env[:machine].box, 'box'), :virtual_size => HandleBoxImage.get_virtual_size(env), :format => box_format, }] else # Handle box v2 format # { - # 'path': '', # can be inferred - # 'name': '' + # 'path': '', + # 'name': '' # optional, will use index # } # - env[:box_volume_number] = disks.length() + target_volumes = Hash[] 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)) - ) + raise Errors::BoxFormatMissingAttribute, attribute: "disks[#{i}]['path']" if disks[i]['path'].nil? + + image_path = HandleBoxImage.get_box_image_path(env[:machine].box, disks[i]['path']) format, virtual_size = HandleBoxImage.get_box_disk_settings(image_path) + volume_name = HandleBoxImage.get_volume_name( + env[:machine].box, + disks[i].fetch('name', disks[i]['path'].sub(/#{File.extname(disks[i]['path'])}$/, '')), + ) + + # allowing name means needing to check that it doesn't cause a clash + existing = target_volumes[volume_name] + if !existing.nil? + raise Errors::BoxFormatDuplicateVolume, volume: volume_name, new_disk: "disks[#{i}]", orig_disk: "disks[#{existing}]" + end + target_volumes[volume_name] = i { :path => image_path, - :name => disks[i].fetch('name', HandleBoxImage.get_volume_name(env, i)), + :name => volume_name, :virtual_size => virtual_size.to_i, :format => HandleBoxImage.verify_box_format(format) } @@ -105,15 +115,15 @@ module VagrantPlugins protected - def self.get_volume_name(env, index) - name = env[:machine].box.name.to_s.dup.gsub('/', '-VAGRANTSLASH-') - name << "_vagrant_box_image_#{ - begin - env[:machine].box.version.to_s - rescue - '' - end}_#{index}.img" - return name + def self.get_volume_name(box, name) + vol_name = box.name.to_s.dup.gsub('/', '-VAGRANTSLASH-') + vol_name << "_vagrant_box_image_#{ + begin + box.version.to_s + rescue + '' + end + }_#{name.dup.gsub('/', '-SLASH-')}.img" end def self.get_virtual_size(env) @@ -123,12 +133,8 @@ module VagrantPlugins return box_virtual_size end - def self.get_default_box_image_path(index) - return index <= 0 ? 'box.img' : "box_#{index}.img" - end - - def self.get_box_image_path(env, box_name) - return env[:machine].box.directory.join(box_name).to_s + def self.get_box_image_path(box, box_name) + return box.directory.join(box_name).to_s end def self.verify_box_format(box_format, disk_index=nil) diff --git a/lib/vagrant-libvirt/errors.rb b/lib/vagrant-libvirt/errors.rb index 672f589..7b1f7f1 100644 --- a/lib/vagrant-libvirt/errors.rb +++ b/lib/vagrant-libvirt/errors.rb @@ -37,7 +37,18 @@ module VagrantPlugins error_key(:image_download_error) end - # Box exceptions + # Box exceptions, capture all under one + class BoxError < VagrantLibvirtError + end + + class BoxFormatMissingAttribute < BoxError + error_key(:box_format_missing_attribute) + end + + class BoxFormatDuplicateVolume < BoxError + error_key(:box_format_duplicate_volume) + end + class BadBoxImage < VagrantLibvirtError error_key(:bad_box_image) end diff --git a/locales/en.yml b/locales/en.yml index aa9e8ac..5daa320 100644 --- a/locales/en.yml +++ b/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. + box_format_duplicate_volume: |- + Encountered a duplicate volume name '%{volume}' generated for disk '%{new_disk}', due to already allocated for disk '%{orig_disk}'. + box_format_missing_attribute: |- + Invalid box metadata, missing expected attribute: '%{attribute}' bad_box_image: |- Received error when query the box image details from '%{image}'. Stdout: %{out} diff --git a/spec/unit/action/handle_box_image_spec.rb b/spec/unit/action/handle_box_image_spec.rb index 6e708ee..6a8b2da 100644 --- a/spec/unit/action/handle_box_image_spec.rb +++ b/spec/unit/action/handle_box_image_spec.rb @@ -50,7 +50,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do [ { :path=>"/test/box.img", - :name=>"test_vagrant_box_image_1.1.1_0.img", + :name=>"test_vagrant_box_image_1.1.1_box.img", :virtual_size=>5, :format=>"qcow2" } @@ -69,10 +69,10 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do it 'should upload disk' do expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_0.img in storage pool default.') + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_box.img in storage pool default.') expect(volumes).to receive(:create).with( hash_including( - :name => "test_vagrant_box_image_1.1.1_0.img", + :name => "test_vagrant_box_image_1.1.1_box.img", :allocation => "5120M", :capacity => "5G", ) @@ -107,12 +107,15 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do allow(env[:machine]).to receive_message_chain("box.metadata") { Hash[ 'disks' => [ { - 'name'=>'send_box_name.img', + 'path' => 'box.img', + 'name' => 'send_box_name', }, { 'path' => 'disk.qcow2', }, - { }, + { + 'path' => 'box_2.img', + }, ], ]} allow(env[:machine]).to receive_message_chain("box.directory.join") do |arg| @@ -137,19 +140,19 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do [ { :path=>"/test/box.img", - :name=>"send_box_name.img", + :name=>"test_vagrant_box_image_1.1.1_send_box_name.img", :virtual_size=>5, :format=>"qcow2" }, { :path=>"/test/disk.qcow2", - :name=>"test_vagrant_box_image_1.1.1_1.img", + :name=>"test_vagrant_box_image_1.1.1_disk.img", :virtual_size=>10, :format=>"qcow2" }, { :path=>"/test/box_2.img", - :name=>"test_vagrant_box_image_1.1.1_2.img", + :name=>"test_vagrant_box_image_1.1.1_box_2.img", :virtual_size=>20, :format=>"qcow2" } @@ -168,30 +171,30 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do it 'should upload all 3 disks' do expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume send_box_name.img in storage pool default.') + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_send_box_name.img in storage pool default.') expect(volumes).to receive(:create).with( hash_including( - :name => "send_box_name.img", + :name => "test_vagrant_box_image_1.1.1_send_box_name.img", :allocation => "5120M", :capacity => "5G", ) ) expect(subject).to receive(:upload_image) expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_1.img in storage pool default.') + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_disk.img in storage pool default.') expect(volumes).to receive(:create).with( hash_including( - :name => "test_vagrant_box_image_1.1.1_1.img", + :name => "test_vagrant_box_image_1.1.1_disk.img", :allocation => "10240M", :capacity => "10G", ) ) expect(subject).to receive(:upload_image) expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_2.img in storage pool default.') + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_box_2.img in storage pool default.') expect(volumes).to receive(:create).with( hash_including( - :name => "test_vagrant_box_image_1.1.1_2.img", + :name => "test_vagrant_box_image_1.1.1_box_2.img", :allocation => "20480M", :capacity => "20G", ) @@ -214,12 +217,12 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do it 'upload disks 1 and 2 only' do expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_1.img in storage pool default.') - expect(volumes).to receive(:create).with(hash_including(:name => "test_vagrant_box_image_1.1.1_1.img")) + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_disk.img in storage pool default.') + expect(volumes).to receive(:create).with(hash_including(:name => "test_vagrant_box_image_1.1.1_disk.img")) expect(subject).to receive(:upload_image) expect(ui).to receive(:info).with('Uploading base box image as volume into Libvirt storage...') - expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_2.img in storage pool default.') - expect(volumes).to receive(:create).with(hash_including(:name => "test_vagrant_box_image_1.1.1_2.img")) + expect(logger).to receive(:info).with('Creating volume test_vagrant_box_image_1.1.1_box_2.img in storage pool default.') + expect(volumes).to receive(:create).with(hash_including(:name => "test_vagrant_box_image_1.1.1_box_2.img")) expect(subject).to receive(:upload_image) expect(subject.call(env)).to be_nil @@ -263,7 +266,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do end - context 'when one of a multi disk definition has wrong disk format in metadata.json' do + context 'when invalid format in metadata.json' do let(:status) { double } before do @@ -271,20 +274,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do allow(box_volume).to receive(:id).and_return(1) allow(env[:machine]).to receive_message_chain("box.name") { 'test' } allow(env[:machine]).to receive_message_chain("box.version") { '1.1.1' } - allow(env[:machine]).to receive_message_chain("box.metadata") { - Hash[ - 'disks' => [ - { - 'name'=>'send_box_name.img', - 'format'=> 'wrongFormat' - }, - { - 'path' => 'disk.qcow2', - }, - { }, - ], - ] - } + allow(env[:machine]).to receive_message_chain("box.metadata") { box_metadata } allow(env[:machine]).to receive_message_chain("box.directory.join") do |arg| '/test/'.concat(arg.to_s) end @@ -300,10 +290,74 @@ describe VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage do ]) end - it 'should be ignored' do - expect(subject.call(env)).to be_nil + context 'with one disk having wrong disk format' do + let(:box_metadata) { + Hash[ + 'disks' => [ + { + 'path' => 'box.img', + 'name' =>'send_box_name.img', + 'format' => 'wrongFormat' + }, + { + 'path' => 'disk.qcow2', + }, + { + 'path' => 'box_2.img', + }, + ], + ] + } + + it 'should be ignored' do + expect(subject.call(env)).to be_nil + end + end + + context 'with one disk missing path' do + let(:box_metadata) { + Hash[ + 'disks' => [ + { + 'path' => 'box.img', + }, + { + 'name' => 'send_box_name', + }, + { + 'path' => 'box_2.img', + }, + ], + ] + } + + it 'should raise an exception' do + expect{ subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::BoxFormatMissingAttribute, /: 'disks\[1\]\['path'\]'/) + end + end + + context 'with one disk name duplicating a path of another' do + let(:box_metadata) { + Hash[ + 'disks' => [ + { + 'path' => 'box.img', + 'name' => 'box_2', + }, + { + 'path' => 'disk.qcow2', + }, + { + 'path' => 'box_2.img', + }, + ], + ] + } + + it 'should raise an exception' do + expect{ subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::BoxFormatDuplicateVolume, /test_vagrant_box_image_1.1.1_box_2.img.*'disks\[2\]'.*'disks\[0\]'/) + end end end - end end