The commit message is one long sentence and a bit hard to read. On Mon, Jun 18, 2018 at 07:56:35AM -0400, John Ferlan wrote:
Commit id f0a23c0c3 introduced qemuMonitorCreateObjectProps to manage wrapping the object name and alias; however, calling
Listing the commit that broke it can be in a separate paragraph.
virJSONValueObjectCreateVArgs and checking a unary ! return status meant when the CreateVAArgs function returned zero, then rather than generating a @propsret, the jump to cleanup would
Rather than explaing the semantics of C operators, it would be helpful to mention that virJSONValueObjectCreateVArgs returns -1 on failure and 0 when we did not need to add any arguments.
result in an unexpected failure and cause virsh to issue the "error: An error occurred, but the cause is unknown" error message for something like "virsh iothreadadd $DOM 10" (assuming that IOThread 10 didn't already exist).
Putting the example error message and virsh commands on separate lines would make the commit message easier to read.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/qemu/qemu_monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b29672d4f1..d6771c1d52 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3051,7 +3051,7 @@ qemuMonitorCreateObjectProps(virJSONValuePtr *propsret, va_start(args, alias); - if (!(virJSONValueObjectCreateVArgs(&props, args))) + if (virJSONValueObjectCreateVArgs(&props, args) < 0)
Looks like qemuDomainHotplugAddIOThread is the only user of qemuMonitorCreateObjectProps passing NULL for args. Can it be tested by our test suite? With more newlines in the commit message: Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list