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