On Mon, Oct 27, 2014 at 01:42:59PM +0100, Martin Kletzander wrote: > On Fri, Oct 24, 2014 at 11:17:48AM +0100, Daniel P. Berrange wrote: > >On Fri, Oct 24, 2014 at 12:15:24PM +0200, Martin Kletzander wrote: > >>On Fri, Oct 24, 2014 at 07:16:44AM +0200, Michal Privoznik wrote: > >>>On 22.10.2014 13:58, Martin Kletzander wrote: > >>>>Hi everyone, > >>>> > >>>>I had this idea that since we are probing QEMU binaries for devices > >>>>using 'qom-list-types', we could store that data in the capabilities > >>>>and check whether device models are supported before starting QEMU. > >>>>We do that for _some newer_ devices, but this would be global. It > >>>>would help out particularly with devices like the following one, for > >>>>example: > >>>> > >>>><interface type='network'> > >>>> <source network='default'/> > >>>> <model type='non-existing_device'/> > >>>></interface> > >>>> > >>>>where we simply construct QEMU command-line with that device model and > >>>>it then properly errors out: > >>>> > >>>>error: internal error: process exited while connecting to monitor: > >>>>qemu-system-x86_64: -device non-existing_device,...: Parameter > >>>>'driver' expects device type > >>>> > >>>>This would be easy to achieve with current data unless there are some > >>>>models hidden from qom-list-types' output. I hope there are none. > >>> > >>>Well, this may vary among distros. While one distro lives with bleeding > >>>edge, the other may just backport some patches. Even those adding new > >>>devices. So I wouldn't rely on that. > >>> > >> > >>I don't quite get what you mean. We'd be still compatible with old > >>QEMU binaries that don't provide the data. > >> > >>>> > >>>>When I checked the output of 'qemu-kvm -device \?', the devices listed > >>>>there are separated into categories and have buses assigned to them > >>>>and that lead me to another idea. What if that data is added to > >>>>qom-list-types' output and used in libvirt as well? We could then > >>>>solve another annoying cases like misusing devices or plugging them > >>>>into unsupported buses. Although I don't know any person who would do > >>>>such a thing, even when solving the first idea, there'd still be a > >>>>possibility to do a thing like: > >>>> > >>>><interface type='network'> > >>>> <source network='default'/> > >>>> <model type='AC97'/> > >>>></interface> > >>>> > >>>>This idea is obviously meant for the QEMU driver, but if there's > >>>>something similar in other drivers, it might be useful as well. > >>>> > >>>>I'd be interested in any feedback and welcome any ideas to whether it > >>>>is useful, how the storing should be done or even if it's something we > >>>>want to have or not. > >>> > >>>So do you suggest that the check should be done on domain startup or > >>>domain define time? It can't be the latter - libvirt should expose all > >>>the domains it knows of - even these that can't be started. The idea > >>>was, users can downgrade qemu and libvirt should cope with that. > >>>If you, however, intend to do this at runtime, I see this as a gap in > >>>our capabilites querying/checking. Or am I missing something here? > >>> > >> > >>Wherever the capabilities are checked now. Of course you cannot do > >>this when defining the domain. This would be just an additional check > >>when building the command line. And the gap is what I'm suggesting to > >>fix. Just to be clear, the only output of such patches would be a > >>nicer error message without relying on starting QEMU, nothing more. > > > >If we can get better error reporting of devices that QEMU does not > >support, when we start QEMU that is a win for me. QEMU has a really > >awful error message for devices that don't exist. > > > > Yes, because it thinks the device name is a parameter because no such > device exists or something like that. Thanks for that, I'll > definitely try to come up with a unified solution. > > However, my question extends to the second use case, which is even > more corner-case (I really don't think anyone is going to try that and > bug us about it), but I want to make sure, does it make sense to > extend qemu's qom-list-types with device category (and supported > buses) only so we can error out with: > > Not supported: Cannot use AC97 as an interface model. > > instead of: > > error: internal error: process exited while connecting to monitor: > qemu-system-x86_64: -device AC97,...: Property '.netdev' not found > > I, personally, think it is a bit of an overkill. I just want to have > more general opinion. As a general rule we whitelist the models we allow through from the XML parsing. IIRC, the <interface> model is the only place we don't explicitly whitelist them. If we fix that, then it will be impossible for libvirt to generate a command line where we pass a NIC model where QEMU wants a disk model or vica-verca. So I don't think we need todo any further validation in this respect Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list