On 3/29/19 8:28 AM, Stoiko Ivanov wrote: > for migrate_vm, clone_vm and move_volume >
some nits, and as for a change an actual real issue at the bottom :-) > Signed-off-by: Stoiko Ivanov <[email protected]> > --- > src/PVE/API2/LXC.pm | 30 +++++++++++++++++++++++++++--- > 1 file changed, 27 insertions(+), 3 deletions(-) > > diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm > index 6de121f..35e6851 100644 > --- a/src/PVE/API2/LXC.pm > +++ b/src/PVE/API2/LXC.pm > @@ -987,6 +987,12 @@ __PACKAGE__->register_method({ > " mounts. NOTE: deprecated, use 'shared' property of mount > point instead.", > optional => 1, > }, > + bwlimit => { > + description => "Override i/o bandwidth limit (in KiB/s).", I/O > + optional => 1, > + type => 'number', > + minimum => '0', > + }, > }, > }, > returns => { > @@ -1250,6 +1256,12 @@ __PACKAGE__->register_method({ > description => "Target node. Only allowed if the original VM is > on shared storage.", > optional => 1, > }), > + bwlimit => { > + description => "Override i/o bandwidth limit (in KiB/s).", I/O also, we always speak about "override a limit", but nowhere we talk about what we override. Maybe this wording could be improved, semi-related to this I'd add a "default" key here, something like: default => 'cluster wide clone limit', or respectively: default => 'cluster wide migrate limit', ..., this could already help with making it clear what this overrides > + optional => 1, > + type => 'number', > + minimum => '0', > + }, > }, > }, > returns => { > @@ -1434,6 +1446,7 @@ __PACKAGE__->register_method({ > local $SIG{HUP} = sub { die "interrupted by signal\n"; }; > > PVE::Storage::activate_volumes($storecfg, $vollist, $snapname); > + my $bwlimit = extract_param($param, 'bwlimit'); > > foreach my $opt (keys %$mountpoints) { > my $mp = $mountpoints->{$opt}; > @@ -1442,8 +1455,10 @@ __PACKAGE__->register_method({ > my $newvolid; > if ($fullclone->{$opt}) { > print "create full clone of mountpoint $opt ($volid)\n"; > - my $target_storage = $storage // > PVE::Storage::parse_volume_id($volid); > - $newvolid = PVE::LXC::copy_volume($mp, $newid, > $target_storage, $storecfg, $newconf, $snapname); > + my $source_storage = > PVE::Storage::parse_volume_id($volid); > + my $target_storage = $storage // $source_storage; > + my $clonelimit = > PVE::Storage::get_bandwidth_limit('clone', [$source_storage, > $target_storage], $bwlimit); > + $newvolid = PVE::LXC::copy_volume($mp, $newid, > $target_storage, $storecfg, $newconf, $snapname, $clonelimit); > } else { > print "create linked clone of mount point $opt > ($volid)\n"; > $newvolid = PVE::Storage::vdisk_clone($storecfg, > $volid, $newid, $snapname); > @@ -1690,7 +1705,13 @@ __PACKAGE__->register_method({ > description => 'Prevent changes if current configuration file > has different SHA1 digest. This can be used to prevent concurrent > modifications.', > maxLength => 40, > optional => 1, > - } > + }, > + bwlimit => { > + description => "Override i/o bandwidth limit (in KiB/s).", > + optional => 1, > + type => 'number', > + minimum => '0', > + }, > }, > }, > returns => { > @@ -1747,6 +1768,9 @@ __PACKAGE__->register_method({ > > eval { > PVE::Storage::activate_volumes($storage_cfg, [ $old_volid > ]); > + my $bwlimit = extract_param($param, 'bwlimit'); > + my $source_storage = > PVE::Storage::parse_volume_id($old_volid); > + my $movelimit = PVE::Storage::get_bandwidth_limit('move', > [$source_storage, $storage], $bwlimit); maybe also pass it to copy_volume so that it actually does something ;-) > $new_volid = PVE::LXC::copy_volume($mpdata, $vmid, > $storage, $storage_cfg, $conf); > $mpdata->{volume} = $new_volid; > > _______________________________________________ pve-devel mailing list [email protected] https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
