Re: [Qemu-devel] [PULL 25/26] block: Remove deprecated -drive option serial

[Kevin Wolf]

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