On 06/15/2018 04:21 PM, Kevin Wolf wrote: > The -drive option serial was deprecated in QEMU 2.10. It's time to > remove it. > > Tests need to be updated to set the serial number with -global instead > of using the -drive option. libvirt 4.5 still creates those (at least on s390x) <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> <target dev='hda' bus='virtio'/> <serial>skel</serial> <boot order='1'/> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> </disk> -> [...] -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on [...] 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' 2018-06-22 11:25:21.098+0000: shutting down, reason=failed So it seems that this breaks s390x.

On 06/15/2018 04:21 PM, Kevin Wolf wrote: > The -drive option serial was deprecated in QEMU 2.10. It's time to > remove it. > > Tests need to be updated to set the serial number with -global instead > of using the -drive option. libvirt 4.5 still creates those (at least on s390x) <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> <target dev='hda' bus='virtio'/> <serial>skel</serial> <boot order='1'/> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> </disk> -> [...] -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on [...] 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' 2018-06-22 11:25:21.098+0000: shutting down, reason=failed So it seems that this breaks s390x.

On 06/22/2018 02:55 PM, Kevin Wolf wrote: > Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: >> >> On 06/15/2018 04:21 PM, Kevin Wolf wrote: >>> The -drive option serial was deprecated in QEMU 2.10. It's time to >>> remove it. >>> >>> Tests need to be updated to set the serial number with -global instead >>> of using the -drive option. >> >> libvirt 4.5 still creates those (at least on s390x) >> >> <disk type='file' device='disk'> >> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> >> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> >> <target dev='hda' bus='virtio'/> >> <serial>skel</serial> >> <boot order='1'/> >> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> >> </disk> >> >> >> -> >> [...] >> -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on >> [...] >> >> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' >> 2018-06-22 11:25:21.098+0000: shutting down, reason=failed >> >> So it seems that this breaks s390x. To me it seems that this is also broken on x86. > > Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is > released. I think this is definitely too short notice. We should not break existing setups just by insisting that users have to update libvirt when they update QEMU. Yes, this might be our policy, but doing so "just because we can" is certainly a very bad attitude. I see no fundamental technical reason why we should not revert this change. > > Sadly, it also shows that deprecation warnings in log files go > unnoticed. In fact whoever added the deprication notice should have followed up with the libvirt team to implement that change. no?

On 06/22/2018 02:55 PM, Kevin Wolf wrote: > Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: >> >> On 06/15/2018 04:21 PM, Kevin Wolf wrote: >>> The -drive option serial was deprecated in QEMU 2.10. It's time to >>> remove it. >>> >>> Tests need to be updated to set the serial number with -global instead >>> of using the -drive option. >> >> libvirt 4.5 still creates those (at least on s390x) >> >> <disk type='file' device='disk'> >> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> >> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> >> <target dev='hda' bus='virtio'/> >> <serial>skel</serial> >> <boot order='1'/> >> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> >> </disk> >> >> >> -> >> [...] >> -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on >> [...] >> >> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' >> 2018-06-22 11:25:21.098+0000: shutting down, reason=failed >> >> So it seems that this breaks s390x. To me it seems that this is also broken on x86. > > Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is > released. I think this is definitely too short notice. We should not break existing setups just by insisting that users have to update libvirt when they update QEMU. Yes, this might be our policy, but doing so "just because we can" is certainly a very bad attitude. I see no fundamental technical reason why we should not revert this change.

> Sadly, it also shows that deprecation warnings in log files go > unnoticed. In fact whoever added the deprication notice should have followed up with the libvirt team to implement that change. no?

On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote: > Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben: > > > > > > On 06/22/2018 02:55 PM, Kevin Wolf wrote: > > > Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: > > >> > > >> On 06/15/2018 04:21 PM, Kevin Wolf wrote: > > >>> The -drive option serial was deprecated in QEMU 2.10. It's time to > > >>> remove it. > > >>> > > >>> Tests need to be updated to set the serial number with -global instead > > >>> of using the -drive option. > > >> > > >> libvirt 4.5 still creates those (at least on s390x) > > >> > > >> <disk type='file' device='disk'> > > >> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> > > >> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> > > >> <target dev='hda' bus='virtio'/> > > >> <serial>skel</serial> > > >> <boot order='1'/> > > >> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> > > >> </disk> > > >> > > >> > > >> -> > > >> [...] > > >> -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on > > >> [...] > > >> > > >> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' > > >> 2018-06-22 11:25:21.098+0000: shutting down, reason=failed > > >> > > >> So it seems that this breaks s390x. > > > > To me it seems that this is also broken on x86. > > > > > > Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is > > > released. > > > > I think this is definitely too short notice. We should not break existing > > setups just by insisting that users have to update libvirt when they update > > QEMU. Yes, this might be our policy, but doing so "just because we can" > > is certainly a very bad attitude. I see no fundamental technical reason why > > we should not revert this change. > > This was in fact one release longer than our deprecation policy says. > Are we serious about the deprecation policy or aren't we? > > I might consider reverting a change if it turned out that this requires > some massive work in libvirt. But I think this one should be rather easy > to fix in libvirt until 3.0 is released. It is probably even possible for us to fix it in our July 1st release

Am 25.06.2018 um 11:53 hat Daniel P. Berrangé geschrieben: > On Fri, Jun 22, 2018 at 03:31:46PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote: > > > Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben:

Thanks! I'll look into werror/rerror support for usb-storage. It shouldn't be too hard, though it's strictly speaking a separate problem related to using -blockdev rather than option deprecation. If Peter wants to wait for QEMU support before converting werror/rerror

to -device, maybe it would make sense to split your patch for v2 so that geometry and serial can get fixed right away?

Am 25.06.2018 um 13:45 hat Peter Krempa geschrieben: > On Mon, Jun 25, 2018 at 13:41:06 +0200, Kevin Wolf wrote: >> Am 25.06.2018 um 11:53 hat Daniel P. Berrangé geschrieben: >>> On Fri, Jun 22, 2018 at 03:31:46PM +0100, Daniel P. Berrangé wrote: >>>> On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote: >>>>> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben: > > [...] > >> >> Thanks! >> >> I'll look into werror/rerror support for usb-storage. It shouldn't be >> too hard, though it's strictly speaking a separate problem related to >> using -blockdev rather than option deprecation. >> >> If Peter wants to wait for QEMU support before converting werror/rerror > > Definitely. I don't want to keep around yet another hack that will > satisfy one specific case and then add another capability for it. We > should then gate the moving of the feature based on the presence of > werror for usb-storage. > >> to -device, maybe it would make sense to split your patch for v2 so that >> geometry and serial can get fixed right away? > > Yes this can be done right away. Has serial/gemoetry been fixed meanwhile and will it make it into the next release?

