Am 13.06.25 um 12:16 schrieb Fabian Grünbichler:
> On June 12, 2025 4:02 pm, Fiona Ebner wrote:
>> @@ -5982,14 +5979,25 @@ sub vm_commandline {
>>
>> my $defaults = load_defaults();
>>
>> + my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid);
>> + my $volumes = [];
>> +
>> + # With -blockdev, it is necessary to activate the volumes before
>> generating the command line
>> + if (!$running) {
>> + $volumes = get_current_vm_volumes($storecfg, $conf);
>> + PVE::Storage::activate_volumes($storecfg, $volumes);
>> + }
>> +
>> my $cmd;
>> eval {
>> $cmd = config_to_command($storecfg, $vmid, $conf, $defaults,
>> $forcemachine, $forcecpu);
>> };
>> my $err = $@;
>>
>> - # if the vm is not running, we need to clean up the reserved/created
>> devices
>> - if (!PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>> + # if the vm is not running, need to clean up the reserved/created
>> devices and activated volumes
>> + if (!$running) {
>> + eval { PVE::Storage::deactivate_volumes($storecfg, $volumes); };
>
> I don't think we are allowed to do that here - `qm showcmd` can run
> concurrently with other tasks (move disk, offline migration, ..) that
> might be at a point in time where they've just activated but not yet
> started using the volume..
>
> in general, volume deactivation is tricky, and should often be avoided
> unless it's 100% clear cut that it's safe to do (e.g., freshly allocated
> volume while holding a lock, freeing a volume, ..) or required
> (migration).
Will drop the deactivation in v2.
>> + warn $@ if $@;
>> eval { cleanup_pci_devices($vmid, $conf) };
>
> and this here might be dangerous as well, given that we don't take a
> lock and the running state might have changed already since we checked?
>
> should `qm showcmd` take a lock? or should we avoid doing the actual PCI
> reservation if we are in the "just show command" code path?
I went with the latter approach now, as it's cleaner to not reserve at all.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
