Re: [pve-devel] [PATCH v4 manager] gui: Add import wizard for disk & VM

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On March 8, 2021 12:38 pm, Dominic Jäger wrote:
> On Wed, Feb 10, 2021 at 10:49:41AM +0100, Fabian Grünbichler wrote:
>> > 
>> > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
>> > index 8172231e..9bf75ab7 100644
>> > --- a/PVE/API2/Nodes.pm
>> > +++ b/PVE/API2/Nodes.pm
>> > @@ -27,6 +27,7 @@ use PVE::HA::Env::PVE2;
>> >  use PVE::HA::Config;
>> >  use PVE::QemuConfig;
>> >  use PVE::QemuServer;
>> > +use PVE::QemuServer::OVF;
>> >  use PVE::API2::Subscription;
>> >  use PVE::API2::Services;
>> >  use PVE::API2::Network;
>> > @@ -224,6 +225,7 @@ __PACKAGE__->register_method ({
>> >  	    { name => 'subscription' },
>> >  	    { name => 'report' },
>> >  	    { name => 'tasks' },
>> > +	    { name => 'readovf' },
>> >  	    { name => 'rrd' }, # fixme: remove?
>> >  	    { name => 'rrddata' },# fixme: remove?
>> >  	    { name => 'replication' },
>> > @@ -2173,6 +2175,44 @@ __PACKAGE__->register_method ({
>> >  	return undef;
>> >      }});
>> 
>> this API endpoint belongs in qemu-server?
> 
> 
> I am not sure. It looked to me like qemu-server implied
> nodes/<node>/qemu/<vmid>/readovf
> but I wanted to avoid <vmid> and place it next to
> nodes/<node>/status, nodes/<node>/rrd, ...
> which are in this file?

qemu-server also ships API paths besides /nodes/<node>/qemu/*, e.g. 
PVE::API2::Qemu::CPU provides /nodes/<node>/cpu, so we might do a 
/nodes/<node>/ovf (or something similar under /cluster, if it's not at 
all node-dependent).

>> >  
>> > +__PACKAGE__->register_method ({
>> > +    name => 'readovf',
>> > +    path => 'readovf',
>> > +    method => 'GET',
>> > +    proxyto => 'node',
>> > +    description => "Read an .ovf manifest.",
>> > +    parameters => {
>> > +	additionalProperties => 0,
>> > +	properties => {
>> > +	    node => get_standard_option('pve-node'),
>> > +	    manifest => {
>> > +		description => ".ovf manifest",
>> > +		type => 'string',
>> > +	    },
>> > +	},
>> > +    },
>> > +    returns => {
>> > +	description => "VM config according to .ovf manifest and digest of manifest",
>> > +	type => "object",
>> 
>> according to the code below, this has a defined schema?
> 
> Yes. Something like
> 
>  returns => {
> 	type => 'object',
> 	additionalProperties => 0,
> 	properties => {
> 	    name => {
> 		type => 'string',
> 		optional => 1,
> 	    },
> 	    cores => {
> 		type => 'integer',
> 		optional => 1,
> 	    },
> 	    memory => {
> 		type => 'integer',
> 		optional => 1,
> 	    },
> 	    ?? => {
> 		type => 'string',
> 		description => 'path of a disk image',
> 	    },
> 	    ?? => {
> 		type => 'string',
> 		description => 'path of a disk image',
> 	    },
> 	    ....
> 	},
> 
> but the only way I have found so far to return paths
> scsi1 => /some/image.img
> sata3 => /some/other.img
> 
> is PVE::QemuServer::json_config_properties( which allows much more than can be
> read from the OVF at the moment.

you can just have your own helper that uses the existing valid disk 
names helper, takes the static part of the return schema and generates 
the full return schema.

>> 
>> > +    },
>> > +    code => sub {
>> > +	my ($param) = @_;
>> > +
>> > +	my $manifest = $param->{manifest};
>> > +	die "$manifest: non-existent or non-regular file\n" if (! -f $manifest);
>> > +
>> > +	my $parsed = PVE::QemuServer::OVF::parse_ovf($manifest, 0, 1);
>> > +	my $result;
>> > +	$result->{cores} = $parsed->{qm}->{cores};
>> > +	$result->{name} =  $parsed->{qm}->{name};
>> > +	$result->{memory} = $parsed->{qm}->{memory};
>> > +	my $disks = $parsed->{disks};
>> > +	foreach my $disk (@$disks) {
>> > +	    $result->{$disk->{disk_address}} = $disk->{backing_file};
>> > +	}
>> > +	return $result;
>> > +}});
>> > +
>> >  # bash completion helper
>> >  
>> >  sub complete_templet_repo {
>