On Tue, Jul 03, 2018 at 12:53:44PM +0200, Christian Borntraeger wrote: > > > On 07/02/2018 10:04 AM, Kevin Wolf wrote: > > Am 25.06.2018 um 13:45 hat Peter Krempa geschrieben: > >> On Mon, Jun 25, 2018 at 13:41:06 +0200, Kevin Wolf wrote: > >>> Am 25.06.2018 um 11:53 hat Daniel P. Berrangé geschrieben: > >>>> On Fri, Jun 22, 2018 at 03:31:46PM +0100, Daniel P. Berrangé wrote: > >>>>> On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote: > >>>>>> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben: > >> > >> [...] > >> > >>> > >>> Thanks! > >>> > >>> I'll look into werror/rerror support for usb-storage. It shouldn't be > >>> too hard, though it's strictly speaking a separate problem related to > >>> using -blockdev rather than option deprecation. > >>> > >>> If Peter wants to wait for QEMU support before converting werror/rerror > >> > >> Definitely. I don't want to keep around yet another hack that will > >> satisfy one specific case and then add another capability for it. We > >> should then gate the moving of the feature based on the presence of > >> werror for usb-storage. > >> > >>> to -device, maybe it would make sense to split your patch for v2 so that > >>> geometry and serial can get fixed right away? > >> > >> Yes this can be done right away. > > > > Has serial/gemoetry been fixed meanwhile and will it make it into the > > next release? > > I cannot find an archive that has it, but it is on the libvirt mailing > list as "[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk device". > Review seems done, but it has missed libvirt 4.5 which was released today. Just posted latest version here: https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html It will be in the next release on ~ Aug 1st

On 3 July 2018 at 12:32, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > Am 03.07.2018 um 13:22 hat Daniel P. Berrangé geschrieben: >> Just posted latest version here: >> >> https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html >> >> It will be in the next release on ~ Aug 1st > > It would have been a lot nicer to have it the July release because this > means that we'll have the released libvirt broken during almost the > whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the > earliest, so I guess we're still okay. People using QEMU from git will > just need libvirt from git as well. I'm still not clear what we gain from having a QEMU that's dropped a feature that is still used by everything except leading-edge not-yet-released versions of libvirt. Is there a strong reason we can't just revert the deletion of the deprecated feature for a QEMU release or two?

> > > Has serial/gemoetry been fixed meanwhile and will it make it into the > > > next release? > > > > I cannot find an archive that has it, but it is on the libvirt mailing > > list as "[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk device". > > Review seems done, but it has missed libvirt 4.5 which was released today. > > Just posted latest version here: > > https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html > > It will be in the next release on ~ Aug 1st It would have been a lot nicer to have it the July release because this means that we'll have the released libvirt broken during almost the whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the earliest, so I guess we're still okay. People using QEMU from git will just need libvirt from git as well.

Am 04.07.2018 um 15:02 hat Cornelia Huck geschrieben: > On Tue, 3 Jul 2018 13:32:29 +0200 > Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > > > > > > Has serial/gemoetry been fixed meanwhile and will it make it into the > > > > > next release? > > > > > > > > I cannot find an archive that has it, but it is on the libvirt mailing > > > > list as "[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk device". > > > > Review seems done, but it has missed libvirt 4.5 which was released today. > > > > > > Just posted latest version here: > > > > > > https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html > > > > > > It will be in the next release on ~ Aug 1st > > > > It would have been a lot nicer to have it the July release because this > > means that we'll have the released libvirt broken during almost the > > whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the > > earliest, so I guess we're still okay. People using QEMU from git will > > just need libvirt from git as well. > > Speaking as an innocent* bystander: > > I would usually presume that I can use any recent libvirt to test > current QEMU, even bleeding edge. In this case, not even the latest > released libvirt version will be fine, I would also need to build > libvirt from git (which is probably not something a non-libvirt > developer will usually do). If everything goes according to plan, I can > only test QEMU with a released libvirt version at the very tail end of > hardfreeze, where only release blockers are appropriate. I understand where you're coming from, but let's be honest: It's not as if disk geometry or serial numbers were features that absolutely need to be there to give QEMU any testing.

Also, my understanding has always been that we expect users to have a libvirt version that isn't older than QEMU. It would be useful to set a clear policy for this and document it.

> I think it would be really beneficial to general QEMU test coverage to > push deleting this option back a release or two. We should make testing > QEMU in conjunction with libvirt as uncomplicated as possible. Essentially, what is important to me isn't getting these options dropped exactly in 3.0, but not setting a bad precedence that deprecation isn't actually worth anything. We may easily end up with this deprecation process: depreate a feature release QEMU version n + 1 release QEMU version n + 2 remove the feature while libvirt hasn't removed use of the feature: # ...and why should it when everything is still working? reinstate the feature release QEMU version n + x remove the feature

When management tools know that this is the process, the motivation to remove the use of the feature gets even lower (not out of malice, but because there will be always more important things), so this will "optimise" itself into an endless loop and we're back to never actually removing old cruft that impedes development. The libvirt patch has just been merged (and I'm almost sure that this wouldn't have happened so quickly if I had just reverted the patch right away), so at least we know now that this specific instance of the loop is going to terminate. What's left is first and foremost that we need to sort out our broken deprecation mechanism, and if that gets done, I don't mind if someone wants to revert the patch for 3.0 as long as they also take care that it gets back into 3.1.

