| Message ID | 20180822095421.11765-8-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Don't check disabled drivers | expand |
On 2018-08-22 11:54, Juan Quintela wrote: > This is only for x86* architecture. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > tests/Makefile.include | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 116e93cdb6..d524e1bdeb 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -257,7 +257,7 @@ check-qtest-i386-y += tests/hd-geo-test$(EXESUF) > gcov-files-i386-y += hw/block/hd-geometry.c > check-qtest-i386-y += tests/boot-order-test$(EXESUF) > check-qtest-i386-y += tests/bios-tables-test$(EXESUF) > -check-qtest-i386-y += tests/boot-serial-test$(EXESUF) > +check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) Reviewed-by: Thomas Huth <thuth@redhat.com> Off-topic: I wonder whether we could even deprecate the SGA device nowadays, since the main seabios now can redirect the console to the serial port, too? Thomas
Thomas Huth <thuth@redhat.com> wrote: > On 2018-08-22 11:54, Juan Quintela wrote: >> This is only for x86* architecture. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> tests/Makefile.include | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index 116e93cdb6..d524e1bdeb 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -257,7 +257,7 @@ check-qtest-i386-y += tests/hd-geo-test$(EXESUF) >> gcov-files-i386-y += hw/block/hd-geometry.c >> check-qtest-i386-y += tests/boot-order-test$(EXESUF) >> check-qtest-i386-y += tests/bios-tables-test$(EXESUF) >> -check-qtest-i386-y += tests/boot-serial-test$(EXESUF) >> +check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Off-topic: I wonder whether we could even deprecate the SGA device > nowadays, since the main seabios now can redirect the console to the > serial port, too? No idea, but then I think we can deprecate it, we already have enough weird things around. Thanks, Juan.
> > Reviewed-by: Thomas Huth <thuth@redhat.com> > > > > Off-topic: I wonder whether we could even deprecate the SGA device > > nowadays, since the main seabios now can redirect the console to the > > serial port, too? > > No idea, but then I think we can deprecate it, we already have enough > weird things around. Deprecating sure. Actually dropping is not so easy I think. The switch is not transparent to the guest, which implies live migration compatibility isssues. So the only thing we can do is blocking it for new machine types (3.1+). Then, in a bunch of years when we deprecate 3.1 & older machine types, we can actually remove it. I remember we even discussed that a while back ... Oh yes: https://git.kraxel.org/cgit/qemu/commit/?h=work/no-sgabios&id=408690999429fdf44011c49b3af6efa50f369b1e Somehow never ended up being merged. cheers, Gerd
On 2018-08-23 09:43, Gerd Hoffmann wrote: >>> Reviewed-by: Thomas Huth <thuth@redhat.com> >>> >>> Off-topic: I wonder whether we could even deprecate the SGA device >>> nowadays, since the main seabios now can redirect the console to the >>> serial port, too? >> >> No idea, but then I think we can deprecate it, we already have enough >> weird things around. > > Deprecating sure. Actually dropping is not so easy I think. The switch > is not transparent to the guest, which implies live migration > compatibility isssues. > > So the only thing we can do is blocking it for new machine types (3.1+). > Then, in a bunch of years when we deprecate 3.1 & older machine types, > we can actually remove it. > > I remember we even discussed that a while back ... > Oh yes: https://git.kraxel.org/cgit/qemu/commit/?h=work/no-sgabios&id=408690999429fdf44011c49b3af6efa50f369b1e > Somehow never ended up being merged. Sounds like a good idea! Could you repost the patch, with an update to qemu-deprecated.texi, please? Maybe also print the deprecation message in any case if !use_sgabios, e.g.: if (sga->use_sgabios) { rom_add_vga(SGABIOS_FILENAME); } else if (machine->enable_graphics) { warn_report("sgabios is deprecated for your machine type, " "please use -machine graphics=off instead of " "-device sga"); rom_add_vga(SGABIOS_FILENAME); } else { /* * For this machine type we expect SeaBIOS to provide a * serial console for -machine graphics=off. No need to * add the sgabios rom here. */ warn_report("sgabios is deprecated and is not required anymore," " SeaBIOS now has the same functionality, too"); } Thomas
> > I remember we even discussed that a while back ... > > Oh yes: https://git.kraxel.org/cgit/qemu/commit/?h=work/no-sgabios&id=408690999429fdf44011c49b3af6efa50f369b1e > > Somehow never ended up being merged. > > Sounds like a good idea! Could you repost the patch, with an update to > qemu-deprecated.texi, please? Maybe also print the deprecation message > in any case if !use_sgabios, e.g.: > > } else { > /* > * For this machine type we expect SeaBIOS to provide a > * serial console for -machine graphics=off. No need to > * add the sgabios rom here. > */ > warn_report("sgabios is deprecated and is not required anymore," > " SeaBIOS now has the same functionality, too"); > } Hmm, IIRC libvirt support is a bit tricky here as there is no easy way to probe for serial console support in seabios. So the idea was to just allow both -machine graphics=off and -device sga, by turning -device sga into a nop in case -machine graphics=off is present. But given we meanwhile had some releases without this I suspect that isn't a good idea any more. With both -machine graphics=off -device sga seabios and sgabios will fight over the serial device ... So, maybe take a completely different route. Add a deprecation_reason field to DeviceClass. Make it visible in qapi introspection, so libvirt can see it and avoid adding it to the command line. Then we can deprecate -device sga (and possibly other devices too) that way. For live migration compatibility reasons it'll be quite a while until we can actually delete the device (much longer than the usual 2 releases deprecation cycle). cheers, Gerd
On Thu, Aug 23, 2018 at 11:48:50AM +0200, Gerd Hoffmann wrote: > > > I remember we even discussed that a while back ... > > > Oh yes: https://git.kraxel.org/cgit/qemu/commit/?h=work/no-sgabios&id=408690999429fdf44011c49b3af6efa50f369b1e > > > Somehow never ended up being merged. > > > > Sounds like a good idea! Could you repost the patch, with an update to > > qemu-deprecated.texi, please? Maybe also print the deprecation message > > in any case if !use_sgabios, e.g.: > > > > } else { > > /* > > * For this machine type we expect SeaBIOS to provide a > > * serial console for -machine graphics=off. No need to > > * add the sgabios rom here. > > */ > > warn_report("sgabios is deprecated and is not required anymore," > > " SeaBIOS now has the same functionality, too"); > > } > > Hmm, IIRC libvirt support is a bit tricky here as there is no easy way > to probe for serial console support in seabios. So the idea was to just > allow both -machine graphics=off and -device sga, by turning -device sga > into a nop in case -machine graphics=off is present. > > But given we meanwhile had some releases without this I suspect that > isn't a good idea any more. With both -machine graphics=off -device sga > seabios and sgabios will fight over the serial device ... > > So, maybe take a completely different route. Add a deprecation_reason > field to DeviceClass. Make it visible in qapi introspection, so libvirt > can see it and avoid adding it to the command line. Then we can > deprecate -device sga (and possibly other devices too) that way. That won't work IIUC. If we stop using "-device sga" based on some introspected deprecation value on the device, that means the guest will have a different ABI from POV of migration stream. So we would be unable to migrate to a QEMU without the deprecation, despite the XML config being unchanged. The only way we can stop using "-device sga" is if we either have something explicit in the XML saying whether to use it or not, or alternatively have some indicator associated with the next versioned machine type that we can hook off. Regards, Daniel
Hi, > The only way we can stop using "-device sga" is if we either have > something explicit in the XML saying whether to use it or not, or > alternatively have some indicator associated with the next versioned > machine type that we can hook off. i.e. add some flag to MachineClass (and print it in introspection) ? cheers, Gerd
Daniel P. Berrangé <berrange@redhat.com> wrote: > On Thu, Aug 23, 2018 at 11:48:50AM +0200, Gerd Hoffmann wrote: >> > > I remember we even discussed that a while back ... >> > > Oh yes: >> > > https://git.kraxel.org/cgit/qemu/commit/?h=work/no-sgabios&id=408690999429fdf44011c49b3af6efa50f369b1e >> > > Somehow never ended up being merged. >> > >> > Sounds like a good idea! Could you repost the patch, with an update to >> > qemu-deprecated.texi, please? Maybe also print the deprecation message >> > in any case if !use_sgabios, e.g.: >> > >> > } else { >> > /* >> > * For this machine type we expect SeaBIOS to provide a >> > * serial console for -machine graphics=off. No need to >> > * add the sgabios rom here. >> > */ >> > warn_report("sgabios is deprecated and is not required anymore," >> > " SeaBIOS now has the same functionality, too"); >> > } >> >> Hmm, IIRC libvirt support is a bit tricky here as there is no easy way >> to probe for serial console support in seabios. So the idea was to just >> allow both -machine graphics=off and -device sga, by turning -device sga >> into a nop in case -machine graphics=off is present. >> >> But given we meanwhile had some releases without this I suspect that >> isn't a good idea any more. With both -machine graphics=off -device sga >> seabios and sgabios will fight over the serial device ... >> >> So, maybe take a completely different route. Add a deprecation_reason >> field to DeviceClass. Make it visible in qapi introspection, so libvirt >> can see it and avoid adding it to the command line. Then we can >> deprecate -device sga (and possibly other devices too) that way. > > That won't work IIUC. If we stop using "-device sga" based on some > introspected deprecation value on the device, that means the guest > will have a different ABI from POV of migration stream. So we would > be unable to migrate to a QEMU without the deprecation, despite the > XML config being unchanged. > > The only way we can stop using "-device sga" is if we either have > something explicit in the XML saying whether to use it or not, or > alternatively have some indicator associated with the next versioned > machine type that we can hook off. I think this is the only sane way to go. From machine type X.Y, device whatever is no longer available. That work for all devices, make live migration working, and we are sure that we are not using the "deprecated" device on new machines. Later, Juan.
Hi, > The only way we can stop using "-device sga" is if we either have > something explicit in the XML saying whether to use it or not, or > alternatively have some indicator associated with the next versioned > machine type that we can hook off. How does libvirt query the machine type properties? thanks, Gerd
On Mon, Aug 27, 2018 at 12:53:21PM +0200, Gerd Hoffmann wrote: > Hi, > > > The only way we can stop using "-device sga" is if we either have > > something explicit in the XML saying whether to use it or not, or > > alternatively have some indicator associated with the next versioned > > machine type that we can hook off. > > How does libvirt query the machine type properties? libvirt is able to query properties against i440FX-pcihost and q35-pcihost devices - currently we only look for one prop: static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsI440FXPCIHost[] = { { "pci-hole64-size", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsQ35PCIHost[] = { { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE }, }; Not sure if that's the kind of thing you're looking for ? Regards, Daniel
On Tue, Aug 28, 2018 at 09:19:20AM +0100, Daniel P. Berrangé wrote: > On Mon, Aug 27, 2018 at 12:53:21PM +0200, Gerd Hoffmann wrote: > > Hi, > > > > > The only way we can stop using "-device sga" is if we either have > > > something explicit in the XML saying whether to use it or not, or > > > alternatively have some indicator associated with the next versioned > > > machine type that we can hook off. > > > > How does libvirt query the machine type properties? > > libvirt is able to query properties against i440FX-pcihost and > q35-pcihost devices - currently we only look for one prop: > > static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsI440FXPCIHost[] = { > { "pci-hole64-size", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE }, > }; > > static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsQ35PCIHost[] = { > { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE }, > }; > > Not sure if that's the kind of thing you're looking for ? Looking for way for libvirt to figure whenever a specific machine type has sga deprecated or not. So, if libvirt can query device properties, can we just add a "deprecated" property to sga and flip that to true for 3.1+ machine types? cheers, Gerd
On Tue, Aug 28, 2018 at 12:20:02PM +0200, Gerd Hoffmann wrote: > On Tue, Aug 28, 2018 at 09:19:20AM +0100, Daniel P. Berrangé wrote: > > On Mon, Aug 27, 2018 at 12:53:21PM +0200, Gerd Hoffmann wrote: > > > Hi, > > > > > > > The only way we can stop using "-device sga" is if we either have > > > > something explicit in the XML saying whether to use it or not, or > > > > alternatively have some indicator associated with the next versioned > > > > machine type that we can hook off. > > > > > > How does libvirt query the machine type properties? > > > > libvirt is able to query properties against i440FX-pcihost and > > q35-pcihost devices - currently we only look for one prop: > > > > static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsI440FXPCIHost[] = { > > { "pci-hole64-size", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE }, > > }; > > > > static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsQ35PCIHost[] = { > > { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE }, > > }; > > > > Not sure if that's the kind of thing you're looking for ? > > Looking for way for libvirt to figure whenever a specific machine type > has sga deprecated or not. So, if libvirt can query device properties, > can we just add a "deprecated" property to sga and flip that to true for > 3.1+ machine types? Unfortunately not, because libvirt probes everthing with '-machine none' Regards, Daniel
diff --git a/tests/Makefile.include b/tests/Makefile.include index 116e93cdb6..d524e1bdeb 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -257,7 +257,7 @@ check-qtest-i386-y += tests/hd-geo-test$(EXESUF) gcov-files-i386-y += hw/block/hd-geometry.c check-qtest-i386-y += tests/boot-order-test$(EXESUF) check-qtest-i386-y += tests/bios-tables-test$(EXESUF) -check-qtest-i386-y += tests/boot-serial-test$(EXESUF) +check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) check-qtest-i386-y += tests/rtc-test$(EXESUF) check-qtest-i386-y += tests/ipmi-kcs-test$(EXESUF)
This is only for x86* architecture. Signed-off-by: Juan Quintela <quintela@redhat.com> --- tests/Makefile.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)