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

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.