Am 04.07.2018 um 15:02 hat Cornelia Huck geschrieben: > On Tue, 3 Jul 2018 13:32:29 +0200 > Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > >>>>> Has serial/gemoetry been fixed meanwhile and will it make it into the >>>>> next release? >>>> >>>> I cannot find an archive that has it, but it is on the libvirt mailing >>>> list as "[libvirt] [PATCH v3] qemu: format serial and geometry on frontend disk device". >>>> Review seems done, but it has missed libvirt 4.5 which was released today. >>> >>> Just posted latest version here: >>> >>> https://www.redhat.com/archives/libvir-list/2018-July/msg00130.html >>> >>> It will be in the next release on ~ Aug 1st >> >> It would have been a lot nicer to have it the July release because this >> means that we'll have the released libvirt broken during almost the >> whole rc phase of QEMU 3.0, but the release is planned for Aug 8th the >> earliest, so I guess we're still okay. People using QEMU from git will >> just need libvirt from git as well. > > Speaking as an innocent* bystander: > > I would usually presume that I can use any recent libvirt to test > current QEMU, even bleeding edge. In this case, not even the latest > released libvirt version will be fine, I would also need to build > libvirt from git (which is probably not something a non-libvirt > developer will usually do). If everything goes according to plan, I can > only test QEMU with a released libvirt version at the very tail end of > hardfreeze, where only release blockers are appropriate. I understand where you're coming from, but let's be honest: It's not as if disk geometry or serial numbers were features that absolutely need to be there to give QEMU any testing.

Also, my understanding has always been that we expect users to have a libvirt version that isn't older than QEMU. It would be useful to set a clear policy for this and document it. > I think it would be really beneficial to general QEMU test coverage to > push deleting this option back a release or two. We should make testing > QEMU in conjunction with libvirt as uncomplicated as possible. Essentially, what is important to me isn't getting these options dropped exactly in 3.0, but not setting a bad precedence that deprecation isn't actually worth anything. We may easily end up with this deprecation process: depreate a feature release QEMU version n + 1 release QEMU version n + 2 remove the feature while libvirt hasn't removed use of the feature: # ...and why should it when everything is still working? reinstate the feature release QEMU version n + x remove the feature When management tools know that this is the process, the motivation to remove the use of the feature gets even lower (not out of malice, but because there will be always more important things), so this will "optimise" itself into an endless loop and we're back to never actually removing old cruft that impedes development.

The libvirt patch has just been merged (and I'm almost sure that this wouldn't have happened so quickly if I had just reverted the patch right away), so at least we know now that this specific instance of the loop is going to terminate. What's left is first and foremost that we need to sort out our broken deprecation mechanism, and if that gets done, I don't mind if someone wants to revert the patch for 3.0 as long as they also take care that it gets back into 3.1.

Essentially, what is important to me isn't getting these options dropped exactly in 3.0, but not setting a bad precedence that deprecation isn't actually worth anything. We may easily end up with this deprecation process: depreate a feature release QEMU version n + 1 release QEMU version n + 2 remove the feature while libvirt hasn't removed use of the feature: # ...and why should it when everything is still working? reinstate the feature release QEMU version n + x remove the feature

On Wed, 4 Jul 2018 17:14:02 +0100 Peter Maydell <peter.maydell(a)linaro.org&gt; wrote: > On 4 July 2018 at 14:34, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > > Essentially, what is important to me isn't getting these options dropped > > exactly in 3.0, but not setting a bad precedence that deprecation isn't > > actually worth anything. We may easily end up with this deprecation > > process: > > > > depreate a feature > > release QEMU version n + 1 > > release QEMU version n + 2 > > remove the feature > > while libvirt hasn't removed use of the feature: > > # ...and why should it when everything is still working? > > reinstate the feature > > release QEMU version n + x > > remove the feature > > My take on the deprecation policy essentially is that it gives > us a *minimum* bar for how soon we can drop something. We > shouldn't be using it as an "always target this speed for > dropping something" -- we ought to be pragmatic. We can > drop stuff that's unused quickly, but should be slower > for things that still have major users (or reconsider > the deprecation entirely, potentially). There should be > a balance between making our work as developers easier and > inconveniencing our users. What about the following? - put a feature on the "normal" deprecation list to remove after two releases Case (a): nobody complains, either within the deprecation period or when it is finally removed -> all is good Case (b): the feature turns out to be widely used, and/or it turns out that it offers value that currently can't be offered easily in another way -> remove from deprecation list; this obviously needs more thinking Case (c): the feature is used, the users are willing to move away from it, but they need a bit more time -> put it on a "deprecation watchlist", listing the users we are waiting for, and then remove after all are done (no +2) That way, we can still easily remove old cruft (case (a)), but still accommodate cases like this (case (c)). The obvious drawback is that we'd need someone to curate the deprecation watchlist, to poke the users we're waiting for, and probably remove anyway after some time if they don't get their act together.

Am 06.07.2018 um 13:11 hat Cornelia Huck geschrieben: > That way, we can still easily remove old cruft (case (a)), but still > accommodate cases like this (case (c)). The obvious drawback is that > we'd need someone to curate the deprecation watchlist, to poke the > users we're waiting for, and probably remove anyway after some time if > they don't get their act together. The problem is that things are only starting to move after two releases have passed.

Peter Maydell <peter.maydell(a)linaro.org&gt; writes: > On 6 July 2018 at 15:56, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: >> Am 06.07.2018 um 13:11 hat Cornelia Huck geschrieben: >>> That way, we can still easily remove old cruft (case (a)), but still >>> accommodate cases like this (case (c)). The obvious drawback is that >>> we'd need someone to curate the deprecation watchlist, to poke the >>> users we're waiting for, and probably remove anyway after some time if >>> they don't get their act together. >> >> The problem is that things are only starting to move after two releases >> have passed. > > Right, so clearly just "put a note in the documentation" isn't > sufficient advertisement/prodding of things going away. Yes. Ideas on more forceful notification have been tossed around, we just have to act on them. > (Also, two > releases is pretty fast. Many of our users will be using distro > packaged versions of QEMU which will lag further behind than > bleeding-edge users. The system version of QEMU on my desktop > machine is 2.5...) If you consume QEMU in a way that's impacted by the changes the deprecation policy guards, you have two sane options: * Track upstream deprecation, either continuously, or at least right after a QEMU release. Since 2.10, they're collected in qemu-doc appendix "Deprecated features".

* Batch that work before you switch QEMU versions. Make sure to allocate enough time. If you consume only downstream versions of QEMU, and don't want to track upstream, go ahead and pick the second option. It should do even if you leap from 2.5 (December 2015) all the way to 3.0.

