Re: [libvirt] [PATCH] test: remove s390 tests used only for testing usb and ide controllers
On Thu, Apr 30, 2015 at 04:40:46PM -0400, Laine Stump wrote:
Back in 2013, commit 877bc089 added in some tests that made sure no
error was generated on a domain definition that had an automatically
added usb controller if that domain didn't have a PCI bus to attach
the usb controller to. In particular, two s390-specific tests were
added, one with <controller type='usb' model='none'/> and another
(called "s390-piix-controllers") that had both usb and ide
controllers, but nothing attached to them.
It was a synthetic test (I never ran the command line) meant to check
the fix for starting s390 guests:
https://www.redhat.com/archives/libvir-list/2013-April/msg01920.html
which I posted instead of this proposed patch
https://www.redhat.com/archives/libvir-list/2013-April/msg01925.html
Then in February of this year, commit 09ab9dcc eliminated the
annoying
auto-adding of a usb device for s390 and s390x machines, stating:
"Since s390 does not support usb the default creation of a usb
controller for a domain should not occur."
Since s390 doesn't support usb and usb controllers aren't added to
s390 domain definitions automatically, there is no reason to have the
tests with a usb controller and expect them to succeed.
QEMU seems to ignore the -usb parameter, so the only reason would be:
'It worked before'. (Unless running the qemu-system-s390x binary without
an guest on x86_64 does not give me results matching the real-world
usage).
I'm okay with breaking the controllers with explicit PCI addresses,
since libvirt stopped adding those almost 2 years ago.
Erroring out on bare <controller type='usb'> would break domains that
worked just three releases ago (the tests and the fix was added
precisely to avoid that).
Also, USB controller model='none' should be allowed.
And since the
only reference of an IDE controller wrt s390 that I've found is in the
one test case mentioned above, and the commit log that added it
specifically mentions the purpose to be quieting error messages on
machines with no PCI bus, I'm assuming that the s390 also doesn't
support IDE controllers. Based on that reasoning
The IDE controller seem to be added only if something uses the IDE bus
(the problem seems to be the disk target starting with 'hd')
Both tests can be fixed to avoid that.
(and the fact that
s390-piix-controllers causes a test error for an upcoming patch),
Honestly, that is not a great reason to remove a test :)
this patch removes those two tests.
Jan
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)