[pve-devel] [PATCH 4/7] add targetstorage to vm_start
Wolfgang Bumiller
w.bumiller at proxmox.com
Thu Oct 20 10:04:33 CEST 2016
On Thu, Oct 20, 2016 at 02:35:13AM +0200, Alexandre Derumier wrote:
> This will create a new drive for each local drive found,
> and start the vm with this new drives.
>
> if targetstorage == 'auto', we use same sid than original vm disk
Maybe don't use 'pve-storage-id' for targetstorage in the schema and
use a '1' instead of 'auto', since 'auto' is a valid storage name while
'1' is not.
>
> a nbd server is started in qemu and expose local volumes to network port
>
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
> PVE/API2/Qemu.pm | 14 ++++++++++-
> PVE/QemuServer.pm | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 83 insertions(+), 4 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 62055d1..69ad938 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1627,6 +1627,10 @@ __PACKAGE__->register_method({
> stateuri => get_standard_option('pve-qm-stateuri'),
> migratedfrom => get_standard_option('pve-node',{ optional => 1 }),
> machine => get_standard_option('pve-qm-machine'),
> + targetstorage => get_standard_option('pve-storage-id', {
> + description => "Target storage.",
> + optional => 1,
> + }),
> },
> },
> returns => {
> @@ -1657,6 +1661,14 @@ __PACKAGE__->register_method({
> raise_param_exc({ migratedfrom => "Only root may use this option." })
> if $migratedfrom && $authuser ne 'root at pam';
>
> + my $targetstorage = extract_param($param, 'targetstorage');
> + raise_param_exc({ migratedfrom => "Only root may use this option." })
s/migratedfrom/targetstorage/
> + if $targetstorage && $authuser ne 'root at pam';
I'll try to remember that we should add permission checks here later
since it should be enough if the user has access to the target storage
I think?
> +
> +
> + raise_param_exc({ targetstorage => "targetstorage can only by used with migratedfrom." })
> + if $targetstorage && !$migratedfrom;
> +
> # read spice ticket from STDIN
> my $spice_ticket;
> if ($stateuri && ($stateuri eq 'tcp') && $migratedfrom && ($rpcenv->{type} eq 'cli')) {
> @@ -1697,7 +1709,7 @@ __PACKAGE__->register_method({
> syslog('info', "start VM $vmid: $upid\n");
>
> PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, $skiplock, $migratedfrom, undef,
> - $machine, $spice_ticket);
> + $machine, $spice_ticket, $targetstorage);
>
> return;
> };
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a5aa4c7..c942838 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4429,7 +4429,7 @@ sub vmconfig_update_disk {
>
> sub vm_start {
> my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused,
> - $forcemachine, $spice_ticket) = @_;
> + $forcemachine, $spice_ticket, $targetstorage) = @_;
>
> PVE::QemuConfig->lock_config($vmid, sub {
> my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
> @@ -4450,6 +4450,55 @@ sub vm_start {
> # set environment variable useful inside network script
> $ENV{PVE_MIGRATED_FROM} = $migratedfrom if $migratedfrom;
>
> + my $local_volumes = {};
> +
> + if($targetstorage){
Spaces before ( and after ) please
> +
> + foreach_drive($conf, sub {
> + my ($ds, $drive) = @_;
> +
> + return if drive_is_cdrom($drive);
> +
> + my $volid = $drive->{file};
> +
> + return if !$volid;
This is indented with spaces only instead of tabs like below.
> +
> + my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
> +
> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> + next if $scfg->{shared};
s/next/return/ since this is a sub {} otherwise we'll get warnings.
> + $local_volumes->{$ds} = $volid;
You could store [$volid, $storeid, $volname] here and reuse it below
to avoid having to call parse_volume_id() twice for each $volid.
> + });
> +
> + my $format = undef;
> +
> + foreach my $opt (sort keys %$local_volumes) {
> +
> + my $volid = $local_volumes->{$opt};
This would become:
my ($volid, $storeid, $volname) = @{$local_volumes->{$opt}};
> + my $drive = parse_drive($opt, $conf->{$opt});
> +
> + my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
And this line above could be dropped
> +
> + #if remote storage is specified, use default format
> + if ($targetstorage && $targetstorage ne 'auto') {
> + $storeid = $targetstorage;
> + my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
> + $format = $defFormat;
> + } else {
> + #else we use same format than original
> + $format = $drive->{format};
> + }
> +
> + my $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, undef, ($drive->{size}/1024));
> + my $newdrive = $drive;
> + $newdrive->{format} = $format;
> + $newdrive->{file} = $newvolid;
> + $local_volumes->{$opt} = PVE::QemuServer::print_drive($vmid, $newdrive);
> + #pass drive to conf for command line
> + $conf->{$opt} = PVE::QemuServer::print_drive($vmid, $newdrive);
There's no need to call print_drive() twice in a row (use a variable).
> + }
> + }
> +
> my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
>
> my $migrate_port = 0;
> @@ -4571,6 +4620,24 @@ sub vm_start {
> warn $@ if $@;
> }
>
> + #start nbd server for storage migration
> + if ($targetstorage) {
> + my $nodename = PVE::INotify::nodename();
> + my $localip = PVE::Cluster::remote_node_ip($nodename, 1);
> + $localip = "[$localip]" if Net::IP::ip_is_ipv6($localip);
> + my $pfamily = PVE::Tools::get_host_address_family($nodename);
> + $migrate_port = PVE::Tools::next_migrate_port($pfamily);
> + #fixme
> +# vm_mon_cmd_nocheck($vmid, "nbd-server-start", addr => { inet => { host => "${localip}", port => "${migrate_port}" } } );
What's the issue with this? (Maybe add more info to the fixme comment)
> + vm_human_monitor_command($vmid, "nbd_server_start ${localip}:${migrate_port}", 1);
> + foreach my $opt (sort keys %$local_volumes) {
> + my $volid = $local_volumes->{$opt};
> + vm_mon_cmd_nocheck($vmid, "nbd-server-add", device => "drive-$opt", writable => JSON::true );
While this seems to be working?
> + my $migrate_storage_uri = "nbd:${localip}:${migrate_port}:exportname=drive-$opt";
> + print "storage migration listens on $migrate_storage_uri volume:$volid\n";
> + }
> + }
> +
> if ($migratedfrom) {
>
> eval {
> @@ -4657,7 +4724,7 @@ sub vm_qmp_command {
> }
>
> sub vm_human_monitor_command {
> - my ($vmid, $cmdline) = @_;
> + my ($vmid, $cmdline, $nocheck) = @_;
>
> my $res;
>
> @@ -4666,7 +4733,7 @@ sub vm_human_monitor_command {
> arguments => { 'command-line' => $cmdline},
> };
>
> - return vm_qmp_command($vmid, $cmd);
> + return vm_qmp_command($vmid, $cmd, $nocheck);
> }
>
> sub vm_commandline {
> --
> 2.1.4
More information about the pve-devel
mailing list