Re: [pve-devel] [PATCH qemu-server 05/31] blockdev: add helpers for attaching and detaching block devices

[Fiona Ebner <f.ebner@proxmox.com>]

Am 30.06.25 um 12:15 schrieb Fabian Grünbichler:
> On June 27, 2025 5:57 pm, Fiona Ebner wrote:
>> +sub attach {
>> +    my ($storecfg, $vmid, $drive, $options) = @_;
>> +
>> +    my $blockdev = generate_drive_blockdev($storecfg, $drive, $options);
>> +
>> +    my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
>> +    if ($blockdev->{'node-name'} eq "drive-$drive_id") { # device top nodes need a throttle group
>> +        my $throttle_group = generate_throttle_group($drive);
>> +        mon_cmd($vmid, 'object-add', $throttle_group->%*);
>> +    }
>> +
>> +    eval { blockdev_add($vmid, $blockdev); };
>> +    if (my $err = $@) {
>> +        eval { mon_cmd($vmid, 'object-del', id => "throttle-drive-$drive_id"); };
>> +        warn $@ if $@;
>> +        die $err;
>> +    }
> 
> not sure whether we want (central) helpers for top-level node name
> (encoding and parsing) and throttle group ID (encoding and parsing)?

Won't hurt.

>> +        # also remove throttle group if it was a device top node
>> +        my $drive_id = $1;
>> +        if (PVE::QemuServer::Drive::is_valid_drivename($drive_id)) {
>> +            mon_cmd($vmid, 'object-del', id => "throttle-drive-$drive_id");
> 
> should this get an eval?

I think it's better to propagate the error (or do you mean having an
eval+die for adding context to the message)?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel