On Fri, Apr 13 2018 10:33:20 +0200, Wolfgang Bumiller wrote:
> Code/style issues aside, in the current shape this is a
> convenience-driven upstream API change for a specific use case /
> personal preference.
Fair enough. It was a simple hack of to solve the issue without large
design changes. But I will add that fixing the underlying issue is *not*
convenience-driven nor would I really call it personal preference: using
only integers to identify things and then reusing those same integers
for different instances of things is frankly bad design. I hope I've at
least brought that to your attention.
> I could imagine having an opt-in change to nextid, but it would have to
> adhere to our coding style (and use file_get/set_contents, correct
> locking to avoid races etc.), and would have to fix the shortcomings
> mentioned in the thread already. (One possibility would be to track a
> list/bitmap of actually used IDs to avoid the "allocating 99999 once"
> issue, and stick to the previous behavior if no such file was
> initialized. Another possibility would be making it call out to a helper
> script if one exists - not sure what the others think about this - but
> the default/main behavior should not be changed.)
There have been no concrete suggestions as to which changes I should
make in order to get this merged, so I am dropping it. We'll continue
running with these patches locally and hope that you recognize the
issues that ID reuse bring (and perhaps solve them in a way you see fit
at some point).
pve-devel mailing list