[pve-devel] [PATCH qemu-server 15/22] vm start/commandline: activate volumes before config_to_command()

[Fiona Ebner]

Am 16.06.25 um 14:46 schrieb Fabian Grünbichler:
>> Fiona Ebner <f.ebner at proxmox.com> hat am 16.06.2025 13:51 CEST geschrieben:
>> Am 16.06.25 um 13:31 schrieb DERUMIER, Alexandre via pve-devel:
>>>>> With '-blockdev', it is necessary to activate the volumes to generate
>>>>> the command line, because it can be necessary to check whether the
>>>>> volume is a block device or a regular file.
>>>
>>> I was thinking about that, but do we have storage with activate_volume
>>> need to be done for a regular file ?
>>>
>>> for lvm plugin for example, we could return always driver=>host_device.
>>>
>>> activate_volume is always done in specific plugin, so the plugin should
>>> be able to tell if it's a block or file storage
>>>
>>> only custom path passthrough in vm configuration need to be checked if
>>> it's a file or device, but we don't have activate_volume anyway
>>
>> Good point! It is needed for the default implementation of
>> qemu_blockdev_options() in Plugin.pm. We could go ahead and have all
>> plugins use their own implementation of qemu_blockdev_options(), that
>> would avoid doing any actual IO there. That sounds good to me.
>>
>> The downside is that it would break third-party plugins that would
>> otherwise work fine with the default implementation. But actually, it's
>> only those that need to do something in activate_volume() to make the
>> volume accessible. Maybe requiring those to provide their own
>> qemu_blockdev_options() implementation is an acceptable level of
>> breakage, not entirely sure. We could even try and just guess in the
>> default implementation if we cannot access the volume, maybe based on
>> the presence of 'path'.
>>
>> @Fabian does that sound acceptable to you?
> 
> not sure I follow completely, to avoid misunderstandings:
> 
> variant A:
> 
> move activate_volume up front, no breakage, but more churn and higher 
> chance of leaving activated volumes around in case starting fails

It's not "no breakage". Plugins that are not happy with the default
implementation of qemu_blockdev_options() would still fail.

> variant B:
> 
> move decision which driver to use to plugin via qemu_blockdev_options

Yes.

> how would the default/fallback look like here? without activation, we
> can't rely on path since that doesn't currently require being activated,
> so we can't use it to distinguish block devices from files prior to
> activation..

As written above, we could still try to probe the file like we currently
do. If that works, great, we don't lose anything compared to variant A.
If that fails, we look at whether path is set. If yes, then assume it's
a file-based storage and use the 'file' driver. Otherwise, use the
'host_device' driver.