On 11/21/19 4:46 PM, Fabian Grünbichler wrote:
On November 4, 2019 11:23 am, Fabian Ebner wrote:
On 10/31/19 10:19 AM, Thomas Lamprecht wrote:
On 10/30/19 10:54 AM, Fabian Ebner wrote:
Doing an online migration with --targetstorage and two unused disks with the
same name on different storages failed, because they would collide on the
target storage. This patch makes sure that we don't use the same name twice.
Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---
PVE/QemuMigrate.pm | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 045f3b0..94c5764 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -10,6 +10,7 @@ use PVE::INotify;
use PVE::Tools;
use PVE::Cluster;
use PVE::Storage;
+use PVE::Storage::Plugin;
use PVE::QemuServer;
use Time::HiRes qw( usleep );
use PVE::RPCEnvironment;
@@ -466,9 +467,12 @@ sub sync_disks {
next if $rep_volumes->{$volid};
push @{$self->{volumes}}, $volid;
+ my $targetvolname = undef;
if (defined($override_targetsid) && $self->{running} && $ref eq
'storage') {
- my (undef, $targetvolname) =
PVE::Storage::parse_volume_id($volid);
+ my $scfg = PVE::Storage::storage_config($self->{storecfg},
$targetsid);
+ $targetvolname =
PVE::Storage::Plugin::get_next_vm_diskname($self->{online_unused_volumes},
$targetsid, $vmid, undef, $scfg, 0);
push @{$self->{online_unused_volumes}},
"${targetsid}:${targetvolname}";
+ $self->log('info', "$volid will become
${targetsid}:${targetvolname} on the target node");
but isn't above racy? I only can assume that the new volid is
OK once I alloced the image on the target storage, or?
Or is it OK as we're protected by the migrate lock anyway?
Yes, we are protected by the migrate lock.
I assume that the target storage does not currently contain any disks
with our VM ID. Using get_next_vm_diskname doesn't look at which disks
are present on the storage, but uses the list we pass along to determine
the next free disk name. If there is a problem with storage_migrate we
die anyways, so we can be sure that our online_unused_volumes list is
consistent with the disks that land on the storage.
that's a pretty big assumption though? e.g., there might be orphaned
disks, or replication going on on that target storage?
The code as it is now makes that assumption as well. If a disk with the
same name is already present on the target storage, then storage_migrate
dies.
IMHO, the right approach would be to teach 'storage_migrate' to allocate
the disk (on storage X, with size Y, with prefered format Z), and return the
allocated volid so that we can put it into the (new) config as unused
volume. maybe we need a new helper/wrapper around storage_migrate for
that?
Aaron (CC-ed) is trying to implement similar functionality for
pve-container atm, but there we have the additional complication that we
also have to differentiate between block/image based volumes, and
subvols.. it would probably be a good idea to discuss the needed
interfaces/changes ;)
Sure, sounds good to me.
}
my $opts = $self->{opts};
@@ -480,7 +484,7 @@ sub sync_disks {
$bwlimit = $bwlimit * 1024 if defined($bwlimit);
PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info}, $targetsid,
- undef, undef, undef, $bwlimit,
$insecure, $with_snapshots);
+ $targetvolname, undef, undef,
$bwlimit, $insecure, $with_snapshots);
}
}
};
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
