[pve-devel] [PATCH qemu-server v4 3/3] Add new qm option 'diskimport' to import external disk images

[Fabian Grünbichler]

On Tue, May 09, 2017 at 04:26:06PM +0200, Emmanuel Kasper wrote:
> The VM must be already existing, and the synthax is

s/synthax/syntax/

other comments inline

> 
> qm diskimport 421 minix204.img pve4tank
> 
> where 421 is an already existing VM
> ---
>  PVE/CLI/qm.pm | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 44439dd..ea3a0d5 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -17,6 +17,7 @@ use PVE::SafeSyslog;
>  use PVE::INotify;
>  use PVE::RPCEnvironment;
>  use PVE::QemuServer;
> +use PVE::QemuServer::ImportDisk;
>  use PVE::API2::Qemu;
>  use JSON;
>  use PVE::JSONSchema qw(get_standard_option);
> @@ -380,6 +381,48 @@ __PACKAGE__->register_method ({
>      }});
>  
>  __PACKAGE__->register_method ({
> +    name => 'importdisk',
> +    path => 'importdisk',
> +    method => 'POST',
> +    description => "Import an external disk image as an unused disk in a VM. The
> + image format has to be supported by qemu-img(1).",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    vmid => get_standard_option('pve-vmid', {completion => \&PVE::QemuServer::complete_vmid}),
> +	    source => {
> +		description => 'Path to the disk image to import',
> +		type => 'string',
> +		optional => 0,
> +	    },
> +	    storage => {
> +		description => 'Target storage ID',
> +		type => 'string',

missing format

> +		completion => \&PVE::Storage::complete_storage_enabled,
> +		optional => 0,
> +	    },
> +	    format => {
> +		type => 'string',
> +		description => 'Target format',
> +		enum => [ 'raw', 'qcow2', 'vmdk' ],

I wonder why we allow vmdk here, but not when allocating an empty disk
(where we explictly don't have it in the enum for the optional format
parameter, but when guessing from the name we allow it!??)

the same applies for the clone_vm and move_disk API paths as well ;)

> +		optional => 1,
> +	    },
> +	},
> +    },
> +    returns => { type => 'null'},
> +    code => sub {
> +	my ($params) = @_;
> +
> +	# check if VM exists
> +	PVE::QemuConfig->load_config($params->{vmid});

I would add some more checks here.. like is the path which we want to
import valid and does it exist? does the storage exist? is the VM
locked?

> +
> +	PVE::QemuServer::ImportDisk::do_import($params->{source}, $params->{vmid},
> +	    $params->{storage}, { format => $params->{format} });
> +
> +	return undef;
> +    }});
> +
> +__PACKAGE__->register_method ({
>      name => 'terminal',
>      path => 'terminal',
>      method => 'POST',
> @@ -587,6 +630,8 @@ our $cmddef = {
>      nbdstop => [ __PACKAGE__, 'nbdstop', ['vmid']],
>  
>      terminal => [ __PACKAGE__, 'terminal', ['vmid']],
> +
> +    importdisk => [ __PACKAGE__, 'importdisk', ['vmid', 'source', 'storage']],
>  };
>  
>  1;
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel