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

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On June 27, 2025 5:57 pm, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/QemuServer/Blockdev.pm | 132 +++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
> 
> diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
> index 6e6b9245..26d70eee 100644
> --- a/src/PVE/QemuServer/Blockdev.pm
> +++ b/src/PVE/QemuServer/Blockdev.pm
> @@ -11,6 +11,7 @@ use PVE::JSONSchema qw(json_bool);
>  use PVE::Storage;
>  
>  use PVE::QemuServer::Drive qw(drive_is_cdrom);
> +use PVE::QemuServer::Monitor qw(mon_cmd);
>  
>  my sub get_node_name {
>      my ($type, $drive_id, $volid, $snap) = @_;
> @@ -221,4 +222,135 @@ sub generate_drive_blockdev {
>      };
>  }
>  
> +my sub blockdev_add {
> +    my ($vmid, $blockdev) = @_;
> +
> +    eval { mon_cmd($vmid, 'blockdev-add', $blockdev->%*); };
> +    if (my $err = $@) {
> +        my $node_name = $blockdev->{'node-name'} // 'undefined';
> +        die "adding blockdev '$node_name' failed : $err\n" if $@;
> +    }
> +
> +    return;
> +}
> +
> +=pod
> +
> +=head3 attach
> +
> +    attach($storecfg, $vmid, $drive, $options);
> +
> +Attach the drive C<$drive> to the VM C<$vmid> considering the additional options C<$options>.
> +
> +Parameters:
> +
> +=over
> +
> +=item C<$storecfg>
> +
> +The storage configuration.
> +
> +=item C<$vmid>
> +
> +The ID of the virtual machine.
> +
> +=item C<$drive>
> +
> +The drive as parsed from a virtual machine configuration.
> +
> +=item C<$options>
> +
> +A hash reference with additional options.
> +
> +=over
> +
> +=item C<< $options->{'read-only'} >>
> +
> +Attach the image as read-only irrespective of the configuration in C<$drive>.
> +
> +=item C<< $options->{size} >>
> +
> +Attach the image with this virtual size. Must be smaller than the actual size of the image. The
> +image format must be C<raw>.
> +
> +=item C<< $options->{'snapshot-name'} >>
> +
> +Attach this snapshot of the volume C<< $drive->{file} >>, rather than the volume itself.
> +
> +=back
> +
> +=back
> +
> +=cut
> +
> +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)?

or alternatively, re-use the throttle-group ID from
generate_throttle_group here?

> +
> +    return;
> +}
> +
> +=pod
> +
> +=head3 detach
> +
> +    detach($vmid, $node_name);
> +
> +Detach the block device C<$node_name> from the VM C<$vmid>. Also removes associated child block
> +nodes.
> +
> +Parameters:
> +
> +=over
> +
> +=item C<$vmid>
> +
> +The ID of the virtual machine.
> +
> +=item C<$node_name>
> +
> +The node name identifying the block node in QEMU.
> +
> +=back
> +
> +=cut
> +
> +sub detach {
> +    my ($vmid, $node_name) = @_;
> +
> +    die "Blockdev::detach - no node name\n" if !$node_name;
> +
> +    # QEMU recursively auto-removes the file children, i.e. file and format node below the top
> +    # node and also implicit backing children referenced by a qcow2 image.
> +    eval { mon_cmd($vmid, 'blockdev-del', 'node-name' => "$node_name"); };
> +    if (my $err = $@) {
> +        return if $err =~ m/Failed to find node with node-name/; # already gone

does this happen regularly?

> +        die "deleting blockdev '$node_name' failed : $err\n";
> +    }
> +
> +    if ($node_name =~ m/^drive-(.+)$/) {

see above

> +        # 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?

> +        }
> +    }
> +
> +    return;
> +}
> +
>  1;
> -- 
> 2.47.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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