[pve-devel] avoiding VMID reuse

[Lauri Tirkkonen]

On Thu, Apr 12 2018 12:42:51 +0200, Fabian Grünbichler wrote:
> > - please send patch series as threads (cover letter and each patch as
> >   separate mail) and configure the subjectprefix accordingly for each
> >   repository. this allows inline comments on each patch.

Ok. This particular patchset isn't a series per se but 1 patch each for
4 different repositories, I was kinda wondering how you wanted to deal
with that. Subject-prefix answers that I suppose, but it means four
threads for this change instead of just one.

> > if I understood you correctly, your intended use case is to prevent
> > accidentally re-using a guest ID formerly used by a no longer existing
> > guest, because of backups / replicated and/or leftover disks that
> > reference this ID?

Yes, that's correct.

> > I assume you have some kind of client / script for managing guests,
> > since we don't call /cluster/nextid in our GUI or anywhere else.
> 
> Dominik just pointed out to me that we do in fact use it in the JS code
> (I only checked the perl code). sorry for the confusion.
> 
> > wouldn't the simple solution be to keep track of "already used" guest
> > IDs in your client? especially if you only have single nodes?
> > alternatively, you could just not use /cluster/nextid and instead use
> > your own counter (and increment and retry in case the chosen ID is
> > already taken - /cluster/nextid does not guarantuee it will still be
> > available when you want to use it anyway..).

Sure, it's not a guarantee (because it isn't an error to use an unused
ID less than nextid -- it would be easy to convert the warning to an
error though). But we don't especially need it to be a guarantee, we
just want casual web interface use to not end us up in a situation where
backups break or data is lost, so it's enough to just fix the suggestion
made by the web interface (which is what /cluster/nextid does
precisely).

> > another approach would be to adapt your snapshot/sync scripts to remove
> > sync targets if the source gets removed, or do a forceful full sync if
> > an ID gets re-used. the latter is how PVE's builtin ZFS replication
> > works if it fails to find a snapshot combination that allows incremental
> > sending.

That sounds super dangerous. If I delete a VM and then someone creates a
new one that now gets the same ID, I also lose all backups of my deleted
VM!

> > I am a bit hesitant to introduce such special case heuristics,
> > especially since we don't know if anybody relies on the current
> > semantics of /cluster/nextid
> 
> that point still stands though ;)

I didn't make this configurable, because I don't really see how someone
could be relying on id's getting reused (unless there's an upper limit
to id numbers that could be argued to be reachable).