On 22.06.2018 14:51, Christian Borntraeger wrote:

adding more CC.

On 06/22/2018 01:38 PM, Christian Borntraeger wrote:

...

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.

I wonder why nobody noticed the deprecation messages before?

So what about reverting commit b0083267444a5e0f28391f6c2831a539f878d424 "block: Remove deprecated -drive option serial" and redo the removal in qemu 3.1 (or 3.2) ? Even if we fix libvirt today, this is certainly a too short period of time to get things fixed in the field.

Agreed, reverting that commit is likely the best thing we can do right now, and then kill it in a later QEMU release. Note that you also need to revert 6266e900b8083945cb766b45c124fb3c42932cb3 first. Thomas

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 03:36:50PM +0200, Christian Borntraeger wrote:

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.

Correct, this is not architecture specific.

...

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 libvirt side I thought we had already stopped using the deprecated syntax, but we clearly missed it :-( Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

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

...

...

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?

I expect the libvirt developers to read the QEMU Changelog at least for incompatible changes and deprecations. We can't reasonably go and hunt for developers for every management tool for QEMU that exists.

Yeah, from libvirt side we need todo a better job of checking this and filing bugs against libvirt if there's something we tickle. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

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:

...

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

Fix posted here:

https://www.redhat.com/archives/libvir-list/2018-June/msg01598.html

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? Kevin

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? Kevin

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 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 3 July 2018 at 12:32, Kevin Wolf <kwolf@redhat.com> 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? thanks -- PMM

On Tue, Jul 03, 2018 at 01:32:29PM +0200, Kevin Wolf wrote:

Am 03.07.2018 um 13:22 hat Daniel P. Berrangé geschrieben:

...

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

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.

Yeah, unfortunately i just missed the window of possibility to get it into yesterday's release. Fedora at least will be ok, and when other distros do pick up new QEMU they'll likely pick up the corresponding libvirt release at same time anyway. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Am 04.07.2018 um 15:02 hat Cornelia Huck geschrieben:

On Tue, 3 Jul 2018 13:32:29 +0200 Kevin Wolf <kwolf@redhat.com> 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. Kevin

Am 04.07.2018 um 15:43 hat Daniel P. Berrangé geschrieben:

On Wed, Jul 04, 2018 at 03:34:40PM +0200, Kevin Wolf wrote:

...

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.

I'd venture to suggest disk geometry is almost never used in practice.

serial strings, however, are pretty common, used by default in fact by openstack.

Interesting, good to know.

...

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.

My understanding was actually mostly the opposite. We'll try not to /knowingly/ break existing libvirt version if reasonable. We've not always followed that, either by accident, or even intentionally in some cases. What's never defined is just how old libvirt we care about not breaking.

Hm, that used to be true, yes. I still seem to remember that we've always expected a new libvirt version. Was that only about new features then? However, the deprecation policy essentially says that we will now knowingly break existing libvirt versions. With it, we also defined that we definitely don't care about libvirt versions older than QEMU n-2. You're right that we're not really making a statement for libvirt versions newer than that.

In the case of QEMU libvirt will in fact be fixed before QEMU 3.0 is released, but only by a week or so.

Probably two weeks because QEMU never does without the extra RC, but that doesn't make a big difference.

I think its mostly a case of being pragmatic about what we do in this respect, rather than defining a hard rule.

In this case I'd personally lean towards reverting this patch, since merging it was not a critical blocker to other work that went into 3.0 IIUC. So block maintaniers burden shouldn't be impacted by a temporary revert and then re-merge when 3.1 opens.

Having to remember to really bring it back to life after the release is a maintainer burden, which is one of the reasons why I don't want this "extended" deprecation process to become the norm. If it stays a one-time thing, and given that we're now in the freeze and a block-next branch will be active anyway, I'm okay with a temporary revert.

...

...

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

Yeah, we definitely don't want to get into that kind of mess in general.

...

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.

What I think this really highlights is

- Lack of libvirt paying attention to deprecations - Lack of a way to get good diagnostic of violations when testing

The first thing is just a human problem on libvirt side.

The second thing could be addressed in code if there was an env variable or cli option to turn all use of deprecated features into abort()s in QEMU. That way libvirt automated testing against git master would have a way to identify places where we use deprecated feature.

The first one is basically just a result of the second. I think the most important improvement will be finding a way how to break things early for libvirt developers, but keep them working with a warning for everyone else. As a more short-term thing, is someone from libvirt looking at other present deprecations in QEMU and what needs to be done for them? Kevin

On Wed, 4 Jul 2018 15:34:40 +0200 Kevin Wolf <kwolf@redhat.com> wrote:

Am 04.07.2018 um 15:02 hat Cornelia Huck geschrieben:

...

On Tue, 3 Jul 2018 13:32:29 +0200 Kevin Wolf <kwolf@redhat.com> 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.

Consider people running regression tests: They likely have a set of machine definitions they use, some of which may include serial numbers et al., and that suddenly breaks if they try with a newer QEMU. If they can just update to a newer (released) libvirt, their regression tests continue to work.

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.

My understanding has been that while a recent-ish libvirt might not exploit the latest QEMU features, it still continues to work. But yes, this is a good point. We need to agree on a clear policy and document that.

...

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.

I understand your concern, but not having a way out if something fell through the cracks is hurting people testing QEMU -- IOW, people we really don't want to hurt. Ideas on what could help: - A clearer way to communicate to libvirt users that libvirt is using a deprecated API, so that they can report it and libvirt can change its code. The main problem is that people usually don't read logs if everything seems to work fine... - A documented way in our QEMU deprecation process to hold off for one release, which can be used at most twice (or maybe just once?) No endless loops that way :)

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.

