On Thu, Dec 06, 2012 at 08:00:56AM -0600, Anthony Liguori wrote: > "Daniel P. Berrange" <berra...@redhat.com> writes: > > > On Tue, Dec 04, 2012 at 03:44:54PM -0600, Anthony Liguori wrote: > >> "Daniel P. Berrange" <berra...@redhat.com> writes: > >> > >> > On Tue, Dec 04, 2012 at 01:13:46PM -0600, Anthony Liguori wrote: > >> >> "Daniel P. Berrange" <berra...@redhat.com> writes: > >> >> > >> >> > > >> >> > In the absence of any way to detect it via QMP, libvirt should > >> >> > fallback > >> >> > to hardcoding it based on the version number. This presumes that QEMU > >> >> > was > >> >> > built with it enabled in configure, but we've no other option for > >> >> > current > >> >> > released 1.2/1.3 versions. > >> >> > >> >> echo quit | qemu -machine none -S -monitor stdio -vnc none -sandbox on > >> >> > >> >> A non-zero execute means QEMU doesn't support the option. This will > >> >> work for any new command line option introduction and can be considered > >> >> a "supported" way of probing for whether options are supported. > >> > > >> > One of the significant benefits to libvirt of the QMP based feature > >> > detection, was that we no longer have to invoke QEMU multiple times > >> > to query different data. I don't want to regress in this regard, > >> > because invoking QEMU many times has a noticable performance impact > >> > for some applications eg virt-sandbox were even 100ms delays are > >> > relevant. So while what you describe does work, I don't think it > >> > is a satisfactory approach for libvirt. > >> > >> Okay, so in terms of what exists today, I don't have a better option. > >> But we could add: > >> > >> { 'enum': 'ConfigEntryType', > >> 'data': [ 'number', 'string', 'bool', 'size' ] } > >> > >> { 'type': 'ConfigEntry', > >> 'data': { 'name': 'str', 'type': 'ConfigEntryType' } } > >> > >> { 'type': 'ConfigSection', > >> 'data': { 'name': 'str', 'fields': [ 'ConfigEntry' ] } } > >> > >> { 'command': 'query-config-schema', > >> 'returns': [ 'ConfigSection' ] } > > > >> > >> This technically introspects config sections but obviously could be used > >> to detect the availability of -sandbox. > >> > >> If it's useful, I can take a quick swing at implementing (or someone > >> else certainly could). > > > > I'm not sure I entirely understand what information a 'ConfigSection' > > would represent. By config here, do you mean any command line argument > > or something else ? > > We no longer should be adding command line arguments that don't use > QemuOpts and have a equivalent -readconfig syntax. We could even > eliminate new options and do something like: > > qemu -conf sandbox:enable=on > > But that's not user friendly so we'll stick with adding higher level > options like -sandbox. > > So what I'm proposing is to introspection on what -readconfig supports > and then from that, you can infer when new higher level command line > arguments are added. > > > Could you give a short example of the actual JSON > > you envisage returning for this schema. Your suggestion sounds good, > > but I want to make sure I'm not mis-understanding things :-) > > [ { 'name': 'sandbox', > 'fields': [ { 'name': 'enable', 'type': 'bool' } ] }, > { 'name': 'add-fd', > 'fields': [ { 'name': 'fd', 'type': 'number' }, > { 'name': 'set', 'type': 'number' }, > { 'name': 'opaque', 'type': 'str' } ] }, > ... > ]
Ok, that all sounds like a good idea to me - it should address one of the major gaps in the new QMP based capabilities detection. 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 :|