Created attachment 237602 [details] [review]
installer: Remove redundant loop

Once the drivers are setup, they are setup for all scripts that can use
them. There is no need to keep setting them up for every script.

Also if there is going to be multiple scripts in the desktop profile
supporting same type (pre- or post-installable) of drivers, Boxes will be
copying them multiple times. That is not a big issue in case of
pre-installable drivers but in case of post-installable drivers, the
mkisofs commandline we use to setup postinstallation drivers will fail.

This patch fixes that potential issue as well.

Created attachment 237603 [details] [review]
installer: Clean-up supported devices related code

Maintain a single list for all supported devices, both for devices
supported by OS out of the box and ones added through additional drivers
by us.

Review of attachment 237602 [details] [review]:

Not sure why we were looping like that... (the short log could probably be more expressive)

Review of attachment 237602 [details] [review]:

Because I wrote bad code? :) Short log ideally needs to be under 50 chars so its hard to be expressive there while keeping that rule in mind. Suggestions welcome!

Review of attachment 237602 [details] [review]:

It's no big deal, this was just a note in passing, not important here.  'remove loop' is not very useful, 'Don't setup drivers for every script' or any higher level description like that would be an improvement.

Review of attachment 237603 [details] [review]:

This does not seem to change existing behaviour, so ACK. However, I'm not sure about the meaning of the devices listed under a <driver> node. Does this mean 'if you install this driver, then these devices will be available to your VM' or 'if your OS supports one of these devices, then you can use this driver for his device'?

if it means the former, then appending devices supported by the OS and devices supported by the drivers we are going to install make sense. If it means the latter, then we should not do the append.
I thought this meant the latter, which would make the appending wrong, but I haven't checked libosinfo to see if that's correct. Any idea?

Review of attachment 237603 [details] [review]:

Well libosinfo doesn't say either of these. libosinfo is simply reporting the list of devices that the device driver in question is for. Based on that, we make a decision in Boxes whether we use a particular device or not. E.g virtio-block is not supported by windows out of the box and Boxes knows this from lack of this device in Os.devices list. Boxes sees that there is however a driver available for this device so it sets up that driver and decides to use virtio-block.

What I'm trying to say is that libosinfo's view to this is lower level than Boxes'.

Review of attachment 237603 [details] [review]:

After looking at libosinfo, the semantics of the <driver> node seem to be  'if you install this driver, then these devices will be usable by your VM', so the former in my previous comment.

Attachment 237602 [details] pushed as e893997 - installer: Remove redundant loop
Attachment 237603 [details] pushed as e69add6 - installer: Clean-up supported devices related code