[pve-devel] [PATCH v5 qemu-server 01/19] Switch to using foreach_volume instead of foreach_drive
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Apr 9 09:53:37 CEST 2020
small comment in-line
On April 8, 2020 11:24 am, Fabian Ebner wrote:
> It was necessary to move foreach_volid back to QemuServer.pm
>
> In VZDump/QemuServer.pm and QemuMigrate.pm the dependency on
> QemuConfig.pm was already there, just the explicit "use" was missing.
>
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> PVE/API2/Qemu.pm | 6 ++--
> PVE/QemuConfig.pm | 2 +-
> PVE/QemuMigrate.pm | 5 +--
> PVE/QemuServer.pm | 70 ++++++++++++++++++++++++++++++-------
> PVE/QemuServer/Cloudinit.pm | 2 +-
> PVE/QemuServer/Drive.pm | 61 --------------------------------
> PVE/VZDump/QemuServer.pm | 3 +-
> 7 files changed, 68 insertions(+), 81 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index b220934..87fbd72 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -63,7 +63,7 @@ my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
> my $check_storage_access = sub {
> my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
>
> - PVE::QemuServer::foreach_drive($settings, sub {
> + PVE::QemuConfig->foreach_volume($settings, sub {
> my ($ds, $drive) = @_;
>
> my $isCDROM = PVE::QemuServer::drive_is_cdrom($drive);
> @@ -96,7 +96,7 @@ my $check_storage_access_clone = sub {
>
> my $sharedvm = 1;
>
> - PVE::QemuServer::foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my $isCDROM = PVE::QemuServer::drive_is_cdrom($drive);
> @@ -216,7 +216,7 @@ my $create_disks = sub {
> }
> };
>
> - eval { PVE::QemuServer::foreach_drive($settings, $code); };
> + eval { PVE::QemuConfig->foreach_volume($settings, $code); };
>
> # free allocated images on error
> if (my $err = $@) {
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index d29b88b..78f5c60 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -66,7 +66,7 @@ sub has_feature {
> my ($class, $feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
>
> my $err;
> - PVE::QemuServer::foreach_drive($conf, sub {
> + $class->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> return if PVE::QemuServer::drive_is_cdrom($drive);
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index e83d60d..2925b90 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -17,6 +17,7 @@ use PVE::ReplicationState;
> use PVE::Storage;
> use PVE::Tools;
>
> +use PVE::QemuConfig;
> use PVE::QemuServer::CPUConfig;
> use PVE::QemuServer::Drive;
> use PVE::QemuServer::Helpers qw(min_version);
> @@ -477,7 +478,7 @@ sub sync_disks {
> }
>
> my $live_replicatable_volumes = {};
> - PVE::QemuServer::foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my $volid = $drive->{file};
> @@ -508,7 +509,7 @@ sub sync_disks {
> # sizes in config have to be accurate for remote node to correctly
> # allocate disks, rescan to be sure
> my $volid_hash = PVE::QemuServer::scan_volids($storecfg, $vmid);
> - PVE::QemuServer::foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($key, $drive) = @_;
> my ($updated, $old_size, $new_size) = PVE::QemuServer::Drive::update_disksize($drive, $volid_hash);
> if (defined($updated)) {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 12f76b8..cd34bf6 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -44,7 +44,7 @@ use PVE::QemuConfig;
> use PVE::QemuServer::Helpers qw(min_version config_aware_timeout);
> use PVE::QemuServer::Cloudinit;
> use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
> -use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive foreach_drive foreach_volid);
> +use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom parse_drive print_drive);
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::Memory;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> @@ -2037,7 +2037,7 @@ sub destroy_vm {
>
> if ($conf->{template}) {
> # check if any base image is still used by a linked clone
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
> return if drive_is_cdrom($drive);
>
> @@ -2051,7 +2051,7 @@ sub destroy_vm {
> }
>
> # only remove disks owned by this VM
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
> return if drive_is_cdrom($drive, 1);
>
> @@ -2340,7 +2340,7 @@ sub check_local_resources {
> sub check_storage_availability {
> my ($storecfg, $conf, $node) = @_;
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my $volid = $drive->{file};
> @@ -2363,7 +2363,7 @@ sub shared_nodes {
> my $nodehash = { map { $_ => 1 } @$nodelist };
> my $nodename = nodename();
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my $volid = $drive->{file};
> @@ -2395,7 +2395,7 @@ sub check_local_storage_availability {
> my $nodelist = PVE::Cluster::get_nodelist();
> my $nodehash = { map { $_ => {} } @$nodelist };
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my $volid = $drive->{file};
> @@ -3463,7 +3463,7 @@ sub config_to_command {
> push @$devices, '-iscsi', "initiator-name=$initiator";
> }
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
> @@ -4252,7 +4252,7 @@ sub qemu_volume_snapshot_delete {
>
> $running = undef;
> my $conf = PVE::QemuConfig->load_config($vmid);
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
> $running = 1 if $drive->{file} eq $volid;
> });
> @@ -4290,6 +4290,52 @@ sub set_migration_caps {
> mon_cmd($vmid, "migrate-set-capabilities", capabilities => $cap_ref);
> }
>
> +sub foreach_volid {
> + my ($conf, $func, @param) = @_;
> +
> + my $volhash = {};
> +
> + my $test_volid = sub {
> + my ($volid, $is_cdrom, $replicate, $shared, $snapname, $size) = @_;
> +
> + return if !$volid;
> +
> + $volhash->{$volid}->{cdrom} //= 1;
> + $volhash->{$volid}->{cdrom} = 0 if !$is_cdrom;
> +
> + $volhash->{$volid}->{replicate} //= 0;
> + $volhash->{$volid}->{replicate} = 1 if $replicate;
> +
> + $volhash->{$volid}->{shared} //= 0;
> + $volhash->{$volid}->{shared} = 1 if $shared;
> +
> + $volhash->{$volid}->{referenced_in_config} //= 0;
> + $volhash->{$volid}->{referenced_in_config} = 1 if !defined($snapname);
> +
> + $volhash->{$volid}->{referenced_in_snapshot}->{$snapname} = 1
> + if defined($snapname);
> + $volhash->{$volid}->{size} = $size if $size;
> + };
> +
> + PVE::QemuConfig->foreach_volume($conf, sub {
> + my ($ds, $drive) = @_;
> + $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, undef, $drive->{size});
> + });
> +
> + foreach my $snapname (keys %{$conf->{snapshots}}) {
> + my $snap = $conf->{snapshots}->{$snapname};
> + $test_volid->($snap->{vmstate}, 0, 1, $snapname);
if vmstate would be moved to QemuServer/Drive.pm 's schema, we could
pass it here as extra key to foreach_volume. also, isn't the vmstate
of a suspended VM missing here? both as potential follow-ups and don't
warrant another round ;)
> + PVE::QemuConfig->foreach_volume($snap, sub {
> + my ($ds, $drive) = @_;
> + $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, $snapname);
> + });
> + }
> +
> + foreach my $volid (keys %$volhash) {
> + &$func($volid, $volhash->{$volid}, @param);
> + }
> +}
> +
> my $fast_plug_option = {
> 'lock' => 1,
> 'name' => 1,
> @@ -4749,7 +4795,7 @@ sub vm_migrate_get_nbd_disks {
> my ($storecfg, $conf, $replicated_volumes) = @_;
>
> my $local_volumes = {};
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> return if drive_is_cdrom($drive);
> @@ -5608,7 +5654,7 @@ sub restore_file_archive {
> my $restore_cleanup_oldconf = sub {
> my ($storecfg, $vmid, $oldconf, $virtdev_hash) = @_;
>
> - foreach_drive($oldconf, sub {
> + PVE::QemuConfig->foreach_volume($oldconf, sub {
> my ($ds, $drive) = @_;
>
> return if drive_is_cdrom($drive, 1);
> @@ -6478,7 +6524,7 @@ sub foreach_storage_used_by_vm {
>
> my $sidhash = {};
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
> return if drive_is_cdrom($drive);
>
> @@ -6529,7 +6575,7 @@ sub template_create {
>
> my $storecfg = PVE::Storage::config();
>
> - foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> return if drive_is_cdrom($drive);
> diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
> index 559f331..b3ae57b 100644
> --- a/PVE/QemuServer/Cloudinit.pm
> +++ b/PVE/QemuServer/Cloudinit.pm
> @@ -466,7 +466,7 @@ sub generate_cloudinitconfig {
>
> my $format = get_cloudinit_format($conf);
>
> - PVE::QemuServer::foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file}, 1);
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index a08df66..f84333f 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -14,8 +14,6 @@ drive_is_cloudinit
> drive_is_cdrom
> parse_drive
> print_drive
> -foreach_drive
> -foreach_volid
> );
>
> our $QEMU_FORMAT_RE = qr/raw|cow|qcow|qcow2|qed|vmdk|cloop/;
> @@ -503,65 +501,6 @@ sub print_drive {
> return PVE::JSONSchema::print_property_string($drive, $alldrive_fmt, $skip);
> }
>
> -sub foreach_drive {
> - my ($conf, $func, @param) = @_;
> -
> - foreach my $ds (valid_drive_names()) {
> - next if !defined($conf->{$ds});
> -
> - my $drive = parse_drive($ds, $conf->{$ds});
> - next if !$drive;
> -
> - &$func($ds, $drive, @param);
> - }
> -}
> -
> -sub foreach_volid {
> - my ($conf, $func, @param) = @_;
> -
> - my $volhash = {};
> -
> - my $test_volid = sub {
> - my ($volid, $is_cdrom, $replicate, $shared, $snapname, $size) = @_;
> -
> - return if !$volid;
> -
> - $volhash->{$volid}->{cdrom} //= 1;
> - $volhash->{$volid}->{cdrom} = 0 if !$is_cdrom;
> -
> - $volhash->{$volid}->{replicate} //= 0;
> - $volhash->{$volid}->{replicate} = 1 if $replicate;
> -
> - $volhash->{$volid}->{shared} //= 0;
> - $volhash->{$volid}->{shared} = 1 if $shared;
> -
> - $volhash->{$volid}->{referenced_in_config} //= 0;
> - $volhash->{$volid}->{referenced_in_config} = 1 if !defined($snapname);
> -
> - $volhash->{$volid}->{referenced_in_snapshot}->{$snapname} = 1
> - if defined($snapname);
> - $volhash->{$volid}->{size} = $size if $size;
> - };
> -
> - foreach_drive($conf, sub {
> - my ($ds, $drive) = @_;
> - $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, undef, $drive->{size});
> - });
> -
> - foreach my $snapname (keys %{$conf->{snapshots}}) {
> - my $snap = $conf->{snapshots}->{$snapname};
> - $test_volid->($snap->{vmstate}, 0, 1, $snapname);
> - foreach_drive($snap, sub {
> - my ($ds, $drive) = @_;
> - $test_volid->($drive->{file}, drive_is_cdrom($drive), $drive->{replicate} // 1, $drive->{shared}, $snapname);
> - });
> - }
> -
> - foreach my $volid (keys %$volhash) {
> - &$func($volid, $volhash->{$volid}, @param);
> - }
> -}
> -
> sub bootdisk_size {
> my ($storecfg, $conf) = @_;
>
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index 08c29ba..38dcdb1 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -19,6 +19,7 @@ use PVE::Storage;
> use PVE::Tools;
> use PVE::VZDump;
>
> +use PVE::QemuConfig;
> use PVE::QemuServer;
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> @@ -68,7 +69,7 @@ sub prepare {
>
> my $vollist = [];
> my $drivehash = {};
> - PVE::QemuServer::foreach_drive($conf, sub {
> + PVE::QemuConfig->foreach_volume($conf, sub {
> my ($ds, $drive) = @_;
>
> return if PVE::QemuServer::drive_is_cdrom($drive);
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
More information about the pve-devel
mailing list