[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Support reboots with the QEMU driver



At 06/16/2011 01:06 AM, Daniel P. Berrange Write:
> For controlled shutdown we issue a 'system_powerdown' command
> to the QEMU monitor. This triggers an ACPI event which (most)
> guest OS wire up to a controlled shutdown. There is no equiv
> ACPI event to trigger a controlled reboot. This patch attempts
> to fake a reboot.
> 
>  - In qemuDomainObjPrivatePtr we have a bool fakeReboot
>    flag.
>  - The virDomainReboot method sets this flag and then
>    triggers a normal 'system_powerdown'.
>  - The QEMU process is started with '-no-shutdown'
>    so that the guest CPUs pause when it powers off the
>    guest
>  - When we receive the 'POWEROFF' event from QEMU JSON
>    monitor if fakeReboot is not set we invoke the
>    qemuProcessKill command and shutdown continues
>    normally
>  - If fakeReboot was set, we spawn a background thread
>    which issues 'system_reset' to perform a warm reboot
>    of the guest hardware. Then it issues 'cont' to
>    start the CPUs again
> 
> * src/qemu/qemu_command.c: Add -no-shutdown flag if
>   we have JSON support
> * src/qemu/qemu_domain.h: Add 'fakeReboot' flag to
>   qemuDomainObjPrivate struct
> * src/qemu/qemu_driver.c: Fake reboot using the
>   system_powerdown command if JSON support is available
> * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
>   src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
>   src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
>   binding for system_reset command
> * src/qemu/qemu_process.c: Reset the guest & start CPUs if
>   fakeReboot is set
> ---
>  src/qemu/qemu_command.c      |    7 +++
>  src/qemu/qemu_domain.h       |    2 +
>  src/qemu/qemu_driver.c       |   65 ++++++++++++++++++++++++++++-
>  src/qemu/qemu_monitor.c      |   19 ++++++++
>  src/qemu/qemu_monitor.h      |    1 +
>  src/qemu/qemu_monitor_json.c |   19 ++++++++
>  src/qemu/qemu_monitor_json.h |    1 +
>  src/qemu/qemu_monitor_text.c |   13 ++++++
>  src/qemu/qemu_monitor_text.h |    1 +
>  src/qemu/qemu_process.c      |   95 +++++++++++++++++++++++++++++++++++++++++-
>  10 files changed, 220 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6346243..9ae1409 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3207,6 +3207,13 @@ qemuBuildCommandLine(virConnectPtr conn,
>          def->onReboot != VIR_DOMAIN_LIFECYCLE_RESTART)
>          virCommandAddArg(cmd, "-no-reboot");
>  
> +    /* If JSON monitor is enabled, we can receive an event
> +     * when QEMU stops. If we use no-shutdown, then we can
> +     * watch for this event and do a soft/warm reboot.
> +     */
> +    if (monitor_json)
> +        virCommandAddArg(cmd, "-no-shutdown");
> +
>      if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
>          virCommandAddArg(cmd, "-no-acpi");
>  
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index bacf5b5..b8469b6 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -89,6 +89,8 @@ struct _qemuDomainObjPrivate {
>  
>      virBitmapPtr qemuCaps;
>      char *lockState;
> +
> +    bool fakeReboot;
>  };
>  
>  struct qemuDomainWatchdogEvent
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 853c84c..4c827cf 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1429,7 +1429,7 @@ cleanup:
>  }
>  
>  
> -static int qemudDomainShutdown(virDomainPtr dom) {
> +static int qemuDomainShutdown(virDomainPtr dom) {
>      struct qemud_driver *driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      int ret = -1;
> @@ -1461,6 +1461,8 @@ static int qemudDomainShutdown(virDomainPtr dom) {
>      ret = qemuMonitorSystemPowerdown(priv->mon);
>      qemuDomainObjExitMonitor(vm);
>  
> +    priv->fakeReboot = false;
> +
>  endjob:
>      if (qemuDomainObjEndJob(vm) == 0)
>          vm = NULL;
> @@ -1472,11 +1474,66 @@ cleanup:
>  }
>  
>  
> +static int qemuDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) {
> +    struct qemud_driver *driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +    qemuDomainObjPrivatePtr priv;
> +
> +    qemuDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +    qemuDriverUnlock(driver);
> +
> +    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;
> +    }
> +    priv = vm->privateData;
> +
> +#if HAVE_YAJL
> +    if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
> +        if (qemuDomainObjBeginJob(vm) < 0)
> +            goto cleanup;
> +
> +        if (!virDomainObjIsActive(vm)) {
> +            qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                            "%s", _("domain is not running"));
> +            goto endjob;
> +        }
> +
> +        qemuDomainObjEnterMonitor(vm);
> +        ret = qemuMonitorSystemPowerdown(priv->mon);
> +        qemuDomainObjExitMonitor(vm);
> +
> +        priv->fakeReboot = true;
> +
> +    endjob:
> +        if (qemuDomainObjEndJob(vm) == 0)
> +            vm = NULL;
> +    } else {
> +#endif
> +        qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                        _("Reboot is not supported without the JSON monitor"));
> +#if HAVE_YAJL
> +    }
> +#endif
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    return ret;
> +}
> +
> +
>  static int qemudDomainDestroy(virDomainPtr dom) {
>      struct qemud_driver *driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      int ret = -1;
>      virDomainEventPtr event = NULL;
> +    qemuDomainObjPrivatePtr priv;
>  
>      qemuDriverLock(driver);
>      vm  = virDomainFindByUUID(&driver->domains, dom->uuid);
> @@ -1488,6 +1545,9 @@ static int qemudDomainDestroy(virDomainPtr dom) {
>          goto cleanup;
>      }
>  
> +    priv = vm->privateData;
> +    priv->fakeReboot = false;
> +
>      /* Although qemuProcessStop does this already, there may
>       * be an outstanding job active. We want to make sure we
>       * can kill the process even if a job is active. Killing
> @@ -8146,7 +8206,8 @@ static virDriver qemuDriver = {
>      .domainLookupByName = qemudDomainLookupByName, /* 0.2.0 */
>      .domainSuspend = qemudDomainSuspend, /* 0.2.0 */
>      .domainResume = qemudDomainResume, /* 0.2.0 */
> -    .domainShutdown = qemudDomainShutdown, /* 0.2.0 */
> +    .domainShutdown = qemuDomainShutdown, /* 0.2.0 */
> +    .domainReboot = qemuDomainReboot, /* 0.9.3 */
>      .domainDestroy = qemudDomainDestroy, /* 0.2.0 */
>      .domainGetOSType = qemudDomainGetOSType, /* 0.2.2 */
>      .domainGetMaxMemory = qemudDomainGetMaxMemory, /* 0.4.2 */
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 1428921..1dafc4b 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1096,6 +1096,25 @@ int qemuMonitorSystemPowerdown(qemuMonitorPtr mon)
>  }
>  
>  
> +int qemuMonitorSystemReset(qemuMonitorPtr mon)
> +{
> +    int ret;
> +    VIR_DEBUG("mon=%p", mon);
> +
> +    if (!mon) {
> +        qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> +                        _("monitor must not be NULL"));
> +        return -1;
> +    }
> +
> +    if (mon->json)
> +        ret = qemuMonitorJSONSystemReset(mon);
> +    else
> +        ret = qemuMonitorTextSystemReset(mon);
> +    return ret;
> +}
> +
> +
>  int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
>                            int **pids)
>  {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 3bb0269..ae74a3c 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -194,6 +194,7 @@ int qemuMonitorStartCPUs(qemuMonitorPtr mon,
>  int qemuMonitorStopCPUs(qemuMonitorPtr mon);
>  int qemuMonitorGetStatus(qemuMonitorPtr mon, bool *running);
>  
> +int qemuMonitorSystemReset(qemuMonitorPtr mon);
>  int qemuMonitorSystemPowerdown(qemuMonitorPtr mon);
>  
>  int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 56ec65b..6cc6ea7 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -947,6 +947,25 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon)
>  }
>  
>  
> +int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
> +{
> +    int ret;
> +    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("system_reset", NULL);
> +    virJSONValuePtr reply = NULL;
> +    if (!cmd)
> +        return -1;
> +
> +    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> +    if (ret == 0)
> +        ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> +
> +
>  /*
>   * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 },
>   *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 393d8fc..a72bf7c 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -49,6 +49,7 @@ int qemuMonitorJSONStopCPUs(qemuMonitorPtr mon);
>  int qemuMonitorJSONGetStatus(qemuMonitorPtr mon, bool *running);
>  
>  int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon);
> +int qemuMonitorJSONSystemReset(qemuMonitorPtr mon);
>  
>  int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon,
>                                int **pids);
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index a16ea91..67333aa 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -417,6 +417,19 @@ int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon) {
>  }
>  
>  
> +int qemuMonitorTextSystemReset(qemuMonitorPtr mon) {
> +    char *info;
> +
> +    if (qemuMonitorHMPCommand(mon, "system_reset", &info) < 0) {
> +        qemuReportError(VIR_ERR_OPERATION_FAILED,
> +                        "%s", _("system reset operation failed"));
> +        return -1;
> +    }
> +    VIR_FREE(info);
> +    return 0;
> +}
> +
> +
>  int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
>                                int **pids)
>  {
> diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
> index 4fa5064..7536557 100644
> --- a/src/qemu/qemu_monitor_text.h
> +++ b/src/qemu/qemu_monitor_text.h
> @@ -46,6 +46,7 @@ int qemuMonitorTextStopCPUs(qemuMonitorPtr mon);
>  int qemuMonitorTextGetStatus(qemuMonitorPtr mon, bool *running);
>  
>  int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon);
> +int qemuMonitorTextSystemReset(qemuMonitorPtr mon);
>  
>  int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
>                                int **pids);
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 6838417..694d8a9 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -348,12 +348,99 @@ qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>  }
>  
>  
> +/*
> + * Since we have the '-no-shutdown' flag set, the
> + * QEMU process will currently have guest OS shutdown
> + * and the CPUS stopped. To fake the reboot, we thus
> + * want todo a reset of the virtual hardware, followed
> + * by restart of the CPUs. This should result in the
> + * guest OS booting up again
> + */
> +static void
> +qemuProcessFakeReboot(void *opaque)
> +{
> +    struct qemud_driver *driver = qemu_driver;
> +    virDomainObjPtr vm = opaque;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virDomainEventPtr event = NULL;
> +    int ret = -1;
> +    VIR_DEBUG("vm=%p", vm);
> +    qemuDriverLock(driver);
> +    virDomainObjLock(vm);
> +    if (qemuDomainObjBeginJob(vm) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("guest unexpectedly quit"));
> +        goto endjob;
> +    }
> +
> +    qemuDomainObjEnterMonitorWithDriver(driver, vm);
> +    if (qemuMonitorSystemReset(priv->mon) < 0) {
> +        qemuDomainObjExitMonitorWithDriver(driver, vm);
> +        goto endjob;
> +    }
> +    qemuDomainObjExitMonitorWithDriver(driver, vm);
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("guest unexpectedly quit"));
> +        goto endjob;
> +    }
> +
> +    if (qemuProcessStartCPUs(driver, vm, NULL,
> +                             VIR_DOMAIN_RUNNING_BOOTED) < 0) {
> +        if (virGetLastError() == NULL)
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            "%s", _("resume operation failed"));
> +        goto endjob;
> +    }
> +    event = virDomainEventNewFromObj(vm,
> +                                     VIR_DOMAIN_EVENT_RESUMED,
> +                                     VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
> +
> +    ret = 0;
> +
> +endjob:
> +    if (qemuDomainObjEndJob(vm) == 0)
> +        vm = NULL;
> +
> +cleanup:
> +    if (vm) {
> +        if (ret == -1)
> +            qemuProcessKill(vm);
> +        if (virDomainObjUnref(vm) > 0)
> +            virDomainObjUnlock(vm);
> +    }
> +    if (event)
> +        qemuDomainEventQueue(driver, event);
> +    qemuDriverUnlock(driver);
> +}
> +
> +
>  static int
>  qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>                            virDomainObjPtr vm)
>  {
> +    qemuDomainObjPrivatePtr priv = vm->privateData;

I think we should get vm->privateData after we lock vm.

> +    VIR_DEBUG("vm=%p", vm);
> +
>      virDomainObjLock(vm);
> -    ((qemuDomainObjPrivatePtr) vm->privateData)->gotShutdown = true;
> +    priv->gotShutdown = true;
> +    if (priv->fakeReboot) {
> +        virDomainObjRef(vm);
> +        virThread th;
> +        if (virThreadCreate(&th,
> +                            false,
> +                            qemuProcessFakeReboot,
> +                            vm) < 0) {
> +            VIR_ERROR("Failed to create reboot thread, killing domain");
> +            qemuProcessKill(vm);

We should hold an extra reference the vm here. If the vm is not persistent
and we destroy the vm before the thread runs, libvirtd may crash(I think, not test
it).

> +        }
> +    } else {
> +        qemuProcessKill(vm);
> +    }
>      virDomainObjUnlock(vm);
>  
>      return 0;
> @@ -1920,6 +2007,11 @@ qemuProcessPrepareMonitorChr(struct qemud_driver *driver,
>  }
>  
>  
> +/*
> + * Precondition: Both driver and vm must be locked,
> + * and a job must be active. This method will call
> + * {Enter,Exit}MonitorWithDriver
> + */
>  int
>  qemuProcessStartCPUs(struct qemud_driver *driver, virDomainObjPtr vm,
>                       virConnectPtr conn, virDomainRunningReason reason)
> @@ -2182,6 +2274,7 @@ int qemuProcessStart(virConnectPtr conn,
>          goto cleanup;
>  
>      vm->def->id = driver->nextvmid++;
> +    priv->fakeReboot = false;
>  
>      /* Run an early hook to set-up missing devices */
>      if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]