On Fri, Jul 12, 2013 at 10:37:12AM -0600, Eric Blake wrote: > On 07/12/2013 10:34 AM, John Ferlan wrote: > > On 07/12/2013 11:57 AM, Eric Blake wrote: > >> On 07/12/2013 06:35 AM, Daniel P. Berrange wrote: > >>> On Thu, Jul 11, 2013 at 07:55:53PM -0400, John Ferlan wrote: > >>>> Add a new qemuMonitorJSONSetObjectProperty() method to support invocation > >>>> of the 'qom-set' JSON monitor command with a provided path, property, and > >>>> expected data type to set. > >>>> > >>>> The test code uses the same "/machine/i440fx" property as the get test and > >>>> attempts to set the "realized" property to "true" (which it should be set > >>>> at anyway). > >>>> --- > >>>> src/qemu/qemu_monitor_json.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ > >>>> src/qemu/qemu_monitor_json.h | 6 +++++ > >>>> tests/qemumonitorjsontest.c | 59 +++++++++++++++++++++++++++++++++++++++++ > >>>> 3 files changed, 127 insertions(+) > >>> > >>> ACK > >> > >> Again, shouldn't this function be marked static, and the .h file left alone? > >> > > > > I can go either way on this - without the .h changes, we don't have the > > test though. So whichever way is preferred - I'll go with it... > > Ah, so the test case is the reason you are exporting the file. That's > reason enough to leave the patch body as-is, but do amend the commit > message to mention this. It might also be worth a comment in the .h > that the function is public only for the testsuite, so we aren't tempted > to use it from qemu_driver.c later. This isn't specific to this api - nothing in qemu_monitor_json.h should be used directly by qemu_driver.c It is exclusively for use by the qemu_monitor.c file & the test suite. 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