Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot support

[DERUMIER, Alexandre via pve-devel]

--- Begin Message ---
> 
> 
> But even with that, you can still have performance impact.
> So yes, I think they are really usecase for workload when you only
> need
> snapshot time to time (before an upgrade for example), but max
> performance with no snaphot exist.

>>my main point here is - all other storages treat snapshots as
>>"cheap". if you combine raw+qcow2 snapshot overlays, suddenly
>>performance will get worse if you keep a snapshot around for whatever
>>reason.. 
 

Ok, I redone a lot of bench yesterday, with a real san storage, and I
don't see too much difference between qcow2 and raw. (something like 
3iops on raw and 28000~29000 iops on qcow2).
I have tested with 2TB qcow2 file to be sure, and with new qcow2 sub-
cluster feature with l2_extended, it's not too much.

The difference is a little more big on a local nvme (I think because of
low latency), but as the usecase is for network storage, it's ok.

Let's go for full .qcow2, it'll be easier ;)


> > > it's a bit confusing to have a volid ending with raw, with the
> > > current volume and all but the first snapshot actually being
> > > stored
> > > in qcow2 files, with the raw file being the "oldest" snapshot in
> > > the
> > > chain..

> if it's too confusing, we could use for example an .snap extension.
> (as we known that it's qcow2 behind)

>>I haven't thought yet about how to encode the snapshot name into the
>>snapshot file name, but yeah, maybe something like  that would be
>>good. or maybe snap-VMID-disk-DISK.qcow2 ?

ok we can use snap-VMID-disk-DISK.qcow2 , I'll be easier for regex :p


> > > storage_migrate needs to handle external snapshots, or at least
> > > error
> > > out. 
> it should already work. (I have tested move_disk, and live migration
> +
> storage migration). qemu_img_convert offline and qemu block job for
> live.

>>but don't all of those lose the snapshots? did you test it with
>>snapshots and rollback afterwards?

ok, sorry, I have tested clone a new vm from a snapshot. (which use the
same code). I don't remember how it's work with move disk of a running
vm when snaphot exist.

> 
> The main problem is when you start a vm on a specific snasphot,
> we don't send the $snapname param.
> 
> One way could be that qemu-server check the current snapshot from
> config when doing specific action like start.

>>if we manage to find a way to make the volid always point at the top
>>overlay, then that wouldn't be needed..

yes, indeed if we are able to rename the current snapshot file to vm-
100-disk-0.qcow2  , it's super easy :)  

I need to do more test, because drive-reopen only seem to work if the
original drive is defined with -blockdev syntax. (It seem to clash on
nodename if it's not defined with -drive ).  

I have begin to look to implement blockdev, it don't seem too much
difficult for the start command line, but I need check the hotplug
part.
Maybe for pve9 ? (it could open door to features like luks encryption
too or or)



I'll rework all the patches after my holiday,  with both renaming of
current snapshot and only use .qcow2 format,  it should be a lot of
more clean and KISS.

Thanks again for the review !


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

Re: [pve-devel] Bug 2582 roadmap

[Thomas Lamprecht]

Hello,

Am 20/09/2024 um 14:32 schrieb Pavel Tide:
> 1) Connect via SSH to the PVE node and deploy a helper virtual machine (so 
> that users don't have to do it manually)
> 2) Access the Proxmox VE API to perform other backup-related tasks (those 
> that cannot be done via SSH)
> 
> In item #1 - the new VM deployment involved usage of root/sudo.
> 
> In item #2 - certain tasks that are performed via API also require root/sudo. 
> We have managed to move those to the SSH part of the workflow, so now users 
> can use one non-root account to perform all necessary operations (instead of 
> using root or having to use two separate accounts).
> 
> We think that in future there might be a situation where we might need a 
> superuser level of privileges while accessing the API, and there will be no 
> workaround to move the operation to the SSH part of the workflow. This will 
> result in forcing our joint users to use 'root' account again, which they 
> hate to do and also deem as an not secure practice.

Which situations/API calls would that be? It would be definitively
helpful to get specifics here, as otherwise it's hard to help and also a
bit hard to tell for sure if the Sys.Root privilege feature request
would even help here.
As that privilege would only allow current root-only API calls to be
used by non-root admin accounts, but it would not allow the account to
gain root access on the system just by having that privilege.

In general, I think it would be better to do less, not more, stuff
manually in the long term and rather check out the in-development
external backup provider API [0], as that would allow easier and safer
access to VM and CT data while integrating better with the existing PVE
stack, ideally reducing the potential for fallout on either site.

[0]: 
https://lore.proxmox.com/pve-devel/20240813132829.117460-1-f.eb...@proxmox.com/

- Thomas


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

Re: [pve-devel] applied: [PATCH container 1/1] status: add some missing descriptions for status return properties

[Dominik Csapak]

On 10/24/24 14:20, Thomas Lamprecht wrote:

Am 21/10/2024 um 11:15 schrieb Dominik Csapak:

Signed-off-by: Dominik Csapak 
---
  src/PVE/LXC.pm | 43 ++-
  1 file changed, 42 insertions(+), 1 deletion(-)




applied, thanks! FYI: I had to fix a duplicate key (see below) and made a few
small adjustments to the wording (not just your additions) as a separate 
follow-up.


+netin => {
+   description => "The amount of traffic that was sent to the guest since the 
process start,"
+   ." in bytes.",
+   type => 'integer',
+   optional => 1,
+   renderer => 'bytes',
+},
+netin => {


changed this to netout and folded that into your commit.


uff, thanks for that. seems i was too fast with the copy&paste...

i wonder if our check target could catch something like that, but
probably not i guess


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

Re: [pve-devel] [PATCH qemu-server] fix #5657: allow configuring RNG device as non-root user

[Fabian Grünbichler]

On September 3, 2024 3:58 pm, Filip Schauer wrote:
> On 02/09/2024 14:21, Fabian Grünbichler wrote:
>> IIRC this was intentional, since passing in the hardware RNG can starve
>> the host of entropy rather quickly. is this no longer the case, or
>> handled by some other check? if so, please include these details here.
>> if not, then I don't think we want to go with this patch - but maybe we
>> want to tighten some other code paths instead 😉
> 
> 
> Reading from /dev/urandom has never consumed entropy and reading from
> /dev/random no longer poses a concern since the kernel no longer uses a
> blocking entropy pool. [1] The only potential issue might be the
> starvation of the hardware RNG when /dev/hwrng is used. So we might not
> want to allow a non-root user to configure /dev/hwrng, but letting
> non-root users configure the other two options (/dev/urandom and
> /dev/random) seems reasonable.

yes, I was talking about the hardware RNG!

> It might make sense to only allow non-root users to configure
> /dev/urandom and /dev/random as RNG sources.

we could also define some sort of mapping-like thing for the hardware
RNG to allow semi-privileged users to pass it through, after a highly
privileged user set it up and gave them access? but we could wait until
somebody requests that ;)

> 
> [1] https://lwn.net/Articles/808575/
> 
> 
> 
> ___
> 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

[pve-devel] applied: [PATCH container] fix #5761: add the "discard" mount option

[Thomas Lamprecht]

Am 09/10/2024 um 16:22 schrieb Filip Schauer:
> Introduce the "discard" mount option for rootfs and mount points. This
> ensures that unused container volume blocks are discarded from the
> underlying storage backend when deleting files within the container.
> 
> Signed-off-by: Filip Schauer 
> ---
>  src/PVE/LXC/Config.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!

please add UI integration like Fiona noticed


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

Re: [pve-devel] [PATCH manager 1/1] api/ui: improve mdev listing for pci mappings

[Thomas Lamprecht]

Am 06/08/2024 um 14:22 schrieb Dominik Csapak:
> currently when we have a pci resource mapping, we manually check only
> the available models for the first pci entry. This often works, but not
> always, since one could have completely different devices in one
> mapping, or with the new nvidia sysfs api we don't get the generally
> available models.
> 
> To improve this, extend the 'pciid' regex to include pciids or mapping
> names, and for mappings, iterate over all local pci devices in it and
> extract the mdev types.
> 
> This also vastly simplifies the ui code, since we only have to give the
> mapping to the selector instead of an (arbitrarily selected) pci id.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/Hardware/PCI.pm | 45 +---
>  www/manager6/qemu/PCIEdit.js | 12 +-
>  2 files changed, 38 insertions(+), 19 deletions(-)
> 
> diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm
> index a3a689bf..7135a605 100644
> --- a/PVE/API2/Hardware/PCI.pm
> +++ b/PVE/API2/Hardware/PCI.pm
> @@ -135,7 +135,7 @@ __PACKAGE__->register_method ({
>  
>  __PACKAGE__->register_method ({
>  name => 'pciindex',
> -path => '{pciid}',
> +path => '{pciid-or-mapping}',
>  method => 'GET',
>  description => "Index of available pci methods",
>  permissions => {
> @@ -145,9 +145,9 @@ __PACKAGE__->register_method ({
>   additionalProperties => 0,
>   properties => {
>   node => get_standard_option('pve-node'),
> - pciid => {
> + 'pciid-or-mapping' => {

While in general this should be fine w.r.t. breaking changes for the API itself,
as the name of those template-variables from the API path can be basically seen
as internal detail, it would IMO be warranted to state that explicitly in the
commit message, as FWICT this is basically the only time where changing the
parameter name mid-release is legit without some backward compat handling.

And just to be sure: you are certain that this cannot be passed anyhow else,
e.g. in some CLI (pvesh?) or other way?


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

[pve-devel] partially-applied: [PATCH common/qemu-server/manager] adapt to nvidia vgpu api changes

[Thomas Lamprecht]

Am 06/08/2024 um 14:21 schrieb Dominik Csapak:
> For many new cards, nvidia changed the kernel interface since kernel
> verion 6.8. Instead of using mediated devices, they provide their own
> api.
> 
> This series adapts to that, with no required change to the vm config,
> and only minimal changes to our api.
> 
> The biggest change is that the mdev types can now be queried on
> /nodes/NODE/hardware/pci/ (like it was before) or via the name of a pci mapping (now checks all
> local devices from that mapping)
> 
> A thing to improve could be to parse the available vgpu types from
> nvidia-smi instead of the sysfs, since that not always contains all
> types (see the common patch 1/2 for details)
> 
> We could abstract the code that deals with different types probably a
> bit more, but for me it seems Ok for now, and finding a good API for
> that is hard with only 3 modes that are very different from each other
> (raw/mdev/nvidia).
> 
> qemu-server patches depend on the common patches, but the manager patch
> does not rely on any other in this series. It is required though
> for the user to be able to select types (in certain conditions).
> 
> note that this series requires my previous patch to the sysfstools to
> improve write reliability[0], otherwise the cleanup or creation may
> fail.
> 
> 0: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064814.html
> 
> pve-common:
> 
> Dominik Csapak (2):
>   SysFSTools: handle new nvidia syfsapi as mdev
>   SysFSTools: lscpi: move mdev and iommugroup check outside of verbose
> 
>  src/PVE/SysFSTools.pm | 83 ++-
>  1 file changed, 51 insertions(+), 32 deletions(-)
> 
> qemu-server:
> 
> Dominik Csapak (3):
>   pci: choose devices: don't reserve pciids when vm is already running
>   pci: remove pci reservation: optionally give list of ids to remove
>   pci: mdev: adapt to nvidia interface with kernel >= 6.8
> 
>  PVE/QemuServer.pm| 30 +--
>  PVE/QemuServer/PCI.pm| 92 +---
>  test/run_config2command_tests.pl |  8 ++-
>  3 files changed, 118 insertions(+), 12 deletions(-)
> 
> pve-manager:
> 
> Dominik Csapak (1):
>   api/ui: improve mdev listing for pci mappings
> 
>  PVE/API2/Hardware/PCI.pm | 45 +---
>  www/manager6/qemu/PCIEdit.js | 12 +-
>  2 files changed, 38 insertions(+), 19 deletions(-)
> 


applied all but the pve-manager patch for now with Christoph's T-b and R-b, 
thanks!


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

[pve-devel] applied: [PATCH container] fix #5762: lxc: setup: support `opensuse-slowroll` as OpenSUSE flavor

[Thomas Lamprecht]

Am 11/10/2024 um 13:29 schrieb Christoph Heiss:
> See the original bug report [0]. OpenSUSE Tumbleweed Slowroll fails to
> be detected corrected due to featuring a different ID in
> /etc/os-release.
> 
> Simply map that ID to the existing opensuse plugin - much like
> Tumbleweed itself. Slowroll is basically just a midway solution between
> Leap and Tumbleweed, so there shouldn't be any incompatibilities
> really - since we determine specifics in the plugin by version anyway.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5762
> [1] https://en.opensuse.org/Portal:Slowroll
> 
> Signed-off-by: Christoph Heiss 
> ---
>  src/PVE/LXC/Setup.pm  |  1 +
>  src/test/test-opensuse-slowroll-001/config|  5 +
>  .../test-opensuse-slowroll-001/etc/hosts.exp  |  5 +
>  .../test-opensuse-slowroll-001/etc/os-release | 16 +++
>  .../etc/resolv.conf.exp   |  5 +
>  .../test-opensuse-slowroll-001/etc/securetty  |  7 +++
>  .../etc/securetty.exp | 12 +++
>  .../etc/sysconfig/network/ifcfg-eth0.exp  |  3 +++
>  .../etc/sysconfig/network/ifcfg-eth1.exp  |  3 +++
>  .../etc/sysconfig/network/ifcfg-eth2.exp  |  2 ++
>  .../etc/sysconfig/network/ifcfg-eth3.exp  |  2 ++
>  .../etc/sysconfig/network/ifroute-eth0.exp|  3 +++
>  .../etc/sysconfig/network/ifroute-eth1|  1 +
>  .../etc/sysconfig/network/ifroute-eth1.exp|  1 +
>  .../root/.ssh/authorized_keys.exp |  3 +++
>  .../systemd/system/container-getty@.service   | 20 +++
>  16 files changed, 89 insertions(+)
>  create mode 100644 src/test/test-opensuse-slowroll-001/config
>  create mode 100644 src/test/test-opensuse-slowroll-001/etc/hosts.exp
>  create mode 100644 src/test/test-opensuse-slowroll-001/etc/os-release
>  create mode 100644 src/test/test-opensuse-slowroll-001/etc/resolv.conf.exp
>  create mode 100644 src/test/test-opensuse-slowroll-001/etc/securetty
>  create mode 100644 src/test/test-opensuse-slowroll-001/etc/securetty.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifcfg-eth0.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifcfg-eth1.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifcfg-eth2.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifcfg-eth3.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifroute-eth0.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifroute-eth1
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/etc/sysconfig/network/ifroute-eth1.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/root/.ssh/authorized_keys.exp
>  create mode 100644 
> src/test/test-opensuse-slowroll-001/usr/lib/systemd/system/container-getty@.service
> 
>

applied, thanks!


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

[pve-devel] applied: [RFC PATCH common] sysfstools: file_write: properly catch errors

[Thomas Lamprecht]

Am 23/07/2024 um 10:29 schrieb Dominik Csapak:
> since `print` is doing buffered IO, we don't always get an error there,
> even if the underlying write does not work.
> 
> To properly catch that, do an unbuffered `syswrite` which circumvents
> all buffers and writes directly to the file handle.
> 
> We aren't actually interested in the specific error here, but only if
> the write was successful or not.
> 
> Signed-off-by: Dominik Csapak 
> ---
> 
> Note: this is heavily used when doing PCI passthrough, and it seems we
> did not catch as many errors as we thought. Maybe we should introduce an
> 'noerr' parameter and pass that on all current code paths, since i fear
> that there are many situations where the sysfs write had failed but it
> still works because we ignored it and qemu/the kernel do the right thing
> anyway.
> 

I also see some regression potential, but IMO it's something that would
be very good to know, and so erroring out explicitly for now is IMO worth
it. We can then adapt this as needed on actual feedback.

>  src/PVE/SysFSTools.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, with Christoph's T-b thanks!


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

Re: [pve-devel] [PATCH v2 pve-storage 2/2] add lvmqcow2 plugin: (lvm with external qcow2 snapshot)

[DERUMIER, Alexandre via pve-devel]

--- Begin Message ---
Thanks Fabian for your time ! 

I have tried to respond as much as possible. (I'm going to Holiday for
1 week tomorrow, so sorry if I don't reply to all your comments)



 Message initial 
De: Fabian Grünbichler 
À: "DERUMIER, Alexandre" , pve-
de...@lists.proxmox.com 
Objet: Re: [pve-devel] [PATCH v2 pve-storage 2/2] add lvmqcow2 plugin:
(lvm with external qcow2 snapshot)
Date: 24/10/2024 09:42:00


> DERUMIER, Alexandre  hat am
> 23.10.2024 15:45 CEST geschrieben:
> 
>  
> > > I am not yet convinced this is somehow a good idea, but maybe you
> > > can
> > > convince me otherwise ;)

>>I maybe judged this too quickly - I thought this was combining LVM +
>>a dir-based storage, but this is putting the qcow2 overlays on LVs,
>>which I missed on the first pass!

Ah ok ! ^_^  yes, this is really 100% lvm (for shared lvm)


> > > variant A: this is just useful for very short-lived snapshots
> > > variant B: these snapshots are supposed to be long-lived
> 
> Can you defined "short "/ "long"  ? and the different usecase ?
> 
> because for me, a snapshot is a snapshot. Sometime I take a snapshot
> before doing some critical changes, but I can't known if I need to
> rollback in next 2h, or next month.

>>yes, this would be an example of a short-lived snapshot
 
ok
> I think that "long-lived" usecase is backup (but we don't need it),
> or replication (this could apply here, if we want to add replication
> for disaster recovery)

>>backup would also be short-lived usually (the snapshot is just to
>>take the backup, not to keep a backup). long-lived usually is
>>something like "take daily snapshot and keep for a few weeks for file
>>recovery", in addition to regular backups. or "snapshot because we
>>just had an incidence and might need this for forensics in a few
>>months" (can also be solved with backups, of course ;)).

>>the main difference between the two is - for short-lived snapshots
>>performance implications related to snapshots existing are not that
>>important. I can live with a few hours of degraded performance, if
>>the snapshot is part of some important process/work flow. with long-
>>lived snapshots there is a requirement for them to not hurt
>>performance just by existing, because otherwise you can't use them.
>>there is a second reason why long-lived snapshots can be impossible 

ok, so here, with qcow2 format, performance shouldn't be a problem.
(That's the whole point of this patch, using qcow2 format instead basic
slow lvm snasphot)

>>if you need to decide up-front how "big" the delta of that snapshot
>>can grow at most, then in PVE context, you always need to allocate
>>the full volume size (regular thick LVM had both issues - bad
>>performance, and new writes going into a thick snapshot volume).

about thick snaphot volume, technically, it could be possible to create
smaller lvm volume than the qcow2 virtual-size, and dynamically extend
it. ovirt is doing it like this. (I nave send a prelimary patch in
september, but for now, I'll like to keep it simple with thick
snapshots volume).


>>if you can support long-lived snapshots, then you automatically also
>>support short-lived snapshots. the other direction doesn't hold.
>>since PVE only has one kind of snapshots, they need to be useful for
>>long-lived snapshots.

ok got it.
> > > A is not something we want. we intentionally don't have non-thin
> > > LVM
> > > snapshots for example.
> 
> AFAIK, we never had implemented it because LVM snasphot is slow as
> hell.(as a lvm extent are around 4MB, if you want 4k on a snapshot,
> you
> need to reread and rewrite the 4MB,  so around 1000x over-
> amplification and slow iops)

>>see above - there's two issues, one is performance, the other is that
>>you need to either
>>- make the snapshot smaller than the original volume (and risk
>>running out of space)
>>- make the snapshot as big as the original volume (and blow up space
>>requirements)
>>
>>(thick) LVM snapshots basically barely work for the "take a
>>consistent backup during quiet periods" use case, and not much else.


> This is really the main blocker for my customers migrating from
> vmware
> (and to be honest I have some of them going to oracle ovlm (with
> ovirt), because ovirt support it this way). 

> > > B once I create a single snapshot, the "original" storage only
> > > contains the data written up to that point, anything else is
> > > stored
> > > on the "snapshot" storage. this means my snapshot storage must be
> > > at
> > > least as fast/good/shared/.. as my original storage. in that
> > > case, I
> > > can just use the snapshot storage directly and ditch the original
> > > storage?
> 
> Sorry, but I don't understand why you are talking about
> original/snapshot storage ? I never have thinked to use another
> storage
> for external snapshot.
> 
> The patch is really to add external snapshot on same lvm storage,
> through lvm additional volume, but with qcow2 format to have good
> performance (vs 

Re: [pve-devel] [PATCH qemu 1/2] vm-network-scripts: move scripts to /usr/libexec

[Fiona Ebner]

It's the "qemu-server" repository, not "qemu".

Am 09.10.24 um 14:55 schrieb Maximiliano Sandoval:
> Moves the network scripts from /var/lib/qemu-server into
> /usr/libexec/qemu-server.
> 
> /usr/libexec is described as binaries run by programs which are not
> intended to be directly executed by the user on [FHS 4.7]. On the other
> hand /var/lib corresponds to variable state information, which does not
> fit the use case here, see [FHS 5.8].
> 
> [FHS 4.7]: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s07.html
> [FHS 5.8]: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05s08.html
> 

Can this cause issues for a brief time during upgrade/unpacking, i.e.

A) if the old QemuServer.pm is still loaded while the scripts get
removed from its old location (or to be more precise, a QEMU process
with the old parameter is started at that time)
B) if the new QemuServer.pm gets loaded before the scripts are extracted
to their new location (or to be more precise, a QEMU process with the
new parameter is started at that time)

?

Is there something preventing those from happening? Otherwise, we might
need to ship the scripts to both locations until the next major release
and drop them from the old location only then.

> Signed-off-by: Maximiliano Sandoval 
> ---
>  PVE/QemuServer.pm |  4 ++--
>  vm-network-scripts/Makefile   | 10 +-
>  vm-network-scripts/pve-bridge-hotplug |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index b26da505..88100638 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1759,8 +1759,8 @@ sub print_netdev_full {
>  my $script = $hotplug ? "pve-bridge-hotplug" : "pve-bridge";
>  
>  if ($net->{bridge}) {
> - $netdev = 
> "type=tap,id=$netid,ifname=${ifname},script=/var/lib/qemu-server/$script"
> - .",downscript=/var/lib/qemu-server/pve-bridgedown$vhostparam";
> + $netdev = 
> "type=tap,id=$netid,ifname=${ifname},script=/usr/libexec/qemu-server/$script"
> + .",downscript=/usr/libexec/qemu-server/pve-bridgedown$vhostparam";
>  } else {
>  $netdev = "type=user,id=$netid,hostname=$vmname";
>  }
> diff --git a/vm-network-scripts/Makefile b/vm-network-scripts/Makefile
> index 5ba537d0..dc2c85ff 100644
> --- a/vm-network-scripts/Makefile
> +++ b/vm-network-scripts/Makefile
> @@ -1,12 +1,12 @@
>  DESTDIR=
> -VARLIBDIR=$(DESTDIR)/var/lib/qemu-server
> +LIBEXECDIR=$(DESTDIR)/usr/libexec/qemu-server
>  
>  .PHONY: install
>  install: pve-bridge pve-bridge-hotplug pve-bridgedown
> - install -d ${VARLIBDIR}
> - install -m 0755 pve-bridge ${VARLIBDIR}/pve-bridge
> - install -m 0755 pve-bridge-hotplug ${VARLIBDIR}/pve-bridge-hotplug
> - install -m 0755 pve-bridgedown ${VARLIBDIR}/pve-bridgedown
> + install -d ${LIBEXECDIR}
> + install -m 0755 pve-bridge ${LIBEXECDIR}/pve-bridge
> + install -m 0755 pve-bridge-hotplug ${LIBEXECDIR}/pve-bridge-hotplug
> + install -m 0755 pve-bridgedown ${LIBEXECDIR}/pve-bridgedown
>  
>  .PHONY: clean
>  clean:
> diff --git a/vm-network-scripts/pve-bridge-hotplug 
> b/vm-network-scripts/pve-bridge-hotplug
> index f36ed408..3ae01ea5 100755
> --- a/vm-network-scripts/pve-bridge-hotplug
> +++ b/vm-network-scripts/pve-bridge-hotplug
> @@ -1,3 +1,3 @@
>  #!/bin/sh
>  
> -exec /var/lib/qemu-server/pve-bridge --hotplug "$@"
> +exec /usr/libexec/qemu-server/pve-bridge --hotplug "$@"


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

Re: [pve-devel] Bug 2582 roadmap

[Pavel Tide via pve-devel]

--- Begin Message ---
Hi Fiona et al,

Just wanted to follow up on the subject.

I think I need some guidance from you and your team members on how we at Veeam 
could facilitate development of this and some other features in Proxmox VE.
Is there any standard protocol for that established, or we should simply submit 
a request/proposal in the mailing list and wait for someone to see it and 
respond?

Thanks!

-Original Message-
From: Pavel Tide 
Sent: Friday, September 20, 2024 14:32
To: Fiona Ebner; Proxmox VE development discussion
Subject: RE: [pve-devel] Bug 2582 roadmap

Hi Fiona,

Thank you for helping me out and sorry for my late response.

The issue is that right now to work with PVE we have to do the following:

1) Connect via SSH to the PVE node and deploy a helper virtual machine (so that 
users don't have to do it manually)
2) Access the Proxmox VE API to perform other backup-related tasks (those that 
cannot be done via SSH)

In item #1 - the new VM deployment involved usage of root/sudo.

In item #2 - certain tasks that are performed via API also require root/sudo. 
We have managed to move those to the SSH part of the workflow, so now users can 
use one non-root account to perform all necessary operations (instead of using 
root or having to use two separate accounts).

We think that in future there might be a situation where we might need a 
superuser level of privileges while accessing the API, and there will be no 
workaround to move the operation to the SSH part of the workflow. This will 
result in forcing our joint users to use 'root' account again, which they hate 
to do and also deem as an not secure practice.

I hope that helps. If there is anything else what we could do from out side 
please let me know.

Thanks!

-Original Message-
From: Fiona Ebner 
Sent: Friday, September 13, 2024 09:59
To: Proxmox VE development discussion
Cc: Pavel Tide
Subject: Re: [pve-devel] Bug 2582 roadmap

Hi Pavel,

Am 10.09.24 um 12:18 schrieb Pavel Tide via pve-devel:
> Hi Proxmox team,
>
> Would you provide any delivery estimates on this item?
> https://bugz/
> illa.proxmox.com%2Fshow_bug.cgi%3Fid%3D2582&data=05%7C02%7CPavel.TIde%
> 40veeam.com%7C9fbe3a27cdb746e4522e08dcd3c9f802%7Cba07baab431b49edadd7c
> bc3542f5140%7C1%7C0%7C638618111644860239%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7
> C%7C&sdata=rnV7UTTM7GUpysGbgpLRfGDOA7xtwoACZXoq7N9anNg%3D&reserved=0
>
> As far as I understand it's been implemented already, but currently stays in 
> the development branch - our lab is up to date and yet we don't see how we 
> can create a separate non-root account to work with PVE cluster.
>

a patch series had been proposed, but the implementation was not finished 
AFAIK, see Fabian's review[0]. Since Oguz left the company, nobody else has 
picked up work on the series yet. Maybe you can describe what exactly your use 
case is, which privileges you'd need in particular. Of course, proposing 
patches for what you need (or a rebased version of Oguz's full series) is 
welcome too :)

[0]:
https://lore.proxmox.com/pve-devel/1658908014.zeyifvlr1o.astr...@nora.none/

Best Regards,
Fiona


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

[pve-devel] applied: [PATCH container 1/1] status: add some missing descriptions for status return properties

[Thomas Lamprecht]

Am 21/10/2024 um 11:15 schrieb Dominik Csapak:
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/LXC.pm | 43 ++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 


applied, thanks! FYI: I had to fix a duplicate key (see below) and made a few
small adjustments to the wording (not just your additions) as a separate 
follow-up.

> +netin => {
> + description => "The amount of traffic that was sent to the guest since 
> the process start,"
> + ." in bytes.",
> + type => 'integer',
> + optional => 1,
> + renderer => 'bytes',
> +},
> +netin => {

changed this to netout and folded that into your commit.

> + description => "The amount of traffic that was sent from the guest 
> since the process start,"
> + ." in bytes.",
> + type => 'integer',
> + optional => 1,
> + renderer => 'bytes',
> +},
>  uptime => {
>   description => "Uptime.",
>   type => 'integer',



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

Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot support

[Giotta Simon RUAGH via pve-devel]

--- Begin Message ---
Hi Everyone

> I mean, if we don't allow .raw files to be snapshotted then this problem 
> doesn't exist ;)

Quick comment from the bleacher; Adding a mechanism to shapshot raw disks might 
solve the TPM (tpmstate) snapshotting issue, as well as allowing containers to 
be snapshot.

For context: 
When using a storage that does not natively support snapshotting (NFS on NetApp 
or similar enterprise storage, in particular), raw disks cannot be snapshot. 
Since tpmstate disks can only be stored as raw (as I understand they are just a 
binary blob?), this makes it impossible to snapshot or (link-)clone any VMs 
that have a TPM. This especially is an issue for current Windows clients.
Same issue for LXC containers, as their storage format is raw only as well.
 
https://bugzilla.proxmox.com/show_bug.cgi?id=4693

Beste Grüsse

Simon Giotta
Systemadministrator

simon.gio...@ruag.ch 

RUAG AG
Schaffhauserstrasse 580 | 8052 Zürich

-Original Message-
From: pve-devel  On Behalf Of Fabian 
Grünbichler
Sent: Donnerstag, 24. Oktober 2024 08:42
To: DERUMIER, Alexandre ; 
pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot 
support


> DERUMIER, Alexandre  hat am 23.10.2024 
> 14:59 CEST geschrieben:
> 
>  
> Hi Fabian,
> 
> thanks for the review !
> 
> >> Message initial 
> >>De: Fabian Grünbichler 
> >>À: Proxmox VE development discussion 
> >>Cc: Alexandre Derumier 
> >>Objet: Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external 
> >>snasphot support
> >>Date: 23/10/2024 12:12:46
> >>
> >>some high level comments:
> >>
> >>I am not sure how much we gain here with the raw support?
> 
> They are really qcow2 overhead, mostly with big disk.
> as for good performance, the qcow2 l2-cache-size need to be keeped in 
> memory (and it's 1MB by disk) 
> https://events.static.linuxfound.org/sites/events/files/slides/kvm-for
> um-2017-slides.pdf
> 
> Hopefully, they are improvments with the "new" sub-cluster feature 
> https://people.igalia.com/berto/files/kvm-forum-2020-slides.pdf
> I'm already using it at snapshot create, but I think we should also 
> use it for main qcow2 volume.
> 
> 
> But even with that, you can still have performance impact.
> So yes, I think they are really usecase for workload when you only 
> need snapshot time to time (before an upgrade for example), but max 
> performance with no snaphot exist.

my main point here is - all other storages treat snapshots as "cheap". if you 
combine raw+qcow2 snapshot overlays, suddenly performance will get worse if you 
keep a snapshot around for whatever reason.. 
 
> >> it's a bit confusing to have a volid ending with raw, with the 
> >>current volume and all but the first snapshot actually being stored 
> >>in qcow2 files, with the raw file being the "oldest" snapshot in the 
> >>chain..

> if it's too confusing, we could use for example an .snap extension.
> (as we known that it's qcow2 behind)

I haven't thought yet about how to encode the snapshot name into the snapshot 
file name, but yeah, maybe something like  that would be good. or maybe 
snap-VMID-disk-DISK.qcow2 ?

> if possible, I'd be much happier with the snapshot name in the 
> snapshot file being a 1:1 match, see comments inline
> 
> >>- makes it a lot easier to understand (admin wants to manually 
> >>remove snapshot "foo", if "foo" was the last snapshot then right now 
> >>the volume called "foo" is actually the current contents!)
> 
> This part is really difficult, because you can't known in advance the 
> name of the snapshot you'll take in the future. The only way could be 
> to create a "current" volume ,  rename it when you took another 
> snasphot (I'm not sure it's possible to do it live, and this could 
> break link chain too)
> 
> Also, I don't known how to manage the main volume, when you take the 
> first snapshot ? we should rename it too.

I mean, if we don't allow .raw files to be snapshotted then this problem 
doesn't exist ;)

> so "vm-disk-100-disk-0.raw|qcow2"  , become "vm-disk-100-disk-0- 
> snap1.(raw|qcow2)" + new "vm-disk-100-disk-0-current.qcow2" ?

the volid changing on snapshot seems like it would require a lot of adaption.. 
OTOH, the volid containing a wrong format might also break things.

> I'll try to do test again to see what is possible.
> 
> 
> 
> 
> >>- means we don't have to do lookups via the full snapshot list all 
> >>the time (e.g., if I want to do a full clone from a snapshot "foo", 
> >>I can just pass the snap-foo volume to qemu-img)
> 
> ok got it
> 
> 
> 
> >>the naming scheme for snapshots needs to be adapted to not clash 
> >>with regular volumes:
> 
> >>$ pvesm alloc extsnap 131314 vm-131314-disk-foobar.qcow2 2G 
> >>Formatting '/mnt/pve/ext4/extsnap/images/131314/vm-131314-disk-
> >>foobar.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off 
> >>preallocation=off compression_type=zlib size=2147483648 
> >>lazy_refcounts=off refcount_bits=16 successfully c

[pve-devel] applied: [PATCH qemu] fix typos in user-visible strings

[Fiona Ebner]

Am 08.10.24 um 17:14 schrieb Maximiliano Sandoval:
> This includes docs, and strings printed to stderr or stdout.
> 
> These were caught with:
> 
> typos --exclude test --exclude changelog
> 
> Signed-off-by: Maximiliano Sandoval 

applied, thanks!

But please note that it's the "qemu-server" repository, not "qemu" ;)


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

[pve-devel] applied: [PATCH manager 1/2] api: subscription: add return schema for 'GET' api

[Thomas Lamprecht]

Am 21/10/2024 um 11:15 schrieb Dominik Csapak:
> This was missing, but it mostly well defined since we're using the rust
> bindings here. I copied most descriptions over from the PBS api, except
> the ones only existing here (like sockets and level)
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/Subscription.pm | 67 +++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


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

Re: [pve-devel] [PATCH pve-network/pve-common/pve-manager] fix #4300 : sdn: add bridge ports isolation

[DERUMIER, Alexandre via pve-devel]

--- Begin Message ---
Hi,

any news about this patch series ?

I think it's still not applied ?   (I see a lot of request about it on
the forum and on the bugzilla)

Regards,

Alexandre


 Message initial 
De: "DERUMIER, Alexandre" 
À: pve-devel@lists.proxmox.com ,
s.hanre...@proxmox.com 
Objet: Re: [pve-devel] [PATCH pve-network/pve-common/pve-manager] fix
#4300 : sdn: add bridge ports isolation
Date: 27/06/2024 18:23:56

Hi!


> > Hi! I gave this a quick test on my machine and everything worked
well.
> > Would we maybe want to expose this setting on the NIC level as
> > well?

I don't think it can work, because a port not isolated, have access to
all other ports,including isolated ports.


"
isolated on or isolated off
Controls whether a given port will be isolated, which means it will be
able to communicate with non-isolated ports only. By default this flag
is off."


for example:
vm1: isolated
vm2: isolated
vm3: non isolated


vm1: can't access to vm2
vm2: can't access to vm1

vm3 have access to vm1 && vm2 isolated.  (but user is thinking that vm1
&& vm2 are secure).
and vm1/vm2 have access to vm3 too.


That's why I have done it at bridge/vnet level,  all or nothing.

The main usage is to have only 1 upstream port non isolated (traffic
coming from outside) 


> > Also I think 'Isolate Ports' or 'Port Isolation' would be the
> > better
> > label, 'Ports Isolation' sounds a bit wrong to me.

I'll send a v2 with "Port Isolation"



Otherwise, consider this:

> > Tested-By: Stefan Hanreich 
> > Reviewed-By: Stefan Hanreich 

Thanks !

 4/25/24 16:43, Alexandre Derumier via pve-devel wrote:
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.vadesecure.com/v4?f=OGhLSzUzUW5ZSnhsUnB1Zwk-
> iBGgPyVY4TTNWFYEVcCg2sqZ42p4ld6uKOxcEXt1&i=enliNE9Ec0FwcDdnUXU4UdqsUW
> Q6P4MlGVBmGUhBgqg&k=qWGl&r=TnY3ZTF2Q2plM1daMndLWY2hdyEItuD5-
> BacJIgJqvZ3qD1cLHhtTB2x5DvZF4UIAZISGlCJrAF01C9VxKgOjg&s=926df6762a5f8
> 47592379de9a2d61dc8a3bf0ade01884ae3830a7e63f216d753&u=https%3A%2F%2Fl
> ists.proxmox.com%2Fcgi-bin%2Fmailman%2Flistinfo%2Fpve-devel



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

[pve-devel] Bug 4300 update

[Francois Prowse via pve-devel]

--- Begin Message ---
Is it possible to understand when we can expect to see bugzilla enhancement 
4300 incorporated into a production build?

Re

Francois
--- End Message ---
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 1/2] tui: show background header on fatal setup error

[Christoph Heiss]

This was missing, as it was only applied for the main installation UI -
the setup error has its own screen setup codepath entirely.

Signed-off-by: Christoph Heiss 
---
 proxmox-tui-installer/src/main.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/proxmox-tui-installer/src/main.rs 
b/proxmox-tui-installer/src/main.rs
index 3fb87a7..b1f11cb 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -219,6 +219,7 @@ fn installer_setup_late(siv: &mut Cursive) {
 }
 
 fn initial_setup_error(siv: &mut CursiveRunnable, message: &str) -> ! {
+siv.add_fullscreen_layer(InstallerBackgroundView::new());
 siv.add_layer(
 Dialog::around(TextView::new(message))
 .title("Installer setup error")
-- 
2.46.0



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

[pve-devel] [PATCH installer 0/2] tui: throw setup error if no network interfaces were found

[Christoph Heiss]

Currently, the TUI does not error out like the GUI if no network
interfaces are found, so remedy that. 

Also, while testing, noticed that if an early/setup error is thrown in
the installer, no background header gets displayed - since it uses a
completely different screen setup codepath. 

Christoph Heiss (2):
  tui: show background header on fatal setup error
  installer-common: throw setup error if no network interfaces were
found

 proxmox-installer-common/src/setup.rs | 2 ++
 proxmox-tui-installer/src/main.rs | 1 +
 2 files changed, 3 insertions(+)

-- 
2.46.0



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

Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot support

[Fabian Grünbichler]

> Fabian Grünbichler  hat am 23.10.2024 12:12 CEST 
> geschrieben:
> 
>  
> some high level comments:
> 
> I am not sure how much we gain here with the raw support? it's a bit 
> confusing to have a volid ending with raw, with the current volume and all 
> but the first snapshot actually being stored in qcow2 files, with the raw 
> file being the "oldest" snapshot in the chain..
> 
> if possible, I'd be much happier with the snapshot name in the snapshot file 
> being a 1:1 match, see comments inline
> - makes it a lot easier to understand (admin wants to manually remove 
> snapshot "foo", if "foo" was the last snapshot then right now the volume 
> called "foo" is actually the current contents!)
> - means we don't have to do lookups via the full snapshot list all the time 
> (e.g., if I want to do a full clone from a snapshot "foo", I can just pass 
> the snap-foo volume to qemu-img)
> 
> the naming scheme for snapshots needs to be adapted to not clash with regular 
> volumes:
> 
> $ pvesm alloc extsnap 131314 vm-131314-disk-foobar.qcow2 2G
> Formatting '/mnt/pve/ext4/extsnap/images/131314/vm-131314-disk-foobar.qcow2', 
> fmt=qcow2 cluster_size=65536 extended_l2=off preallocation=off 
> compression_type=zlib size=2147483648 lazy_refcounts=off refcount_bits=16
> successfully created 'extsnap:131314/vm-131314-disk-foobar.qcow2'
> $ qm rescan --vmid 131314
> rescan volumes...
> can't parse snapname from path at /usr/share/perl5/PVE/Storage/Plugin.pm line 
> 1934.
> 
> storage_migrate needs to handle external snapshots, or at least error out. I 
> haven't tested that part or linked clones or a lot of other advanced related 
> actions at all ;)

I'll add some more high-level comments (the threading seems to be broken for 
some reason, so I'll use this as "entrypoint"):

- snapext should probably be fixed for dir-type storages as well
- the volume ID should be static for both plugins, snapshots should be encoded 
on the storage layer in a fashion that doesn't "break through" to the API 
layers and makes it impossible to confuse the "main" volname with snapshots:
-- alloc_image shouldn't be able to allocate a volume that is then interpreted 
as snapshot
-- free_image shouldn't be able to free a snapshot volume directly
-- listing images should never return snapshots
-- ..
- for the LVM part, snapshots still require allocation a full-sized volume+some 
overhead for the qcow2 each. should we attempt to shrink them once they become 
read-only? in practice, only the LV backing the top image needs to be 
full-sized.. how do we ensure the underlying storage doesn't waste all that 
"empty" space?


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

Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot support

[Fabian Grünbichler]

> Giotta Simon RUAGH via pve-devel  hat am 
> 24.10.2024 09:59 CEST geschrieben:
> > I mean, if we don't allow .raw files to be snapshotted then this problem 
> > doesn't exist ;)
> 
> Quick comment from the bleacher; Adding a mechanism to shapshot raw disks 
> might solve the TPM (tpmstate) snapshotting issue, as well as allowing 
> containers to be snapshot.
> 
> For context: 
> When using a storage that does not natively support snapshotting (NFS on 
> NetApp or similar enterprise storage, in particular), raw disks cannot be 
> snapshot. 
> Since tpmstate disks can only be stored as raw (as I understand they are just 
> a binary blob?), this makes it impossible to snapshot or (link-)clone any VMs 
> that have a TPM. This especially is an issue for current Windows clients.
> Same issue for LXC containers, as their storage format is raw only as well.
>  
> https://bugzilla.proxmox.com/show_bug.cgi?id=4693

no it does not - with the mechanisms proposed in this patch series, only the 
initial volume can be raw, if it is snapshotted, the overlays are qcow2. so 
anything reading from the volume needs qcow2 support, including swtpm. that's 
why containers are not on the table for now either..


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

Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external snasphot support

[Fabian Grünbichler]

> DERUMIER, Alexandre  hat am 23.10.2024 
> 14:59 CEST geschrieben:
> 
>  
> Hi Fabian,
> 
> thanks for the review !
> 
> >> Message initial 
> >>De: Fabian Grünbichler 
> >>À: Proxmox VE development discussion 
> >>Cc: Alexandre Derumier 
> >>Objet: Re: [pve-devel] [PATCH v2 pve-storage 1/2] add external
> >>snasphot support
> >>Date: 23/10/2024 12:12:46
> >>
> >>some high level comments:
> >>
> >>I am not sure how much we gain here with the raw support?
> 
> They are really qcow2 overhead, mostly with big disk.
> as for good performance, the qcow2 l2-cache-size need to be keeped in
> memory (and it's 1MB by disk)
> https://events.static.linuxfound.org/sites/events/files/slides/kvm-forum-2017-slides.pdf
> 
> Hopefully, they are improvments with the "new" sub-cluster feature
> https://people.igalia.com/berto/files/kvm-forum-2020-slides.pdf
> I'm already using it at snapshot create, but I think we should also use
> it for main qcow2 volume.
> 
> 
> But even with that, you can still have performance impact.
> So yes, I think they are really usecase for workload when you only need
> snapshot time to time (before an upgrade for example), but max
> performance with no snaphot exist.

my main point here is - all other storages treat snapshots as "cheap". if you 
combine raw+qcow2 snapshot overlays, suddenly performance will get worse if you 
keep a snapshot around for whatever reason.. 
 
> >> it's a bit confusing to have a volid ending with raw, with the
> >>current volume and all but the first snapshot actually being stored
> >>in qcow2 files, with the raw file being the "oldest" snapshot in the
> >>chain..

> if it's too confusing, we could use for example an .snap extension.
> (as we known that it's qcow2 behind)

I haven't thought yet about how to encode the snapshot name into the snapshot 
file name, but yeah, maybe something like  that would be good. or maybe 
snap-VMID-disk-DISK.qcow2 ?

> if possible, I'd be much happier with the snapshot name in the snapshot
> file being a 1:1 match, see comments inline
> 
> >>- makes it a lot easier to understand (admin wants to manually remove
> >>snapshot "foo", if "foo" was the last snapshot then right now the
> >>volume called "foo" is actually the current contents!)
> 
> This part is really difficult, because you can't known in advance the
> name of the snapshot you'll take in the future. The only way could be
> to create a "current" volume ,  rename it when you took another
> snasphot (I'm not sure it's possible to do it live,
> and this could break link chain too)
> 
> Also, I don't known how to manage the main volume, when you take the
> first snapshot ? we should rename it too.

I mean, if we don't allow .raw files to be snapshotted then this problem 
doesn't exist ;)

> so "vm-disk-100-disk-0.raw|qcow2"  , become "vm-disk-100-disk-0-
> snap1.(raw|qcow2)" + new "vm-disk-100-disk-0-current.qcow2" ?

the volid changing on snapshot seems like it would require a lot of adaption.. 
OTOH, the volid containing a wrong format might also break things.

> I'll try to do test again to see what is possible.
> 
> 
> 
> 
> >>- means we don't have to do lookups via the full snapshot list all
> >>the time (e.g., if I want to do a full clone from a snapshot "foo", I
> >>can just pass the snap-foo volume to qemu-img)
> 
> ok got it
> 
> 
> 
> >>the naming scheme for snapshots needs to be adapted to not clash with
> >>regular volumes:
> 
> >>$ pvesm alloc extsnap 131314 vm-131314-disk-foobar.qcow2 2G
> >>Formatting '/mnt/pve/ext4/extsnap/images/131314/vm-131314-disk-
> >>foobar.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off
> >>preallocation=off compression_type=zlib size=2147483648
> >>lazy_refcounts=off refcount_bits=16
> >>successfully created 'extsnap:131314/vm-131314-disk-foobar.qcow2'
> >>$ qm rescan --vmid 131314
> >>rescan volumes...
> >>can't parse snapname from path at
> >>/usr/share/perl5/PVE/Storage/Plugin.pm line 1934.
> 
> any preference for naming scheme ? for lvm external snap, I have used
> "vm-131314-disk-0-snap-";

see above

> >>storage_migrate needs to handle external snapshots, or at least error
> >>out. 
> it should already work. (I have tested move_disk, and live migration +
> storage migration). qemu_img_convert offline and qemu block job for
> live.

but don't all of those lose the snapshots? did you test it with snapshots and 
rollback afterwards?

> >>I haven't tested that part or linked clones or a lot of other
> >>advanced related actions at all ;)
> 
> For linked clone, we can't have a base image with snapshots (other than
> _base_). so It'll be safe.

ack

> > Alexandre Derumier via pve-devel  hat am
> > 30.09.2024 13:31 CEST geschrieben:
> > Signed-off-by: Alexandre Derumier  > cyllene.com>
> > ---
> >  src/PVE/Storage/DirPlugin.pm |   1 +
> >  src/PVE/Storage/Plugin.pm    | 225 +++--
> > --
> >  2 files changed, 201 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/PVE/Storage/DirPlugin.