Fully agreed on sorting out our deprecation mechanism. I can send a revert patch, if nobody else beats me to it. Thanks!

On Wed, 4 Jul 2018 17:14:02 +0100 Peter Maydell <peter.maydell@linaro.org> wrote:

On 4 July 2018 at 14:34, Kevin Wolf <kwolf@redhat.com> 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.

In this particular case, reverting this deletion seems like a fairly easy call to me.

We should also definitely work on improving how we can let management-layer developers easily test that they're not accidentally relying on deprecated features, certainly (and also on better documentation for command line users of how to switch away from deprecated features -- for instance I am still using -redir in some of my scripts because the warning about it being deprecated is not precise about what exact command line option can be used instead, especially for the case where the ethernet device is builtin rather than created with -device...)

Yes, this as well.

On Fri, Jul 06, 2018 at 04:56:46PM +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@linaro.org> wrote:

...

On 4 July 2018 at 14:34, Kevin Wolf <kwolf@redhat.com> 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.

FYI I checked the list of deprecated features, and aside from the drive ones, libvirt is impacted by the VNC TLS stuff, and the change to "target" in the query-cpus-fast QMP command. I've got bugs open against libvirt to address all of these Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Peter Maydell <peter.maydell@linaro.org> writes:

On 6 July 2018 at 15:56, Kevin Wolf <kwolf@redhat.com> 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@redhat.com> wrote:

...

Peter Maydell <peter.maydell@linaro.org> writes:

...

On 6 July 2018 at 15:56, Kevin Wolf <kwolf@redhat.com> 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. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 12.07.2018 08:32, Markus Armbruster wrote:

Daniel P. Berrangé <berrange@redhat.com> 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). Thomas

On Fri, Jul 13, 2018 at 01:35:02PM +0200, Cornelia Huck wrote:

On Thu, 12 Jul 2018 17:47:00 +0200 Thomas Huth <thuth@redhat.com> wrote:

...

On 12.07.2018 08:32, Markus Armbruster wrote:

...

Daniel P. Berrangé <berrange@redhat.com> writes:

[...]

...

...

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).

+1, like that idea

Are there other consumers of QEMU's interfaces which should be cc:ed in a similar way?

Perhaps starting with libvirt is fine -- as most of the open source management applications rely on it. (But if we do know other consumers, then they can be trivially added.) -- /kashyap

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@linaro.org> wrote:

...

On 4 July 2018 at 14:34, Kevin Wolf <kwolf@redhat.com> 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@redhat.com> 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@redhat.com> writes:

Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben:

