devices: allow bridge devclass assignment#474
Conversation
| """ | ||
| if ( | ||
| assignment.devclass not in ("pci", "testclass", "block") | ||
| assignment.devclass not in self._vm.app.list_deviceclass() |
There was a problem hiding this comment.
This doesn't make any sense. It guards which devclasses are allowed as "required", and here you replace it basically with "any". In that case, simply remove this check (and the one below too, as with the change is similarly useless).
But then, ensure at the backend side it's really checked that for example "usb" are not allowed as "required"
There was a problem hiding this comment.
Is there not one that's supposed to exclude tests classes?
There was a problem hiding this comment.
In all the cases, the spelling is a hell in the new devices API, similar to discussions on Matrix.
There was a problem hiding this comment.
@marmarek are you ok to introduce a flag in device class to notify that it is "required" type class?
There was a problem hiding this comment.
I don't think we have a place to add meta-data of a device class as a whole. But maybe, a device could have a property with list of supported assignment modes? @piotrbartman what do you think?
There was a problem hiding this comment.
I'm going into the way of exposing via admin.DeviceClass.List pci+required. It extends the protocol and allows client to know without relying only on back. Especially, it allows to stop hardcoding values. So each device class will have this parameter easily retrieved on call for list of classes.
There was a problem hiding this comment.
I'm going into the way of exposing via admin.DeviceClass.List .
That's not a good idea, because it breaks existing clients as they will interpret it whole as a device class (once dom0 is updated, all users, including sys-gui etc will break). This could be solved by adding extra info only if some specific argument is given, like admin.DeviceClass.List+details. But more importantly, it isn't a long term solution, if any other property would be needed, you still don't have a way to send it.
And then, both on the backend and frontend side we need a mechanism to pass this info. On the backend, it could be a class property of relevant DeviceInfo subclass maybe? On the frontend side, some function to return list of classes with extra metadata?
Anyway, I would like to hear @piotrbartman opinion first. Because setting it on individual devices possibly would be more flexible. Or maybe there is some other option I didn't think of.
There was a problem hiding this comment.
Is there not one that's supposed to exclude tests classes?
Only this classes are allowed as "required"
But maybe, a device could have a property with list of supported assignment modes? @piotrbartman what do you think?
We definitely have to keep info from this table somewhere "in the code", available from API. It could be per device.

And then, both on the backend and frontend side we need a mechanism to pass this info. On the backend, it could be a class property of relevant DeviceInfo subclass maybe? On the frontend side, some function to return list of classes with extra metadata?
at first glance it seems okay, but I need to think it over
There was a problem hiding this comment.
The problem is supporting other classes of devices that are not in the codebase => bridges and maybe some other kind too.
Few reformats