On Mon, Jul 09, 2018 at 01:08:38PM +0200, Cornelia Huck wrote: > On Mon, 09 Jul 2018 08:33:05 +0200 > Markus Armbruster <armbru(a)redhat.com&gt; wrote: > > > Peter Maydell <peter.maydell(a)linaro.org&gt; writes: > > > > > On 6 July 2018 at 15:56, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > > >> Am 06.07.2018 um 13:11 hat Cornelia Huck geschrieben: > > >>> That way, we can still easily remove old cruft (case (a)), but still > > >>> accommodate cases like this (case (c)). The obvious drawback is that > > >>> we'd need someone to curate the deprecation watchlist, to poke the > > >>> users we're waiting for, and probably remove anyway after some time if > > >>> they don't get their act together. > > >> > > >> The problem is that things are only starting to move after two releases > > >> have passed. > > > > > > Right, so clearly just "put a note in the documentation" isn't > > > sufficient advertisement/prodding of things going away. > > > > Yes. Ideas on more forceful notification have been tossed around, we > > just have to act on them. > > > > > (Also, two > > > releases is pretty fast. Many of our users will be using distro > > > packaged versions of QEMU which will lag further behind than > > > bleeding-edge users. The system version of QEMU on my desktop > > > machine is 2.5...) > > > > If you consume QEMU in a way that's impacted by the changes the > > deprecation policy guards, you have two sane options: > > > > * Track upstream deprecation, either continuously, or at least right > > after a QEMU release. Since 2.10, they're collected in qemu-doc > > appendix "Deprecated features". > > Can we draw more attention to this in any way? Point it out prominently > in the release notes? Send a list to known consumers (e.g. libvirt) on > release time? Yes, we should all newly deprecated stuff in the release notes.

For libvirt, I think whenever something is proposed for deprecation we could just CC libvir-list, or ask one of the libvirt people to confirm its not being used. If it is, then we should file BZ against libvirt.

On 12.07.2018 08:32, Markus Armbruster wrote: > Daniel P. Berrangé <berrange(a)redhat.com&gt; writes: [...] >> For libvirt, I think whenever something is proposed for deprecation >> we could just CC libvir-list, or ask one of the libvirt people to >> confirm its not being used. If it is, then we should file BZ against >> libvirt. > > Makes sense, but relying on developers getting their cc: right every > time is a setting us up for failure. > > Our tool to help with getting cc: wrong less often is the MAINTAINERS > file. Could one of the libvirt developers watch changes to qemu-doc > appendix "Deprecated features"? Would putting the appendix in its own > .texi help with that? Sound like a good idea to put the appendix in its own texi file. Then add an "R: libvir-list" entry for that file to MAINTAINERS and we should be fine (at least for the people who use the get_maintainers.pl script).

On 12.07.2018 08:32, Markus Armbruster wrote: > Daniel P. Berrangé <berrange(a)redhat.com&gt; writes: [...] >> For libvirt, I think whenever something is proposed for deprecation >> we could just CC libvir-list, or ask one of the libvirt people to >> confirm its not being used. If it is, then we should file BZ against >> libvirt. > > Makes sense, but relying on developers getting their cc: right every > time is a setting us up for failure. > > Our tool to help with getting cc: wrong less often is the MAINTAINERS > file. Could one of the libvirt developers watch changes to qemu-doc > appendix "Deprecated features"? Would putting the appendix in its own > .texi help with that? Sound like a good idea to put the appendix in its own texi file. Then add an "R: libvir-list" entry for that file to MAINTAINERS and we should be fine (at least for the people who use the get_maintainers.pl script).

On Fri, Jul 06, 2018 at 16:56:46 +0200, Kevin Wolf wrote: > Am 06.07.2018 um 13:11 hat Cornelia Huck geschrieben: > > On Wed, 4 Jul 2018 17:14:02 +0100 > > Peter Maydell <peter.maydell(a)linaro.org&gt; wrote: > > > > > On 4 July 2018 at 14:34, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: > > > > Essentially, what is important to me isn't getting these options dropped > > > > exactly in 3.0, but not setting a bad precedence that deprecation isn't > > > > actually worth anything. We may easily end up with this deprecation > > > > process: > > > > > > > > depreate a feature > > > > release QEMU version n + 1 > > > > release QEMU version n + 2 > > > > remove the feature > > > > while libvirt hasn't removed use of the feature: > > > > # ...and why should it when everything is still working? > > > > reinstate the feature > > > > release QEMU version n + x > > > > remove the feature > > > > > > My take on the deprecation policy essentially is that it gives > > > us a *minimum* bar for how soon we can drop something. We > > > shouldn't be using it as an "always target this speed for > > > dropping something" -- we ought to be pragmatic. We can > > > drop stuff that's unused quickly, but should be slower > > > for things that still have major users (or reconsider > > > the deprecation entirely, potentially). There should be > > > a balance between making our work as developers easier and > > > inconveniencing our users. > > > > What about the following? > > > > - put a feature on the "normal" deprecation list to remove after two > > releases > > Case (a): nobody complains, either within the deprecation period or > > when it is finally removed > > -> all is good > > Case (b): the feature turns out to be widely used, and/or it turns out > > that it offers value that currently can't be offered easily in another > > way > > -> remove from deprecation list; this obviously needs more thinking > > Case (c): the feature is used, the users are willing to move away from > > it, but they need a bit more time > > -> put it on a "deprecation watchlist", listing the users we are > > waiting for, and then remove after all are done (no +2) > > > > That way, we can still easily remove old cruft (case (a)), but still > > accommodate cases like this (case (c)). The obvious drawback is that > > we'd need someone to curate the deprecation watchlist, to poke the > > users we're waiting for, and probably remove anyway after some time if > > they don't get their act together. > > The problem is that things are only starting to move after two releases > have passed. The original idea was to already use that time. If we don't > use it, then waiting for two releases is pointless and we can just > directly let things go to a deprecation watchlist. > > Maybe we can just use the existing wiki page: > > https://wiki.qemu.org/index.php/Features/LegacyRemoval > > And add a column for whether libvirt is ready? Of course, that only > makes sense if libvirt people make use of and contribute to this page. This should not be a problem, but I think we need some active encouraging/prodding to remove deprecated stuff. Otherwise we might miss the news.

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru(a)redhat.com&gt; wrote: > In addition to actively pulling libvirt developers into review of > deprecation patches, we should pursue the idea to optionally let QEMU > fail on use of deprecated features, then have libvirt run its test suite > that way. What about the following: qemu_deprecated_option("old_option", "modern_option"); Which would then print (in normal operation) "WARNING: 'old_option' is deprecated and will be removed; use 'modern_option' instead" to the monitor (or to stderr? to both?). If you start QEMU with a -no-deprecated-options switch, it would print "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" and do an exit(1). Would that be workable?

