[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 03/10] Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]