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

[Fabian Grünbichler]

> Fiona Ebner <f.ebner at proxmox.com> hat am 30.06.2025 13:45 CEST geschrieben:
> 
>  
> 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)?

I was thinking about a similar case like above - what if the throttle
group object was already removed.

but I guess it's more likely to hit the following sequence:

1. first detach runs into blockdev-del timeout and dies
2. blockdev deletion completes
3. second detach runs into blockdev no longer exists and returns

no object-del called at all.. should we maybe make it more robust
and handle the object already existing when attaching?