[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 10/13] qemu: remove test for allowing ide controller in s390, rename usb tests




On 05/05/2015 02:03 PM, 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. This was done because, at that time, libvirt
> was automatically adding a usb controller to *any* domain definition
> that didn't have one.  Along with permitting the controller, two
> s390-specific tests were added to ensure this behavior was maintained
> - 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.
> 
> 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."
> 
> Although, as verified here, the s390 doesn't support usb, and usb
> controllers aren't currently added to s390 domain definitions
> automatically, there are likely still some domain definitions in the
> wild that have a usb controller (which was added *by libvirt*, not by
> the user), so we will keep the tests verifying that behavior for
> now. But this patch changes the names of the tests to reflect that
> they don't actually contain a valid s390 config; this way future
> developers won't propagate the incorrect idea that an s390 virtual
> machine can have a USB (or IDE) bus.
> 
> In the case of the IDE controller, though, libvirt has never
> automatically added an IDE controller unless a user added an IDE disk
> (which itself would have caused an error), and we specifically *do*
> want to begin generating an error when someone tries to add an IDE
> controller to a domain that can't support one. For that reason, while
> renaming the sz390-piix-controllers patch, this patch removes the
> <controller type='ide'...> from it (otherwise the upcoming patch would
> break make check)
> ---
>  ...rollers.args => qemuxml2argv-s390-allow-bogus-usb-controller.args} | 0
>  ...ntrollers.xml => qemuxml2argv-s390-allow-bogus-usb-controller.xml} | 3 ---
>  ...s390-usb-none.args => qemuxml2argv-s390-allow-bogus-usb-none.args} | 0
>  ...v-s390-usb-none.xml => qemuxml2argv-s390-allow-bogus-usb-none.xml} | 0
>  tests/qemuxml2argvtest.c                                              | 4 ++--
>  5 files changed, 2 insertions(+), 5 deletions(-)
>  rename tests/qemuxml2argvdata/{qemuxml2argv-s390-piix-controllers.args => qemuxml2argv-s390-allow-bogus-usb-controller.args} (100%)
>  rename tests/qemuxml2argvdata/{qemuxml2argv-s390-piix-controllers.xml => qemuxml2argv-s390-allow-bogus-usb-controller.xml} (86%)
>  rename tests/qemuxml2argvdata/{qemuxml2argv-s390-usb-none.args => qemuxml2argv-s390-allow-bogus-usb-none.args} (100%)
>  rename tests/qemuxml2argvdata/{qemuxml2argv-s390-usb-none.xml => qemuxml2argv-s390-allow-bogus-usb-none.xml} (100%)
> 

ACK - hopefully anyone who looks to change them afterwards reads the
commit message ;-)

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]