Cole Robinson wrote:
> Daniel P. Berrange wrote:
>> On Mon, Dec 01, 2008 at 05:36:34PM -0500, Cole Robinson wrote:
>>> The attached patch cleans up output we read from the qemu monitor. This
>>> is a simplified form of a patch I posted awhile ago. From the patch:
>>>
>>> /* The monitor doesn't dump clean output after we have written to
>>> * it. Every character we write dumps a bunch of useless stuff,
>>> * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
>>> * Try to throw away everything before the first full command
>>> * occurence, and inbetween the command and the newline starting
>>> * the response
>>> */
>>>
>
> <snip>
>
>>
>> This loooks a little overly complex to me, doesn't handle alloction failures
>> in strdup correctly & leaks tmpbuf.
>
> Argh, yeah, I'm always botching this.
>
>> if ((commptr = strstr(buf, cmd)))
>> memmove(buf, commptr, strlen(commptr)+1);
>>
>
> Yes that's much simpler, though it doesn't take into account the control
> characters after the command string and before the newline starting the
> command response.
>
> We could just do:
>
> if ((commptr = strstr(buf, cmd)))
> memmove(buf, commptr, strlen(commptr)+1);
> if ((dupptr = strchr(buf, '\n'))
> memmove(buf+strlen(cmd), dupptr, strlen(dupptr)+1);
>
> I was also trying to avoid having the command string in the 'reply', but
> it was an unnecessary restriction.
>
> Updated patch attached. It also fixes an error check from the blockstats
> monitor command which wouldn't have worked in the first place, but
> should be accurate now.
>
Just want to bump this. I'd like to commit this if there are no
objections.
Thanks,
Cole
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 0130d61..0c4226c 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1145,7 +1145,23 @@ qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED,
/* Look for QEMU prompt to indicate completion */
if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
- tmp[0] = '\0';
+ char *commptr = NULL, *nlptr = NULL;
+
+ /* Preserve the newline */
+ tmp[1] = '\0';
+
+ /* The monitor doesn't dump clean output after we have written to
+ * it. Every character we write dumps a bunch of useless stuff,
+ * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
+ * Try to throw away everything before the first full command
+ * occurence, and inbetween the command and the newline starting
+ * the response
+ */
+ if ((commptr = strstr(buf, cmd)))
+ memmove(buf, commptr, strlen(commptr)+1);
+ if ((nlptr = strchr(buf, '\n')))
+ memmove(buf+strlen(cmd), nlptr, strlen(nlptr)+1);
+
break;
}
pollagain:
@@ -2596,7 +2612,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("cannot change cdrom media"));
+ "%s", _("could not change cdrom media"));
VIR_FREE(cmd);
return -1;
}
@@ -2607,7 +2623,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
DEBUG ("ejectable media change reply: %s", reply);
if (strstr(reply, "\ndevice ")) {
qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("changing cdrom media failed"));
+ _("changing cdrom media failed: %s"), reply);
VIR_FREE(reply);
VIR_FREE(cmd);
return -1;
@@ -3143,7 +3159,7 @@ qemudDomainBlockStats (virDomainPtr dom,
* unlikely to be the name of a block device, we can use this
* to detect if qemu supports the command.
*/
- if (STRPREFIX (info, "info ")) {
+ if (strstr(info, "\ninfo ")) {
qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
"%s",
_("'info blockstats' not supported by this qemu"));
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list