[pve-devel] [PATCH v3 qemu-server 03/11] blockdev : convert qemu_driveadd && qemu_drivedel

[Fabian Grünbichler]

> Alexandre Derumier via pve-devel <pve-devel at lists.proxmox.com> hat am 16.12.2024 10:12 CET geschrieben:
> fixme/testme :
> PVE/VZDump/QemuServer.pm:    eval { PVE::QemuServer::qemu_drivedel($vmid, "tpmstate0-backup"); };
> 
> Signed-off-by: Alexandre Derumier <alexandre.derumier at groupe-cyllene.com>
> ---
>  PVE/QemuServer.pm | 64 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 19 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2832ed09..baf78ec0 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1582,6 +1582,42 @@ sub print_drive_throttle_group {
>     return $throttle_group;
>  }
>  
> +sub generate_throttle_group {
> +    my ($drive) = @_;
> +
> +    my $drive_id = get_drive_id($drive);
> +
> +    my $throttle_group = { id => "throttle-drive-$drive_id" };
> +    my $limits = {};
> +
> +    foreach my $type (['', '-total'], [_rd => '-read'], [_wr => '-write']) {
> +	my ($dir, $qmpname) = @$type;
> +
> +	if (my $v = $drive->{"mbps$dir"}) {
> +	    $limits->{"bps$qmpname"} = int($v*1024*1024);
> +	}
> +	if (my $v = $drive->{"mbps${dir}_max"}) {
> +	    $limits->{"bps$qmpname-max"} = int($v*1024*1024);
> +	}
> +	if (my $v = $drive->{"bps${dir}_max_length"}) {
> +	    $limits->{"bps$qmpname-max-length"} = int($v)
> +	}
> +	if (my $v = $drive->{"iops${dir}"}) {
> +	    $limits->{"iops$qmpname"} = int($v);
> +	}
> +	if (my $v = $drive->{"iops${dir}_max"}) {
> +	    $limits->{"iops$qmpname-max"} = int($v);
> +	}
> +	if (my $v = $drive->{"iops${dir}_max_length"}) {
> +	    $limits->{"iops$qmpname-max-length"} = int($v);
> +	}
> +   }
> +
> +   $throttle_group->{limits} = $limits;
> +
> +   return $throttle_group;

this and the corresponding print sub are exactly the same, so the print sub could call this and join the limits with the `x-` prefix added? how does this interact with the qemu_block_set_io_throttle helper used when updating the limits at runtime?

> +}
> +
>  sub generate_file_blockdev {
>      my ($storecfg, $drive, $nodename) = @_;
>  
> @@ -4595,32 +4631,22 @@ sub qemu_iothread_del {
>  }
>  
>  sub qemu_driveadd {
> -    my ($storecfg, $vmid, $device) = @_;
> +    my ($storecfg, $vmid, $drive) = @_;
>  
> -    my $kvmver = get_running_qemu_version($vmid);
> -    my $io_uring = min_version($kvmver, 6, 0);
> -    my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef, $io_uring);
> -    $drive =~ s/\\/\\\\/g;
> -    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
> -
> -    # If the command succeeds qemu prints: "OK"
> -    return 1 if $ret =~ m/OK/s;
> +    my $drive_id = get_drive_id($drive);
> +    my $throttle_group = generate_throttle_group($drive);

do we always need a throttle group? or would we benefit from only adding it when limits are set, and skip that node when I/O is unlimited?

> +    mon_cmd($vmid, 'object-add', "qom-type" => "throttle-group", %$throttle_group);
>  
> -    die "adding drive failed: $ret\n";
> +    my $blockdev = generate_drive_blockdev($storecfg, $vmid, $drive);
> +    mon_cmd($vmid, 'blockdev-add', %$blockdev, timeout => 10 * 60);
> +    return 1;
>  }
>  
>  sub qemu_drivedel {
>      my ($vmid, $deviceid) = @_;
>  
> -    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60);
> -    $ret =~ s/^\s+//;
> -
> -    return 1 if $ret eq "";
> -
> -    # NB: device not found errors mean the drive was auto-deleted and we ignore the error
> -    return 1 if $ret =~ m/Device \'.*?\' not found/s;
> -
> -    die "deleting drive $deviceid failed : $ret\n";
> +    mon_cmd($vmid, 'blockdev-del', 'node-name' => "drive-$deviceid", timeout => 10 * 60);
> +    mon_cmd($vmid, 'object-del', id => "throttle-drive-$deviceid");
>  }
>  
>  sub qemu_deviceaddverify {
> -- 
> 2.39.5