Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Wed, Sep 19, 2018 at 07:09:24PM +0200, Paolo Bonzini wrote: > On 19/09/2018 18:36, Eduardo Habkost wrote: > > On Tue, Sep 18, 2018 at 05:35:20PM +0200, Paolo Bonzini wrote: > >> On 18/09/2018 16:22, Eduardo Habkost wrote: > >>> On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote: > On 18/09/2018 15:14, Eduardo Habkost wrote: > > If it broke something, we should restore the option names and > > declare them as deprecated. > > I think in this particular case it's okay to add them back as no-ops, > especially we'd actually want them to be customizable for user-mode > emulation. > >>> > >>> We can make the flag work on user-mode emulation, but I wouldn't > >>> like to keep QEMU silent if the option was explicitly set in the > >>> command-line in system emulation, because it means the user or > >>> some software component is really confused and trying to do > >>> something that is never going to work. > >> > >> They just want to copy the host flags blindly into the guest. osxsave > >> and ospke require some collaboration from the guest OS, but it should be > >> pretty harmless to have them on the command line, because in the end the > >> apps _should_ anyway be checking OSPKE. If somebody is writing such an > >> app and is puzzled that OSPKE is missing, they should first of all ask > >> themselves if they have installed the right guest OS. > > > > I wouldn't say that a no-op option that would just confuse > > apps/users is harmless. I really don't see any benefit in > > keeping it. > > But how would it be confusing? It's perhaps worth warning, but that's it. If we have a reason to warn users, it qualifies as confusing to me. Anyway, this specific case is not a big deal. But I would say that our tendency to needlessly sacrifice maintainability for the sake of compatibility is a big problem in the long run. -- Eduardo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On 19/09/2018 18:36, Eduardo Habkost wrote: > On Tue, Sep 18, 2018 at 05:35:20PM +0200, Paolo Bonzini wrote: >> On 18/09/2018 16:22, Eduardo Habkost wrote: >>> On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote: On 18/09/2018 15:14, Eduardo Habkost wrote: > If it broke something, we should restore the option names and > declare them as deprecated. I think in this particular case it's okay to add them back as no-ops, especially we'd actually want them to be customizable for user-mode emulation. >>> >>> We can make the flag work on user-mode emulation, but I wouldn't >>> like to keep QEMU silent if the option was explicitly set in the >>> command-line in system emulation, because it means the user or >>> some software component is really confused and trying to do >>> something that is never going to work. >> >> They just want to copy the host flags blindly into the guest. osxsave >> and ospke require some collaboration from the guest OS, but it should be >> pretty harmless to have them on the command line, because in the end the >> apps _should_ anyway be checking OSPKE. If somebody is writing such an >> app and is puzzled that OSPKE is missing, they should first of all ask >> themselves if they have installed the right guest OS. > > I wouldn't say that a no-op option that would just confuse > apps/users is harmless. I really don't see any benefit in > keeping it. But how would it be confusing? It's perhaps worth warning, but that's it. Paolo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Tue, Sep 18, 2018 at 05:35:20PM +0200, Paolo Bonzini wrote: > On 18/09/2018 16:22, Eduardo Habkost wrote: > > On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote: > >> On 18/09/2018 15:14, Eduardo Habkost wrote: > >>> If it broke something, we should restore the option names and > >>> declare them as deprecated. > >> > >> I think in this particular case it's okay to add them back as no-ops, > >> especially we'd actually want them to be customizable for user-mode > >> emulation. > > > > We can make the flag work on user-mode emulation, but I wouldn't > > like to keep QEMU silent if the option was explicitly set in the > > command-line in system emulation, because it means the user or > > some software component is really confused and trying to do > > something that is never going to work. > > They just want to copy the host flags blindly into the guest. osxsave > and ospke require some collaboration from the guest OS, but it should be > pretty harmless to have them on the command line, because in the end the > apps _should_ anyway be checking OSPKE. If somebody is writing such an > app and is puzzled that OSPKE is missing, they should first of all ask > themselves if they have installed the right guest OS. I wouldn't say that a no-op option that would just confuse apps/users is harmless. I really don't see any benefit in keeping it. I would agree with keeping them if it avoided additional complexity on the app side. But if an app is copying host flags to the QEMU command-line, the app should be already aware that only a subset of CPUID flags is configurable in QEMU. The only reason I can see for keeping the options is compatibility with command-lines generated by current versions of apps/libvirt. But that's exactly why we agreed on a deprecation policy, isn't it? -- Eduardo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On 18/09/2018 16:22, Eduardo Habkost wrote: > On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote: >> On 18/09/2018 15:14, Eduardo Habkost wrote: >>> If it broke something, we should restore the option names and >>> declare them as deprecated. >> >> I think in this particular case it's okay to add them back as no-ops, >> especially we'd actually want them to be customizable for user-mode >> emulation. > > We can make the flag work on user-mode emulation, but I wouldn't > like to keep QEMU silent if the option was explicitly set in the > command-line in system emulation, because it means the user or > some software component is really confused and trying to do > something that is never going to work. They just want to copy the host flags blindly into the guest. osxsave and ospke require some collaboration from the guest OS, but it should be pretty harmless to have them on the command line, because in the end the apps _should_ anyway be checking OSPKE. If somebody is writing such an app and is puzzled that OSPKE is missing, they should first of all ask themselves if they have installed the right guest OS. Paolo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote: > On 18/09/2018 15:14, Eduardo Habkost wrote: > > If it broke something, we should restore the option names and > > declare them as deprecated. > > I think in this particular case it's okay to add them back as no-ops, > especially we'd actually want them to be customizable for user-mode > emulation. We can make the flag work on user-mode emulation, but I wouldn't like to keep QEMU silent if the option was explicitly set in the command-line in system emulation, because it means the user or some software component is really confused and trying to do something that is never going to work. -- Eduardo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On 18/09/2018 15:14, Eduardo Habkost wrote: > If it broke something, we should restore the option names and > declare them as deprecated. I think in this particular case it's okay to add them back as no-ops, especially we'd actually want them to be customizable for user-mode emulation. Paolo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Tue, Sep 18, 2018 at 03:26:18PM +0200, Jiri Denemark wrote: > On Tue, Sep 18, 2018 at 10:14:45 -0300, Eduardo Habkost wrote: > > On Tue, Sep 18, 2018 at 03:07:35PM +0200, Jiri Denemark wrote: > > > Sure, libvirt could just avoid passing feature=off for any feature which > > > is not supported by the QEMU binary it is about to start since such > > > feature should be disabled anyway. And if it actually is enabled even if > > > it is not supported (which can apparently happen according to the commit > > > message which removed ospke), we can at least rely on QEMU doing it > > > consistently. But what should we do if the user explicitly asked for the > > > feature to be enabled, QEMU enabled it, and suddenly the feature is not > > > supported by new QEMU? > > > > I incorrectly assumed that nobody would be using the option > > names, because the options did nothing (ospke and osxsave are > > runtime CPU state flags exposed via CPUID, and were never > > configurable from the command-line). > > > > If it broke something, we should restore the option names and > > declare them as deprecated. > > > > > > > > Any ideas on how we should deal with the two features which were removed > > > from QEMU 3.0 and how to make sure we don't have to discuss this again > > > when some other feature is removed? > > > > Removing the flags without notice was a mistake. Next time, we > > should follow the deprecation policy and communicate the option > > removal more clearly. > > Fair enough, however once they are declared as deprecated they will > actually be removed at some point. And we're back in this situation. > Deprecating or removing command line options or QMP commands is > different, because libvirt can detect what is (un)supported and use an > alternative command or option. > > But if a CPU feature is removed (unfortunately it doesn't really matter > if it was doing something or not), we need to have a plan what to do in > a situation when the feature is used in domain XML. We can detect the > feature is not supported, but does it mean the feature was didn't have > any effect and thus it was removed or are we just talking to a QEMU > binary which does not support the feature (yet)? In this specific case, libvirt needs to encode the knowledge that it was using an option that had zero effects on the resulting virtual machine. When QEMU provides an alternative, this is also easy to deal with: we need to teach libvirt to use the alternative. Now, if we're going to remove actual features from QEMU without providing a replacement, this is a more complex discussion. I was planning to start a thread about that, because we need to be able to remove features from QEMU in the future (esp. CPU models and machine-types) even if existing VMs are using them, and libvirt and management apps need to deal with that somehow. -- Eduardo
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Tue, Sep 18, 2018 at 10:14:45 -0300, Eduardo Habkost wrote: > On Tue, Sep 18, 2018 at 03:07:35PM +0200, Jiri Denemark wrote: > > Sure, libvirt could just avoid passing feature=off for any feature which > > is not supported by the QEMU binary it is about to start since such > > feature should be disabled anyway. And if it actually is enabled even if > > it is not supported (which can apparently happen according to the commit > > message which removed ospke), we can at least rely on QEMU doing it > > consistently. But what should we do if the user explicitly asked for the > > feature to be enabled, QEMU enabled it, and suddenly the feature is not > > supported by new QEMU? > > I incorrectly assumed that nobody would be using the option > names, because the options did nothing (ospke and osxsave are > runtime CPU state flags exposed via CPUID, and were never > configurable from the command-line). > > If it broke something, we should restore the option names and > declare them as deprecated. > > > > > Any ideas on how we should deal with the two features which were removed > > from QEMU 3.0 and how to make sure we don't have to discuss this again > > when some other feature is removed? > > Removing the flags without notice was a mistake. Next time, we > should follow the deprecation policy and communicate the option > removal more clearly. Fair enough, however once they are declared as deprecated they will actually be removed at some point. And we're back in this situation. Deprecating or removing command line options or QMP commands is different, because libvirt can detect what is (un)supported and use an alternative command or option. But if a CPU feature is removed (unfortunately it doesn't really matter if it was doing something or not), we need to have a plan what to do in a situation when the feature is used in domain XML. We can detect the feature is not supported, but does it mean the feature was didn't have any effect and thus it was removed or are we just talking to a QEMU binary which does not support the feature (yet)? Jirka
Re: [Qemu-devel] Dropped CPU feature names and backward compatibility
On Tue, Sep 18, 2018 at 03:07:35PM +0200, Jiri Denemark wrote: > Hi, > > I noticed two x86_64 CPU features were removed from QEMU in 3.0.0: > - ospke removed by 9ccb9784b57 > - osxsave removed by f1a23522b03 > > More precisely, the CPUID bits are still there (and for example Icelake > CPU model has the ospke bit set), but the string representations were > removed. Thus QEMU will no longer report there features and it will not > accept it on the command line anymore. Which is a regression which we > need to deal with somehow. > > With QEMU < 3.0: > > $ qemu-system-x86_64 -machine pc,accel=kvm -cpu Skylake-Client,osxsave=on > qemu-system-x86_64: warning: host doesn't support requested feature: > CPUID.01H:ECX.osxsave [bit 27] > > $ qemu-system-x86_64 -machine pc,accel=kvm -cpu Skylake-Client,osxsave=off > > > While new QEMU 3.0 complains: > > $ qemu-system-x86_64 -machine pc,accel=kvm -cpu Skylake-Client,osxsave=off > qemu-system-x86_64: can't apply global Skylake-Client-x86_64-cpu.osxsave=off: > Property '.osxsave' not found > > > I accept the argument that setting those two features was never actually > allowed, but that doesn't mean they should be just removed. It's quite > likely that somebody has a libvirt domain defined with either of the two > features explicitly enabled or disabled. It's likely because both > virt-manager and virt-install can be told to copy host CPU configuration > to the domain XML. This is normally not a problem, because it will just > ask QEMU to enable all features available on the host and QEMU will > enable only some of them. To make sure libvirt can provide stable guest > CPU ABI after migration or save/restore, we ask QEMU what CPU features > were enabled and disabled and update the active definition. This means > that an explicit will likely > turn into . In other words, > the -cpu option will have an explicit osxsave=off parameter during > migration. As a result of this such domain would happily migrate to QEMU > < 3.0, but fail to migrate to QEMU 3.0. > > Sure, libvirt could just avoid passing feature=off for any feature which > is not supported by the QEMU binary it is about to start since such > feature should be disabled anyway. And if it actually is enabled even if > it is not supported (which can apparently happen according to the commit > message which removed ospke), we can at least rely on QEMU doing it > consistently. But what should we do if the user explicitly asked for the > feature to be enabled, QEMU enabled it, and suddenly the feature is not > supported by new QEMU? I incorrectly assumed that nobody would be using the option names, because the options did nothing (ospke and osxsave are runtime CPU state flags exposed via CPUID, and were never configurable from the command-line). If it broke something, we should restore the option names and declare them as deprecated. > > Any ideas on how we should deal with the two features which were removed > from QEMU 3.0 and how to make sure we don't have to discuss this again > when some other feature is removed? Removing the flags without notice was a mistake. Next time, we should follow the deprecation policy and communicate the option removal more clearly. -- Eduardo