Kevin Wolf <kwolf(a)redhat.com&gt; writes: > Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben: >> On Tue, 10 Jul 2018 07:59:15 +0200 >> Markus Armbruster <armbru(a)redhat.com&gt; wrote: >> >> > In addition to actively pulling libvirt developers into review of >> > deprecation patches, we should pursue the idea to optionally let QEMU >> > fail on use of deprecated features, then have libvirt run its test suite >> > that way. >> >> What about the following: >> >> qemu_deprecated_option("old_option", "modern_option"); >> >> Which would then print (in normal operation) >> >> "WARNING: 'old_option' is deprecated and will be removed; use 'modern_option' instead" >> >> to the monitor (or to stderr? to both?). >> >> If you start QEMU with a -no-deprecated-options switch, it would print >> >> "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" >> >> and do an exit(1). >> >> Would that be workable? > > I think the function should just take a message: > > /* Works like error_report(), except for the WARNING/ERROR prefix > * and exit(1) if -no-deprecated-options is set */ > void deprecation_report(const char *fmt, ...); I like it. The contract could use a bit of polish, but that's detail.

On Thu, 12 Jul 2018 08:51:16 +0200 Markus Armbruster <armbru(a)redhat.com&gt; wrote: > Markus Armbruster <armbru(a)redhat.com&gt; writes: > > > Kevin Wolf <kwolf(a)redhat.com&gt; writes: > > > >> Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben: > >>> On Tue, 10 Jul 2018 07:59:15 +0200 > >>> Markus Armbruster <armbru(a)redhat.com&gt; wrote: > >>> > >>> > In addition to actively pulling libvirt developers into review of > >>> > deprecation patches, we should pursue the idea to optionally let QEMU > >>> > fail on use of deprecated features, then have libvirt run its test suite > >>> > that way. > >>> > >>> What about the following: > >>> > >>> qemu_deprecated_option("old_option", "modern_option"); > >>> > >>> Which would then print (in normal operation) > >>> > >>> "WARNING: 'old_option' is deprecated and will be removed; use 'modern_option' instead" > >>> > >>> to the monitor (or to stderr? to both?). > >>> > >>> If you start QEMU with a -no-deprecated-options switch, it would print > >>> > >>> "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" > >>> > >>> and do an exit(1). > >>> > >>> Would that be workable? > >> > >> I think the function should just take a message: > >> > >> /* Works like error_report(), except for the WARNING/ERROR prefix > >> * and exit(1) if -no-deprecated-options is set */ > >> void deprecation_report(const char *fmt, ...); > > > > I like it. The contract could use a bit of polish, but that's detail.

> Suggest --deprecated={silent,warn,error}, default silent. I like that, but I'd prefer to default to warn (so that command line users have a better chance to notice it).

Kevin Wolf <kwolf(a)redhat.com&gt; writes: > Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben: >> On Tue, 10 Jul 2018 07:59:15 +0200 >> Markus Armbruster <armbru(a)redhat.com&gt; wrote: >> Would that be workable? > > I think the function should just take a message: > > /* Works like error_report(), except for the WARNING/ERROR prefix > * and exit(1) if -no-deprecated-options is set */ > void deprecation_report(const char *fmt, ...); I like it. The contract could use a bit of polish, but that's detail. > We don't necessarily deprecate only options, but we might also deprecate > monitor commands, specific options values (while keeping other values of > the same option) etc. Exactly.

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru(a)redhat.com&gt; wrote: > In addition to actively pulling libvirt developers into review of > deprecation patches, we should pursue the idea to optionally let QEMU > fail on use of deprecated features, then have libvirt run its test suite > that way. What about the following: qemu_deprecated_option("old_option", "modern_option");

Which would then print (in normal operation) "WARNING: 'old_option' is deprecated and will be removed; use 'modern_option' instead" to the monitor (or to stderr? to both?). If you start QEMU with a -no-deprecated-options switch, it would print "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" and do an exit(1). Would that be workable?

On Tue, 10 Jul 2018 16:39:31 +0200 Peter Krempa <pkrempa(a)redhat.com&gt; wrote: > On Tue, Jul 10, 2018 at 16:22:08 +0200, Cornelia Huck wrote: > > On Tue, 10 Jul 2018 07:59:15 +0200 > > Markus Armbruster <armbru(a)redhat.com&gt; wrote:

> > "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" > > > > and do an exit(1). > > > > Would that be workable? > > For delivering the warnings via monitor you'll need a store that will > collect all the warnings and prepare them for delivery. You've got > basically two options: > > 1) monitor command to poll for deprecated options > 2) event with deprecated options > > Both require storing them since libvirt connects to the monitor only > after the command line is processed. > > Warnings printed to stderr are nearly useless because until something > breaks nobody bothers to read the log files. So, from that I gather that a hard failure would be the easiest for libvirt to detect (and everything else would become complicated really quickly), right?

If we fail with exit(1), can libvirt check any message that is logged right before that?

> To make any reasonable use of -no-deprecated-options we'd also need > something that simulates qemu startup (no resources are touched in fact) > so that we can run it against the testsuite. Otherwise the use will be > limited to developers using it with the configuration they are > currently testing. Would that moan loudly that you should poke the libvirt developers if some kind of testsuite failure is detected? Or am I misunderstanding?

Who is, in general, testing which libvirt version? I can think of: - libvirt developers, which will probably run libvirt current git, but more likely a released QEMU?

- QEMU (and other related tools) developers, who will probably use QEMU current git, but a released libvirt

- normal (technical) users and (integration) testers, who will probably use released versions of libvirt and QEMU

On 10.07.2018 17:24, Peter Krempa wrote: > On Tue, Jul 10, 2018 at 17:01:22 +0200, Cornelia Huck wrote:

>> So, from that I gather that a hard failure would be the easiest for >> libvirt to detect (and everything else would become complicated really >> quickly), right? > > People start complaining only when stuff breaks. If anything is optional > people will usually not enable it. That makes any non-mandatory option > not work in most cases. So would it help if we "invert" the logic, i.e. deprecated_report() would do exit(1) by default? Then, if the (human) users still want to continue with the deprecated option, they have to add a "--ignore-deprecation" command line switch to make QEMU start successfully...

