Re: [PATCH 1/3] qemu_agent: Create file system freeze and thaw functions

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 24.01.2012 19:21, Jiri Denemark wrote:
> On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote:

>> +
>> +    if (qemuAgentCommand(mon, cmd, &reply) < 0 ||
>> +        qemuAgentCheckError(cmd, reply) < 0)
>> +        goto cleanup;
>> +
>> +    virJSONValueObjectGetNumberInt(reply, "return", &ret);
> 
> Doesn't the above function need to be checked for errors? That is, what if
> there's no "return" key in the reply? In json monitor code, we usually have
> something like the following code:
> 
>         if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
>             qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("guest-fsfreeze-freeze reply was missing return"
>                               " data"));
>         }
> 

In fact this check is done in qemuAgentCheckError(); which reports error
as well. But I agree I should check if returned value is integer and
throw an error. I have squashed this into my local branch:

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 3a5cc4b..9df5546 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1137,7 +1137,10 @@ int qemuAgentFSFreeze(qemuAgentPtr mon)
         qemuAgentCheckError(cmd, reply) < 0)
         goto cleanup;

-    virJSONValueObjectGetNumberInt(reply, "return", &ret);
+    if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
+        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("malformed return value"));
+    }

 cleanup:
     virJSONValueFree(cmd);
@@ -1171,7 +1174,10 @@ int qemuAgentFSThaw(qemuAgentPtr mon)
         qemuAgentCheckError(cmd, reply) < 0)
         goto cleanup;

-    virJSONValueObjectGetNumberInt(reply, "return", &ret);
+    if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
+        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("malformed return value"));
+    }

 cleanup:
     virJSONValueFree(cmd);

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list