The QEMU driver internal API for interacting with the monitor console takes a 'struct qemud_driver *' parameter. This is a problem because any access to this struct must be protected by a lock, but we do not want to hold the global driver lock for all monitor operations since it would destroy any hope of concurrency. Fortunately tracing down many levels of call reveals this parameter is totally unused, so this patch removes it from a tonne of internal APIs qemu_driver.c | 109 ++++++++++++++++++++++++++-------------------------------- 1 file changed, 49 insertions(+), 60 deletions(-) Daniel diff --git a/src/qemu_driver.c b/src/qemu_driver.c --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -136,8 +136,7 @@ static void qemudShutdownVMDaemon(virCon static int qemudDomainGetMaxVcpus(virDomainPtr dom); -static int qemudMonitorCommand (const struct qemud_driver *driver, - const virDomainObjPtr vm, +static int qemudMonitorCommand (const virDomainObjPtr vm, const char *cmd, char **reply); @@ -386,14 +385,12 @@ qemudShutdown(void) { /* Return -1 for error, 1 to continue reading and 0 for success */ typedef int qemudHandlerMonitorOutput(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *output, int fd); static int qemudReadMonitorOutput(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, int fd, char *buf, @@ -452,7 +449,7 @@ qemudReadMonitorOutput(virConnectPtr con } else { got += ret; buf[got] = '\0'; - if ((ret = func(conn, driver, vm, buf, fd)) != 1) + if ((ret = func(conn, vm, buf, fd)) != 1) return ret; } } @@ -466,7 +463,6 @@ qemudReadMonitorOutput(virConnectPtr con static int qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED, - struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *output, int fd) @@ -480,7 +476,6 @@ qemudCheckMonitorPrompt(virConnectPtr co } static int qemudOpenMonitor(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *monitor) { int monfd; @@ -504,7 +499,7 @@ static int qemudOpenMonitor(virConnectPt } ret = qemudReadMonitorOutput(conn, - driver, vm, monfd, + vm, monfd, buf, sizeof(buf), qemudCheckMonitorPrompt, "monitor"); @@ -564,7 +559,6 @@ static int qemudExtractMonitorPath(virCo static int qemudFindCharDevicePTYs(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *output, int fd ATTRIBUTE_UNUSED) @@ -603,7 +597,7 @@ qemudFindCharDevicePTYs(virConnectPtr co } /* Got them all, so now open the monitor console */ - ret = qemudOpenMonitor(conn, driver, vm, monitor); + ret = qemudOpenMonitor(conn, vm, monitor); cleanup: VIR_FREE(monitor); @@ -611,11 +605,10 @@ cleanup: } static int qemudWaitForMonitor(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm) { char buf[1024]; /* Plenty of space to get startup greeting */ int ret = qemudReadMonitorOutput(conn, - driver, vm, vm->stderr_fd, + vm, vm->stderr_fd, buf, sizeof(buf), qemudFindCharDevicePTYs, "console"); @@ -632,7 +625,6 @@ static int qemudWaitForMonitor(virConnec static int qemudDetectVcpuPIDs(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm) { char *qemucpus = NULL; char *line; @@ -656,7 +648,7 @@ qemudDetectVcpuPIDs(virConnectPtr conn, return 0; } - if (qemudMonitorCommand(driver, vm, "info cpus", &qemucpus) < 0) { + if (qemudMonitorCommand(vm, "info cpus", &qemucpus) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to fetch CPU thread info")); VIR_FREE(vm->vcpupids); @@ -732,7 +724,6 @@ error: static int qemudInitCpus(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom) { char *info = NULL; @@ -772,7 +763,7 @@ qemudInitCpus(virConnectPtr conn, if (migrateFrom == NULL) { /* Allow the CPUS to start executing */ - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); return -1; @@ -999,9 +990,9 @@ static int qemudStartVMDaemon(virConnect VIR_EVENT_HANDLE_HANGUP, qemudDispatchVMEvent, driver, NULL)) < 0) || - (qemudWaitForMonitor(conn, driver, vm) < 0) || - (qemudDetectVcpuPIDs(conn, driver, vm) < 0) || - (qemudInitCpus(conn, driver, vm, migrateFrom) < 0)) { + (qemudWaitForMonitor(conn, vm) < 0) || + (qemudDetectVcpuPIDs(conn, vm) < 0) || + (qemudInitCpus(conn, vm, migrateFrom) < 0)) { qemudShutdownVMDaemon(conn, driver, vm); return -1; } @@ -1143,8 +1134,7 @@ cleanup: } static int -qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED, - const virDomainObjPtr vm, +qemudMonitorCommand (const virDomainObjPtr vm, const char *cmd, char **reply) { int size = 0; @@ -1683,7 +1673,7 @@ static int qemudDomainSuspend(virDomainP goto cleanup; } if (vm->state != VIR_DOMAIN_PAUSED) { - if (qemudMonitorCommand(driver, vm, "stop", &info) < 0) { + if (qemudMonitorCommand(vm, "stop", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("suspend operation failed")); goto cleanup; @@ -1732,7 +1722,7 @@ static int qemudDomainResume(virDomainPt goto cleanup; } if (vm->state == VIR_DOMAIN_PAUSED) { - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("resume operation failed")); goto cleanup; @@ -1774,7 +1764,7 @@ static int qemudDomainShutdown(virDomain goto cleanup; } - if (qemudMonitorCommand(driver, vm, "system_powerdown", &info) < 0) { + if (qemudMonitorCommand(vm, "system_powerdown", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("shutdown operation failed")); goto cleanup; @@ -2171,7 +2161,7 @@ static int qemudDomainSave(virDomainPtr goto cleanup; } - if (qemudMonitorCommand(driver, vm, command, &info) < 0) { + if (qemudMonitorCommand(vm, command, &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("migrate operation failed")); goto cleanup; @@ -2541,7 +2531,7 @@ static int qemudDomainRestore(virConnect /* If it was running before, resume it now. */ if (header.was_running) { char *info; - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); goto cleanup; @@ -2821,7 +2811,6 @@ static char *qemudDiskDeviceName(const v } static int qemudDomainChangeEjectableMedia(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -2905,7 +2894,7 @@ static int qemudDomainChangeEjectableMed } VIR_FREE(devname); - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot change cdrom media")); VIR_FREE(cmd); @@ -2934,7 +2923,6 @@ static int qemudDomainChangeEjectableMed } static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -2971,7 +2959,7 @@ static int qemudDomainAttachPciDiskDevic return ret; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("cannot attach %s disk"), type); VIR_FREE(cmd); @@ -3006,7 +2994,6 @@ static int qemudDomainAttachPciDiskDevic } static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3041,7 +3028,7 @@ static int qemudDomainAttachUsbMassstora return ret; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach usb disk")); VIR_FREE(cmd); @@ -3070,7 +3057,6 @@ static int qemudDomainAttachUsbMassstora } static int qemudDomainAttachHostDevice(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3096,7 +3082,7 @@ static int qemudDomainAttachHostDevice(v return -1; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach usb device")); VIR_FREE(cmd); @@ -3132,12 +3118,14 @@ static int qemudDomainAttachDevice(virDo qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); - goto cleanup; - } - - if (!virDomainIsActive(vm)) { + qemuDriverUnlock(driver); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + qemuDriverUnlock(driver); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot attach device on inactive domain")); goto cleanup; @@ -3146,6 +3134,7 @@ static int qemudDomainAttachDevice(virDo dev = virDomainDeviceDefParse(dom->conn, driver->caps, vm->def, xml); + qemuDriverUnlock(driver); if (dev == NULL) goto cleanup; @@ -3154,14 +3143,14 @@ static int qemudDomainAttachDevice(virDo switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemudDomainChangeEjectableMedia(dom->conn, driver, vm, dev); + ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev); break; case VIR_DOMAIN_DISK_DEVICE_DISK: if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { - ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm, dev); + ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev); } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { - ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, dev); + ret = qemudDomainAttachPciDiskDevice(dom->conn, vm, dev); } break; default: @@ -3172,7 +3161,7 @@ static int qemudDomainAttachDevice(virDo } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - ret = qemudDomainAttachHostDevice(dom->conn, driver, vm, dev); + ret = qemudDomainAttachHostDevice(dom->conn, vm, dev); } else { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("this device type cannot be attached")); @@ -3183,12 +3172,10 @@ cleanup: virDomainDeviceDefFree(dev); if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { int i, ret = -1; @@ -3222,7 +3209,7 @@ static int qemudDomainDetachPciDiskDevic goto cleanup; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("failed to execute detach disk %s command"), detach->dst); goto cleanup; @@ -3268,18 +3255,21 @@ static int qemudDomainDetachDevice(virDo qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); - goto cleanup; - } - - if (!virDomainIsActive(vm)) { + qemuDriverUnlock(driver); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + "%s", _("no domain with matching uuid")); + goto cleanup; + } + + if (!virDomainIsActive(vm)) { + qemuDriverUnlock(driver); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot attach device on inactive domain")); goto cleanup; } dev = virDomainDeviceDefParse(dom->conn, driver->caps, vm->def, xml); + qemuDriverUnlock(driver); if (dev == NULL) goto cleanup; @@ -3288,7 +3278,7 @@ static int qemudDomainDetachDevice(virDo dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) - ret = qemudDomainDetachPciDiskDevice(dom->conn, driver, vm, dev); + ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev); else qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("only SCSI or virtio disk device can be detached dynamically")); @@ -3297,7 +3287,6 @@ cleanup: virDomainDeviceDefFree(dev); if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } @@ -3444,7 +3433,7 @@ qemudDomainBlockStats (virDomainPtr dom, goto cleanup; len = strlen (qemu_dev_name); - if (qemudMonitorCommand (driver, vm, "info blockstats", &info) < 0) { + if (qemudMonitorCommand (vm, "info blockstats", &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("'info blockstats' command failed")); goto cleanup; @@ -3710,7 +3699,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, /* Issue the memsave command. */ snprintf (cmd, sizeof cmd, "memsave %llu %zi \"%s\"", offset, size, tmp); - if (qemudMonitorCommand (driver, vm, cmd, &info) < 0) { + if (qemudMonitorCommand (vm, cmd, &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("'memsave' command failed")); goto cleanup; @@ -4028,7 +4017,7 @@ qemudDomainMigratePerform (virDomainPtr if (!(flags & VIR_MIGRATE_LIVE)) { /* Pause domain for non-live migration */ snprintf(cmd, sizeof cmd, "%s", "stop"); - qemudMonitorCommand (driver, vm, cmd, &info); + qemudMonitorCommand (vm, cmd, &info); DEBUG ("stop reply: %s", info); VIR_FREE(info); @@ -4043,7 +4032,7 @@ qemudDomainMigratePerform (virDomainPtr if (resource > 0) { /* Issue migrate_set_speed command. Don't worry if it fails. */ snprintf (cmd, sizeof cmd, "migrate_set_speed %lum", resource); - qemudMonitorCommand (driver, vm, cmd, &info); + qemudMonitorCommand (vm, cmd, &info); DEBUG ("migrate_set_speed reply: %s", info); VIR_FREE (info); @@ -4059,7 +4048,7 @@ qemudDomainMigratePerform (virDomainPtr snprintf (cmd, sizeof cmd, "migrate \"%s\"", safe_uri); VIR_FREE (safe_uri); - if (qemudMonitorCommand (driver, vm, cmd, &info) < 0) { + if (qemudMonitorCommand (vm, cmd, &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("migrate operation failed")); goto cleanup; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list