...

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru@redhat.com> 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.

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 Thu, 12 Jul 2018 08:51:16 +0200 Markus Armbruster <armbru@redhat.com> wrote:

Markus Armbruster <armbru@redhat.com> writes:

...

Kevin Wolf <kwolf@redhat.com> writes:

...

Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben:

...

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru@redhat.com> 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).

Cornelia Huck <cohuck@redhat.com> writes:

On Thu, 12 Jul 2018 08:51:16 +0200 Markus Armbruster <armbru@redhat.com> wrote:

...

Markus Armbruster <armbru@redhat.com> writes:

...

Kevin Wolf <kwolf@redhat.com> writes:

...

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).

Fair enough.

Peter Krempa <pkrempa@redhat.com> writes:

On Thu, Jul 12, 2018 at 08:38:25 +0200, Markus Armbruster wrote:

...

Kevin Wolf <kwolf@redhat.com> writes:

...

Am 10.07.2018 um 16:22 hat Cornelia Huck geschrieben:

...

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru@redhat.com> 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.

For monitor commands we luckily have QMP introspection which can help a lot in this case. At least for deprecating stuff that is expressable by the schema.

Introspection doesn't convey "deprecated", but...

In libvirt we are actually doing schema validation of the blockdev-add arguments generators and most commands which are covered by the qemumonitorjsontest. The schema used is based on our capability detection so it's gathered from the most-recent version of qemu we have required for our tests (which is most of the time based on GIT version of qemu if there are any significant new features).

If the deprecation will be expressable by the schema it should be rather simple to modify the schema validator to catch the deprecation flags and report errors in our testsuite.

... we can certainly make it if it's useful.

CI-ifying of the above should be then also very simple. We'd just gather fresh QMP schema rather than using one from our test case pantry.

Some time ago I also added testing of the commandline generator in libvirt with the most recent capabilities rather than using the historically declared capabilities that we've added when the test was created. This means that we actually test some valid combinations and also if stuff covered by our capability probing is removed the tests will catch it.

I was also thinking of adding a tool which would use the above tests to attempt starting of a qemu process until the monitor shows up. That test then could also use -no-deprecated-options. I'm hoping waiting for the monitor is sufficient to excercise most of the code which could contain deprecation warnings. (Alternatively we can go through the pre-cpu-startup setup done on the monitor as well). Unfortunately doing this will not be as simple asi the test cases contain random disk paths and other resources which may not be available. This means that it will require some in-place modification and creative temporary resource usage.

Yes. If you find QEMU makes testing something hard, we should talk. Together we might find a reasonable way to make it easier.

On Tue, 10 Jul 2018 16:39:31 +0200 Peter Krempa <pkrempa@redhat.com> 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@redhat.com> 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");

I think this is too simplified. You can deprecate only a certain value for an option or even just a combination of values and options. The check will need to be programatic and error reporting probably can't be reasonably machine readable anyways.

Kevin's suggestion of using a simple message instead of this old/modern option thing is actually better (and would cover more). My intention behind printing a message (somewhere) was to make this friendly to command line users as well.

...

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?

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:

...

On Tue, 10 Jul 2018 16:39:31 +0200 Peter Krempa <pkrempa@redhat.com> 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@redhat.com> 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.

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... Thomas

Thomas Huth <thuth@redhat.com> writes:

On 10.07.2018 17:24, Peter Krempa wrote:

...

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@redhat.com> 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@redhat.com> 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.

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...

You owe the God of Backward Compatibility one rubber chicken for thinking this heretic thought!

On Thu, Jul 12, 2018 at 08:59:44 +0200, Markus Armbruster wrote:

Peter Krempa <pkrempa@redhat.com> 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@redhat.com> 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.

On Tue, Jul 10, 2018 at 05:01:22PM +0200, Cornelia Huck wrote:

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?

In general libvirt devs tend to run a mixture of whatever the default QEMU is in Fedora / RHEL, along side current QEMU git master, depending on what feature(s) we're working on.

- 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