On Tue, Jul 10, 2018 at 17:01:22 +0200, Cornelia Huck wrote: > On Tue, 10 Jul 2018 16:39:31 +0200 > Peter Krempa <pkrempa(a)redhat.com&gt; wrote: > > On Tue, Jul 10, 2018 at 16:22:08 +0200, Cornelia Huck wrote: > > > On Tue, 10 Jul 2018 07:59:15 +0200 > > > Markus Armbruster <armbru(a)redhat.com&gt; wrote: [...] > > > "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" > > > > > > and do an exit(1). > > > > > > Would that be workable? > > > > For delivering the warnings via monitor you'll need a store that will > > collect all the warnings and prepare them for delivery. You've got > > basically two options: > > > > 1) monitor command to poll for deprecated options > > 2) event with deprecated options > > > > Both require storing them since libvirt connects to the monitor only > > after the command line is processed. > > > > Warnings printed to stderr are nearly useless because until something > > breaks nobody bothers to read the log files. > > So, from that I gather that a hard failure would be the easiest for > libvirt to detect (and everything else would become complicated really > quickly), right? People start complaining only when stuff breaks. If anything is optional people will usually not enable it. That makes any non-mandatory option not work in most cases. Since we are talking about deprecation we can't really make any of this default though so there will always be a level of user interaction required. An option is to do a automatic testing where one of this approaches will be enabled. For that you need a way to generate configurations which libvirt would use in real life. We have a rather big collection of XMLs which describe a valid configuration but the problem with using them on a real qemu is that most of the disk paths/network targets/other resources are made up and making them work with a real qemu would range from being painful to being impossible.

If we start from scratch you then lack coverage. > If we fail with exit(1), can libvirt check any message that is logged > right before that? Yes we currently use this for very early failures which occur prior to the monitor working. > > To make any reasonable use of -no-deprecated-options we'd also need > > something that simulates qemu startup (no resources are touched in fact) > > so that we can run it against the testsuite. Otherwise the use will be > > limited to developers using it with the configuration they are > > currently testing. >> > Would that moan loudly that you should poke the libvirt developers if > some kind of testsuite failure is detected? Or am I misunderstanding? Generally it should make somebody complain. But there is a problem. Since we are talking deprecation it can't be enabled by default. And by not making it default most of the users will not enable that option.

On Thu, Jul 12, 2018 at 08:59:44 +0200, Markus Armbruster wrote: > Peter Krempa <pkrempa(a)redhat.com&gt; writes: > > On Tue, Jul 10, 2018 at 17:01:22 +0200, Cornelia Huck wrote: > >> On Tue, 10 Jul 2018 16:39:31 +0200 > >> Peter Krempa <pkrempa(a)redhat.com&gt; wrote: [...] > > An option is to do a automatic testing where one of this approaches will > > be enabled. For that you need a way to generate configurations which > > libvirt would use in real life. We have a rather big collection of XMLs > > which describe a valid configuration but the problem with using them on > > a real qemu is that most of the disk paths/network targets/other > > resources are made up and making them work with a real qemu would range > > from being painful to being impossible. > > I sympathize. However, it's not clear which one's harder, providing > environments for a sufficiently wide range of configurations (possibly > mockups), or hacking QEMU to do nothing but check configuration. QEMU That's the main reason I think we should make it possible to use the data for the 'qemuxml2argv' test suite in libvirt. We require that new features are covered by this so that means that the testsuite is possibly the most comprehensive collection of libvirt configurations I know of. It's not perfect as we in many cases don't test any possible value but just try to excercise the code to generate them and others are left behind. Another historical problem was that we've defined a set of capabilities rather than using any real example to do this so many of the commandlines generated and tested are basically impossible to get in real life. That's why I added testing with real capabilities. We'll just need to generate a bunch of files to achieve full coverage here. > isn't designed for that, and configuration checking is intertwined with > everything else. Complete disentanglement looks impractical to me. I I agree. Getting anything special than the real codepath may create bubbles of problems still. On the other hand we'll need some guidance on what's sufficient to do to execute the deprecation detection code. This may require some coding style guidelines in qemu. E.g. no deprecation warnings after the vCPUs are started. Running a full operating system to check the warinigns would be utterly impractical.

Preferrably we would get away with starting qemu and waiting for the monitor to start.

> guess we could do something useful at the QAPI level, though. Yet > another reason to qapify the command line... That would be great, but I think that there's a subset of things that can be deprecated but can't be expressed by schema. In such case we still need to run the programatic checks to see.

> > If we start from scratch you then lack coverage. > > > >> If we fail with exit(1), can libvirt check any message that is logged > >> right before that? > > > > Yes we currently use this for very early failures which occur prior to > > the monitor working. > > > >> > To make any reasonable use of -no-deprecated-options we'd also need > >> > something that simulates qemu startup (no resources are touched in fact) > >> > so that we can run it against the testsuite. Otherwise the use will be > >> > limited to developers using it with the configuration they are > >> > currently testing. > >>> > >> Would that moan loudly that you should poke the libvirt developers if > >> some kind of testsuite failure is detected? Or am I misunderstanding? > > > > Generally it should make somebody complain. But there is a problem. > > Since we are talking deprecation it can't be enabled by default. And by > > not making it default most of the users will not enable that option. > > I don't think end users should do the work of catching use of deprecated > features. It's a CI job. > > In a CI context, we don't need fancy QMP infrastructure to communicate > "you used a deprecated feature", we can get away with printing an > explanation to stderr and exit(1). That should make CI fail, and the > failure should make a developer read the explanation. To unbreak CI, he > can either fix the problem right away, or file a BZ and suppress the CI > failure until it's fixed, say by downgrading --deprecated=error to > --deprecated=warn. Definitely. Plain untranslated error message is fine. The only thing is that it should be easy to detect. exit(1) is that solution. Or rather exit($VALUE_SPECIFIC_FOR_DEPRECATION) so that we can automatically discriminate test failures from deprecation warnings.

Who is, in general, testing which libvirt version? I can think of: - libvirt developers, which will probably run libvirt current git, but more likely a released QEMU? - QEMU (and other related tools) developers, who will probably use QEMU current git, but a released libvirt - normal (technical) users and (integration) testers, who will probably use released versions of libvirt and QEMU

