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

[Fabian Grünbichler]

> Fiona Ebner <f.ebner at proxmox.com> hat am 16.06.2025 14:57 CEST geschrieben:
> 
>  
> 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.

yes. what I meant is no breakage caused by file vs host_device, since if
we activate up front, we can query the returned path which should exist
at that point, and make an informed decision. of course, if a storage
requires a different driver altogether and/or doesn't return an actual
path that we can query, then its plugin needs adaptation in any case.

> 
> > 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.

but this assumption will be wrong half the time? if we don't activate
before deciding the driver, then we can only guess that a given path
is a file, and not a block device? if we have to activate before guessing,
then we are back at variant A (or at a mixed variant)?