We don't currently have any integration testing CI system, but we are in process of getting hardware resource to allow us to run CI. With that we can setup jobs which test various relevant QEMU versions, both git master and released versions. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 10 July 2018 at 15:22, Cornelia Huck <cohuck@redhat.com> wrote:

On Tue, 10 Jul 2018 07:59:15 +0200 Markus Armbruster <armbru@redhat.com> 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"... thanks -- PMM

On 06.07.2018 13:11, Cornelia Huck wrote:

On Wed, 4 Jul 2018 17:14:02 +0100 Peter Maydell <peter.maydell@linaro.org> wrote:

...

On 4 July 2018 at 14:34, Kevin Wolf <kwolf@redhat.com> 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... Thomas

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. 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.

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 have not heard any reason what we gain by removing these features (and no I dont believe that this increases your maintenance burden a lot). But it clearly breaks things. I suggest: revert the removal patches (all of them cyls,secs,serial etc) and redo them for 3.1.

...

...

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?

I expect the libvirt developers to read the QEMU Changelog at least for incompatible changes and deprecations. We can't reasonably go and hunt for developers for every management tool for QEMU that exists.

And anyway, if you come across a deprecation warning, that's the time you should act, not only when it finally breaks.

Kevin

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. 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. Kevin

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.

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 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Fri, Jun 22, 2018 at 07:54:00PM +0200, Kevin Wolf wrote:

Am 22.06.2018 um 17:40 hat Daniel P. Berrangé geschrieben:

...

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 ?

You're right, usb-storage doesn't allow using -device to specify these, and it does use the values from -drive.

For werror/rerror, we should clearly implement the option forwarding to scsi-disk so that you can make use of it.

This missing werror item looks like a blocker for libvirt to be able to switch to using -blockdev without a regression.

I'm not sure how sane specifying a non-standard CHS geometry for a USB stick actually is. As an additional difficulty, usb-storage internally creates a scsi-disk device (not scsi-hd), which is also considered legacy and doesn't support the geometry options either, so it's not just simple forwarding. We removed an actual feature there, but that feature was probably never intended nor used.

If someone comes up with a compelling reason why they really need to configure the CHS geometry of their USB sticks, I guess we can do it. My real USB sticks I tested don't even support MODE_PAGE_HD_GEOMETRY (though they have MODE_PAGE_FLEXIBLE_DISK_GEOMETRY).

I don't think libvirt has a compelling reason. The ability to set CHS on usb-storage is just something we got for free because it used the same -drive syntax as other disk device frontends. It would be nice to avoid the regression but I doubt people will actually notice in practice as usb-storage users are few & far between, and even fewer of those will care about CHS. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Jun 25, 2018 at 11:01:46 +0100, Peter Maydell wrote:

On 22 June 2018 at 16:40, Daniel P. Berrangé <berrange@redhat.com> 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.

On 22.06.2018 16:25, 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.

Actually, if we assume that the chapter in qemu-doc.texi is the "official" way to deprecate things, these options are only officially deprecated since QEMU v2.12, since we missed to add them to the deprecation chapter earlier: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c08d46a9 (We've mentioned them in https://wiki.qemu.org/ChangeLog/2.10#Deprecated_options already, though) Thomas

On Fri, Jun 22, 2018 at 04:19:29PM +0200, Markus Armbruster wrote:

Kevin Wolf <kwolf@redhat.com> 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 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 06/22/2018 09:30 AM, Daniel P. Berrangé wrote:

...

...

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

This could be even easier if there was a --no-deprecations flag to QEMU which triggered abort() whenever mgmt app uses a deprecated feature.

Yes, a QMP event (which libvirt could then turn into a hard error if it ever receives the event) or a qemu command line option to make deprecated usage fatal (which libvirt would choose to enable) would both be pragmatic approaches to quickly vetting whether libvirt is using something that qemu has marked deprecated - provided that we are careful to always wire up the event/abort into qemu at each location where we also add a deprecation message. An event might be more flexible than qemu aborting (as libvirt could make programmatic decisions on whether to keep going in spite of the event, rather than the guest unconditionally being lost). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

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. Thomas