On 10 July 2018 at 15:22, Cornelia Huck <cohuck(a)redhat.com&gt; wrote: > On Tue, 10 Jul 2018 07:59:15 +0200 > Markus Armbruster <armbru(a)redhat.com&gt; wrote: > >> In addition to actively pulling libvirt developers into review of >> deprecation patches, we should pursue the idea to optionally let QEMU >> fail on use of deprecated features, then have libvirt run its test suite >> that way. > > What about the following: > > qemu_deprecated_option("old_option", "modern_option"); > > Which would then print (in normal operation) > > "WARNING: 'old_option' is deprecated and will be removed; use 'modern_option' instead" > > to the monitor (or to stderr? to both?). > > If you start QEMU with a -no-deprecated-options switch, it would print > > "ERROR: 'old_option' is deprecated and will be removed; use 'modern_option' instead" I'd prefer to see more hand-tailored deprecation messages that can be more detailed about exactly what the new syntax needs to be. The ideal would be if you could get right down to the detailed level of "replace -old_thing -with_foo=x with -device new_thing,foo=x"...

On 06.07.2018 13:11, Cornelia Huck wrote: > On Wed, 4 Jul 2018 17:14:02 +0100 > Peter Maydell <peter.maydell(a)linaro.org&gt; wrote: > >> On 4 July 2018 at 14:34, Kevin Wolf <kwolf(a)redhat.com&gt; wrote: >>> Essentially, what is important to me isn't getting these options dropped >>> exactly in 3.0, but not setting a bad precedence that deprecation isn't >>> actually worth anything. We may easily end up with this deprecation >>> process: >>> >>> depreate a feature >>> release QEMU version n + 1 >>> release QEMU version n + 2 >>> remove the feature >>> while libvirt hasn't removed use of the feature: >>> # ...and why should it when everything is still working? >>> reinstate the feature >>> release QEMU version n + x >>> remove the feature >> >> My take on the deprecation policy essentially is that it gives >> us a *minimum* bar for how soon we can drop something. We >> shouldn't be using it as an "always target this speed for >> dropping something" -- we ought to be pragmatic. We can >> drop stuff that's unused quickly, but should be slower >> for things that still have major users (or reconsider >> the deprecation entirely, potentially). There should be >> a balance between making our work as developers easier and >> inconveniencing our users. > > What about the following? > > - put a feature on the "normal" deprecation list to remove after two > releases > Case (a): nobody complains, either within the deprecation period or > when it is finally removed > -> all is good > Case (b): the feature turns out to be widely used, and/or it turns out > that it offers value that currently can't be offered easily in another > way > -> remove from deprecation list; this obviously needs more thinking > Case (c): the feature is used, the users are willing to move away from > it, but they need a bit more time > -> put it on a "deprecation watchlist", listing the users we are > waiting for, and then remove after all are done (no +2) That sounds like another indication that we should have a list of "legacy" options in our qemu-doc, i.e. a list of interfaces that we consider as old and unwanted, but do not intend to remove in 2 releases yet. That idea has recently also come up already when we discussed the "-enable-kvm" and "-no-kvm" options. The remainders "-usbdevice" is also another good candidate for that list...

So if everyone has adopted we can certainly follow our deprecation policy. Now if deprecation breaks some real world cases it makes no sense to "insist" on that deprecation policy. Really: If latest greatest libvirt does not work 2 weeks before soft freeze I consider this too late. Why: This breaks MY regression test setup before softfreeze. So I will stop testing qemu in the most critical point in time. If you would come up with your statement (taking deprecation policy more serious than users) in the Linux kernel I can pretty much guarantee that Linus would call you names.

Am 22.06.2018 um 16:38 hat Christian Borntraeger geschrieben: > > > On 06/22/2018 04:25 PM, Kevin Wolf wrote: >> Am 22.06.2018 um 15:36 hat Christian Borntraeger geschrieben: >>> >>> >>> On 06/22/2018 02:55 PM, Kevin Wolf wrote: >>>> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: >>>>> >>>>> On 06/15/2018 04:21 PM, Kevin Wolf wrote: >>>>>> The -drive option serial was deprecated in QEMU 2.10. It's time to >>>>>> remove it. >>>>>> >>>>>> Tests need to be updated to set the serial number with -global instead >>>>>> of using the -drive option. >>>>> >>>>> libvirt 4.5 still creates those (at least on s390x) >>>>> >>>>> <disk type='file' device='disk'> >>>>> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> >>>>> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> >>>>> <target dev='hda' bus='virtio'/> >>>>> <serial>skel</serial> >>>>> <boot order='1'/> >>>>> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> >>>>> </disk> >>>>> >>>>> >>>>> -> >>>>> [...] >>>>> -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on >>>>> [...] >>>>> >>>>> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' >>>>> 2018-06-22 11:25:21.098+0000: shutting down, reason=failed >>>>> >>>>> So it seems that this breaks s390x. >>> >>> To me it seems that this is also broken on x86. >>>> >>>> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is >>>> released. >>> >>> I think this is definitely too short notice. We should not break existing >>> setups just by insisting that users have to update libvirt when they update >>> QEMU. Yes, this might be our policy, but doing so "just because we can" >>> is certainly a very bad attitude. I see no fundamental technical reason why >>> we should not revert this change. >> >> This was in fact one release longer than our deprecation policy says. >> Are we serious about the deprecation policy or aren't we? > > I think it makes more sense to have 2 releases after everything was fixed > instead of 2 releases after it was announced. This means effectively banning feature removal. The only time that people actually starting fixing things is when it breaks. So if you never remove it before everything is fixed, you just never remove it at all.

It's unfortunate, but breaking things at some point is necessary. I hope the breakage will only last a few days because libvirt will fix this. Maybe one thing we could look into for the future is a special deprecation warning function rather than just error_report(), and we would make that one fatal in non-release builds so that things break early, but you can still override it with a ./configure option. > So if everyone has adopted we can certainly follow our deprecation policy. > Now if deprecation breaks some real world cases it makes no sense to > "insist" on that deprecation policy. Really: If latest greatest libvirt > does not work 2 weeks before soft freeze I consider this too late. > > Why: This breaks MY regression test setup before softfreeze. So I will stop > testing qemu in the most critical point in time. > > If you would come up with your statement (taking deprecation policy more > serious than users) in the Linux kernel I can pretty much guarantee that > Linus would call you names. Users shouldn't use random git snapshots. Developers can revert the change locally until libvirt is fixed.g If contrary to all expectations, libvirt doesn't manage to get this fixed until 3.0-rc2, I will consider reverting the patch. But not significantly earlier than that.

