On Tue, 12 Jan 2021 18:41:38 +0000 Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > On Tue, Jan 12, 2021 at 07:28:45PM +0100, Peter Krempa wrote: > > On Tue, Jan 12, 2021 at 19:20:58 +0100, Igor Mammedov wrote: > > > On Tue, 12 Jan 2021 12:35:19 +0100 > > > Peter Krempa <pkrempa@xxxxxxxxxx> wrote: > > > > > > > On Tue, Jan 12, 2021 at 12:29:58 +0100, Michal Privoznik wrote: > > > > > On 1/12/21 12:19 PM, Peter Krempa wrote: > > > > > > On Tue, Jan 12, 2021 at 09:29:49 +0100, Michal Privoznik wrote: > > > > > > > This capability tracks whether memory-backend-file has > > > > > > > "x-use-canonical-path-for-ramblock-id" attribute. Introduced into > > > > > > > QEMU by commit v4.0.0-rc0~189^2. While "x-" prefix is considered > > > > > > > > > > > > Please use a commit hash instead of this. > > > > > > > > > > > > > experimental or internal to QEMU, the next commit justifies its > > > > > > > use. > > > > > > > > > > > > NACK unless qemu adds a statement to their code and documentation that > > > > > > the this property is considered stable despite the 'x-prefix' and you > > > > > > add a link to the appropriate qemu upstream commit once it's done. > > > > > > > > > > > > We don't want to depend on experimental stuff so we need a strong > > > > > > excuse. > > > > > > > > > > > > > > > > That's done in the next commit. Do you want me to copy it here too? I > > > > > figured I'd put the justification where I'm actually setting the internal > > > > > knob. > > > > > > > > Yes, because this is also mentioning the an 'x-' prefixed property. I > > > > want to be absolutely clear in any places (including a comment in the > > > > code, which you also should add into the capability code) that this is > > > > extraordinary circumstance and that qemu is actually considering that > > > > property stable. > > > > > > the only reason to keep x- prefix in this case is to cause less issues for > > > downstream QEMUs. Since this compat property is copied to their own machine types. > > > If we keep prefix downstream doesn't have to do anything, if we rename it, > > > then downstreams have to carry a separate patch that does the same for > > > their old machine types. > > > > That would be okay if it's limited to past versions, but in this > > instance it is not. Allowing x-prefixed properties for any future > > release is a dangerous precedent. If we want to allow to detect the > > capability also for future release, we must declare that it's for a very > > particular reason and also that qemu will not delete it at will. > > > > This is to prevent any future discussions of unwaranted usage of > > x-prefixed properties in libvirt. > > Yeah it is pretty dubious on the QEMU side to have used an "x-" prefix > here at all, when use of this option is mandatory to make migration > work :-( if generic consensus is to drop prefix, I can post a QEMU patch to do so and let downstream(s) to carry burden. > > Regards, > Daniel
