> Fiona Ebner <f.eb...@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?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
