On 02/10/2012 06:43 AM, Michal Privoznik wrote:
Pretty light on the commit message.
> ---
> src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.c | 20 +++++++++++++++
> src/qemu/qemu_monitor.h | 2 +
> src/qemu/qemu_monitor_json.c | 21 ++++++++++++++++
> src/qemu/qemu_monitor_json.h | 2 +
> src/qemu/qemu_monitor_text.c | 26 ++++++++++++++++++++
> src/qemu/qemu_monitor_text.h | 2 +
I wouldn't bother touching qemu_monitor_text.[ch], but make this
JSON-only. Why? Because this is a qemu-1.1 feature; and because you
can't pmsuspend the guest without guest agent, which is JSON-only. The
only reason to support a text command is if we are ever targetting an
older qemu (pre-0.15) where json did not exist, or where text was more
featured than json.
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + qemuReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + if (!virDomainObjIsActive(vm)) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("domain is not running"));
> + goto cleanup;
> + }
No need to check for isactive here,
> +
> + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> + goto cleanup;
> +
> + if (!virDomainObjIsActive(vm)) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("domain is not running"));
> + goto endjob;
as well as here. The second one is mandatory (since obtaining the job
temporarily drops lock, during which time the guest can change state),
and the first one would only avoid obtaining the job just to still end
in error, but penalizes the common case of no error.
> + }
> +
> + priv = vm->privateData;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> + ret = qemuMonitorSystemWakeup(priv->mon);
Should we be checking for QEMU_CAPS_WAKEUP here, and issuing a nicer
error message if the command is not present?
> + qemuDomainObjExitMonitor(driver, vm);
> +
> +endjob:
> + if (qemuDomainObjEndJob(driver, vm) == 0)
> + vm = NULL;
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + return ret;
> +}
> +
> static virDriver qemuDriver = {
> .no = VIR_DRV_QEMU,
> .name = "QEMU",
> @@ -12198,6 +12251,7 @@ static virDriver qemuDriver = {
> .domainGetDiskErrors = qemuDomainGetDiskErrors, /* 0.9.10 */
> .domainSetMetadata = qemuDomainSetMetadata, /* 0.9.10 */
> .domainGetMetadata = qemuDomainGetMetadata, /* 0.9.10 */
> + .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.10 */
Minor, but I'd sort this next to .domainPMSuspendForDuration.
> };
>
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 7872e3d..77ae9c1 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2785,3 +2785,23 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
>
> return ret;
> }
> +
> +int qemuMonitorSystemWakeup(qemuMonitorPtr mon)
> +{
> + int ret = -1;
> +
> + VIR_DEBUG("mon=%p", mon);
> +
> + if (!mon) {
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
> +
> + if (mon->json)
> + ret = qemuMonitorJSONSystemWakeup(mon);
> + else
> + ret = qemuMonitorTextSystemWakeup(mon);
Error out if not json.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list