Hi Dan,
That's definitely be a useful feature. Some comments...
> @@ -453,7 +454,7 @@ static int qemudOpenMonitor(virConnectPt
> char buf[1024];
> int ret = -1;
>
> - if (!(monfd = open(monitor, O_RDWR))) {
> + if (!(monfd = open(monitor, O_NOCTTY |O_RDWR))) {
Is this just to ensure portability or does it change the behavior?
> @@ -1365,13 +1360,35 @@ static int qemudMonitorCommand(struct qe
> + retry1:
> + if (write(vm->logfile, buf, strlen(buf)) < 0) {
> + /* Log, but ignore failures to write logfile for VM */
> + if (errno == EINTR)
> + goto retry1;
> + qemudLog(QEMUD_WARN, "Unable to log VM console data: %s",
> + strerror(errno));
> + }
> +
> *reply = buf;
> return 0;
> +
> + error:
> + if (buf) {
> + retry2:
> + if (write(vm->logfile, buf, strlen(buf)) < 0) {
> + /* Log, but ignore failures to write logfile for VM */
> + if (errno == EINTR)
> + goto retry2;
> + qemudLog(QEMUD_WARN, "Unable to log VM console data: %s",
> + strerror(errno));
> + }
> + free(buf);
> + }
> + return -1;
I think both of these retry loops could be replaced with safewrite
from util.c:
if (safewrite(vm->logfile, buf, strlen(buf)) != strlen(buf))
qemudLog(...)
> +static int qemudDomainChangeCDROM(virDomainPtr dom,
> + struct qemud_vm *vm,
> + struct qemud_vm_disk_def *olddisk,
> + struct qemud_vm_disk_def *newdisk) {
> + struct qemud_driver *driver = (struct qemud_driver
> *)dom->conn->privateData;
> + char *cmd;
> + char *reply;
> + /* XXX QEMU only supports a single CDROM for now */
> + /*cmd = malloc(strlen("change ") + strlen(olddisk->dst) + 1 +
> strlen(newdisk->src) + 2);*/
> + cmd = malloc(strlen("change ") + strlen("cdrom") + 1 +
> strlen(newdisk->src) + 2);
> + if (!cmd) {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, "monitor
> command");
> + return -1;
> + }
> + strcpy(cmd, "change ");
> + /* XXX QEMU only supports a single CDROM for now */
> + /*strcat(cmd, olddisk->dst);*/
> + strcat(cmd, "cdrom");
> + strcat(cmd, " ");
> + strcat(cmd, newdisk->src);
> + strcat(cmd, "\n");
Commands should be terminated with "\r", otherwise the terminal layer
replaces "\n" -> "\r\n", and bugs in earlier qemu means it would
execute the command twice. Recent qemu will still print the "(qemu) "
prompt twice in this case, which might confuse qemudMonitorCommand
into thinking that a subsequent command is finished before it's even
started.
Unless the O_NOCTTY somehow fixes that?
-jim
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
