Re: [pve-devel] [PATCH v3 storage 1/9] plugin: add method to get qemu blockdevice options for volume

From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Fiona Ebner <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH v3 storage 1/9] plugin: add method to get qemu blockdevice options for volume
Date: Wed, 2 Jul 2025 10:40:42 +0200 (CEST)	[thread overview]
Message-ID: <405174354.595.1751445642696@webmail.proxmox.com> (raw)
In-Reply-To: <3579475c-b0d6-4ccd-8f39-9d669231726f@proxmox.com>

> Thomas Lamprecht <t.lamprecht@proxmox.com> hat am 02.07.2025 10:27 CEST geschrieben:
> 
>  
> Am 01.07.25 um 13:09 schrieb Fabian Grünbichler:
> >> Am 01.07.25 um 11:28 schrieb Thomas Lamprecht:
> >>> might make sense to pass some versioning information here, as otherwise this
> >>> is a bit to tight coupling for my taste and might cause long term maintenance
> >>> headache.
> > 
> > we discussed this in the past - this coupling is already there (see below), and
> > either qemu-server needs to hard-code storage things (which excludes third party
> > plugins), or pve-storage needs to encode some qemu things (the option we chose,
> > since there was already precedent and QEMU does provide a somewhat stable
> > interface there).
> 
> > this is also how plugins already work:
> > - path returns arbitrary drive options (if it's not an actual file path)
> 
> Which means my assumption that external plugins could not pass options
> in my other reply was wrong, so we would at least need to grandfather
> limitations in (like checking if api-version > current-version), not ideal
> but would be a way forward.

yes. we initially also evaluated trying to avoid all breakage here and
mapping back a returned `path` string to the new block dev options syntax,
but given that we don't/can't know what external plugins are doing, that is
probably also error-prone..

> > - plugins are 100% trusted and executed in root context already, so there isn't
> >   much we can safeguard against..
> So, we do lots of work here and got the chance to quite probably make
> the future target of having a more contained plugin and QEMU a little
> bit easier just to argue moving laterally because the status quo is
> like that? I'd care much if this would be internal only, but as it's a
> versioned public API that I do not want to break if not necessary I'd like
> to have this explored more, not just (to exaggerate, I know you both
> are quite thorough) hand waved away.

no, I just wanted to avoid the impression that this opens up some kind of
big hole here. and I think even if we make progress on locking down the
storage layer, we'd still need to delegate part of that work to the plugin
or admin in case of external plugins, so plugins will remain "trusted" in
that sense probably forever, even if their code during execution is put
into some kind of sandbox to reduce the blast radius of bugs.

> I mean, OTOH, once actual works towards plugin/VM containment starts we
> probably want to have a feature for that storage plugins need to signal
> support for, maybe combined with some other mechanisms, and that again
> would be grandfathered in for one major version (and/or configurable by
> the user) so we probably can still move towards more limitations also here
> in the future without a hard break. So, the option limitations don't need
> to necessarily happen now, but I'd really add the machine version now already,
> as that should at least allows plugin authors to better navigate the coupling
> we got here.

yes, adding the machine version so that plugin code can make informed decisions
seems sensible for future-proofing.

and if we can make that work, adding a schema in pve-storage to limit what a
plugin can return can also make sense, but I am not sure how much work that
entails in practice. there is some additional potential for breakage if we don't
account for all things external plugins might use (or if we want to forbid certain
things, but then it would be intentional breakage from our side ;)).

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