On Fri, Jun 22, 2018 at 04:25:13PM +0200, Kevin Wolf wrote: > This was in fact one release longer than our deprecation policy says. > Are we serious about the deprecation policy or aren't we? > > I might consider reverting a change if it turned out that this requires > some massive work in libvirt. But I think this one should be rather easy > to fix in libvirt until 3.0 is released. I've got a patch mostly ready that converts libvirt to setting these things on the frontend device, however, I've got some queries... - usb-storage - doesn't appear to support geometry or werror/rerror Will we loose functionality by stopping use of -drive for werror Loosing geometry feels relevant too, unless it was already ignored when set opf -drive ?

- SD card - requires -drive with if=sd, no -device support AFAICT Will we loose functionality be stopping use of -drive for if=sd, or is this stuff ignored anyway ?

- ide-cd/scsi-id - doesn't support geometry Presumably becuase its irrelevant for CDs

- SD card - requires -drive with if=sd, no -device support AFAICT

On Mon, Jun 25, 2018 at 11:01:46 +0100, Peter Maydell wrote: > On 22 June 2018 at 16:40, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > - SD card - requires -drive with if=sd, no -device support AFAICT > > Hmm? You can use -device if you want: > -drive if=none,id=mydrive,... -device sd-card,...,drive=mydrive > (the sd-card device then plugs into the sd controller device > via the sd-bus bus). > > Most command lines floating around on the web use if=sd, though > (which doesn't require explicit creation of the sd-card device.) When I've spoke with Markus during the KVM forum he found out that some machine types still don't support that, so that why libvirt didn't bother converting it yet.

Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: > > On 06/15/2018 04:21 PM, Kevin Wolf wrote: > > The -drive option serial was deprecated in QEMU 2.10. It's time to > > remove it. > > > > Tests need to be updated to set the serial number with -global instead > > of using the -drive option. > > libvirt 4.5 still creates those (at least on s390x) > > <disk type='file' device='disk'> > <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> > <source file='/var/lib/libvirt/qemu/image.zhyp137'/> > <target dev='hda' bus='virtio'/> > <serial>skel</serial> > <boot order='1'/> > <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> > </disk> > > > -> > [...] > -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on > [...] > > 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' > 2018-06-22 11:25:21.098+0000: shutting down, reason=failed > > So it seems that this breaks s390x. Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is released. Sadly, it also shows that deprecation warnings in log files go unnoticed.

On Fri, Jun 22, 2018 at 04:19:29PM +0200, Markus Armbruster wrote: > Kevin Wolf <kwolf(a)redhat.com&gt; writes: > > > Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: > >> > >> On 06/15/2018 04:21 PM, Kevin Wolf wrote: > >> > The -drive option serial was deprecated in QEMU 2.10. It's time to > >> > remove it. > >> > > >> > Tests need to be updated to set the serial number with -global instead > >> > of using the -drive option. > >> > >> libvirt 4.5 still creates those (at least on s390x) > >> > >> <disk type='file' device='disk'> > >> <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> > >> <source file='/var/lib/libvirt/qemu/image.zhyp137'/> > >> <target dev='hda' bus='virtio'/> > >> <serial>skel</serial> > >> <boot order='1'/> > >> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> > >> </disk> > >> > >> > >> -> > >> [...] > >> -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on > >> [...] > >> > >> 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' > >> 2018-06-22 11:25:21.098+0000: shutting down, reason=failed > >> > >> So it seems that this breaks s390x. > > > > Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is > > released. > > > > Sadly, it also shows that deprecation warnings in log files go > > unnoticed. > > Nobody reads log files until things have gone belly up, and even then > unrelated log entries get ignored. > > The way to get deprecation warnings noticed it to have the management > application fail its "make check". > > Perhaps we could use a more structured notification, to make detecting > use of deprecated features programmatically trivial. A QMP event might > do. Libvirt currently has CI that is largely focused on unit testing. We recently did some work, however, to get our functional test suite working properly again (Sys-Virt-TCK) and are trying to get some new CI hardware. So if we get that running, we coud run tests on real QEMU versions and check the /var/log/libvirt/qemu/$GUEST.logs to make sure we're not triggering unexpected warnings from QEMU

Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: > > On 06/15/2018 04:21 PM, Kevin Wolf wrote: > > The -drive option serial was deprecated in QEMU 2.10. It's time to > > remove it. > > > > Tests need to be updated to set the serial number with -global instead > > of using the -drive option. > > libvirt 4.5 still creates those (at least on s390x) > > <disk type='file' device='disk'> > <driver name='qemu' type='qcow2' cache='none' io='native' iothread='1'/> > <source file='/var/lib/libvirt/qemu/image.zhyp137'/> > <target dev='hda' bus='virtio'/> > <serial>skel</serial> > <boot order='1'/> > <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> > </disk> > > > -> > [...] > -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native -device virtio-blk-ccw,iothread=iothread1,scsi=off,devno=fe.0.0000,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on > [...] > > 2018-06-22T11:25:20.946024Z qemu-system-s390x: -drive file=/var/lib/libvirt/qemu/image.zhyp137,format=qcow2,if=none,id=drive-virtio-disk0,serial=skel,cache=none,aio=native: Block format 'qcow2' does not support the option 'serial' > 2018-06-22 11:25:21.098+0000: shutting down, reason=failed > > So it seems that this breaks s390x. Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is released.

On 25.06.2018 09:16, Peter Krempa wrote: > On Fri, Jun 22, 2018 at 14:55:02 +0200, Kevin Wolf wrote: >> Am 22.06.2018 um 13:38 hat Christian Borntraeger geschrieben: >>> >>> On 06/15/2018 04:21 PM, Kevin Wolf wrote: >>>> The -drive option serial was deprecated in QEMU 2.10. It's time to >>>> remove it. >>>> >>>> Tests need to be updated to set the serial number with -global instead >>>> of using the -drive option. [...] >>> So it seems that this breaks s390x. >> >> Thanks for bringing this up. libvirt should fix this before QEMU 3.0 is >> released. > > So, I already extracted the code that formats the attributes which > should be actually done for the frontend few months ago, but did not > swithc to it since I did not want to check when everything was added. > > The fix will be rather simple if we are certain that the disk serial, > geometry and error policies were supported in qemu 1.5. In that case we > can switch unconditionally to the new format. The CHS device properties are available since QEMU v1.2: - IDE: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ba801960 - SCSI: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=d252df48 - virtio-blk: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=e63e7fde2 The "serial" property and the PCI "addr" property are also available since 2012 at least, I didn't fully track their origins, but QEMU v1.2 already supported them. So yes, all these properties should be available in QEMU 1.5 already.