Utilize the existing fake reboot mechanism to do reboot for TDX guest.
Different from normal guest, TDX guest doesn't support system_reset,
so have to kill the old guest and start a new one to simulate the reboot.
Co-developed-by: Chenyi Qiang <chenyi.qiang@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
src/qemu/qemu_process.c | 74 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bd8624e3f6..35758d882f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -441,6 +441,75 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED,
}
+static void
+qemuProcessSecFakeReboot(void *opaque)
+{
+ virDomainObj *vm = opaque;
+ qemuDomainObjPrivate *priv = vm->privateData;
+ virQEMUDriver *driver = priv->driver;
+ unsigned int stopFlags = 0;
+ virObjectEvent *event = NULL;
+ virObjectEvent * event2 = NULL;
+
+ VIR_DEBUG("Handle secure guest reboot: destroy phase");
+
+ virObjectLock(vm);
+ if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, 0) < 0)
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0) {
+ virDomainObjEndJob(vm);
+ goto cleanup;
+ }
+
+ if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN)
+ stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
+
+ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED,
+ VIR_ASYNC_JOB_NONE, stopFlags);
+ virDomainAuditStop(vm, "destroyed");
+
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
+
+ virObjectEventStateQueue(driver->domainEventState, event);
+ /* skip remove inactive domain from active list */
+ virDomainObjEndJob(vm);
+
+ VIR_DEBUG("Handle secure guest reboot: boot phase");
+
+ if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, 0) < 0) {
+ qemuDomainRemoveInactive(driver, vm, 0, false);
+ goto cleanup;
+ }
+
+ if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_START,
+ NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ 0) < 0) {
+ virDomainAuditStart(vm, "booted", false);
+ qemuDomainRemoveInactive(driver, vm, 0, false);
+ goto endjob;
+ }
+
+ virDomainAuditStart(vm, "booted", true);
+ event2 = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_STARTED_BOOTED);
+ virObjectEventStateQueue(driver->domainEventState, event2);
+
+ qemuDomainSaveStatus(vm);
+
+ endjob:
+ qemuProcessEndJob(vm);
+
+ cleanup:
+ qemuDomainSetFakeReboot(vm, false);
+ virDomainObjEndAPI(&vm);
+}
+
+
/*
* Since we have the '-no-shutdown' flag set, the
* QEMU process will currently have guest OS shutdown
@@ -459,6 +528,11 @@ qemuProcessFakeReboot(void *opaque)
int ret = -1, rc;
VIR_DEBUG("vm=%p", vm);
+
+ if (vm->def->sec &&
+ vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_TDX)
+ return qemuProcessSecFakeReboot(opaque);
+
virObjectLock(vm);
if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
goto cleanup;
--
2.34.1Hello Zhenzhong and Daniel, With this implementation, upon TD reboot, some events VIR_DOMAIN_EVENT_ID_LIFECYCLE are emitted (STARTED, STOPPED and probably SHUTDOWN and RESUMED). For normal VM, only the event VIR_DOMAIN_EVENT_ID_REBOOT is emitted. Do you think it is good to align the API for TD VM and normal VM ? So the reboot of a TD VM will be transparent to existing control plane software ? I would like to ask because we have a complaint about control plane software being broken because it only expects to receive the event VIR_DOMAIN_EVENT_ID_REBOOT. Best regards
On Mon, Oct 21, 2024 at 12:34:23PM -0000, hector.cao@canonical.com wrote: > Hello Zhenzhong and Daniel, > > With this implementation, upon TD reboot, some events VIR_DOMAIN_EVENT_ID_LIFECYCLE are emitted (STARTED, STOPPED and probably SHUTDOWN and RESUMED). > > For normal VM, only the event VIR_DOMAIN_EVENT_ID_REBOOT is emitted. > > Do you think it is good to align the API for TD VM and normal VM ? So the reboot of a TD VM will be transparent to existing control plane software ? > > I would like to ask because we have a complaint about control plane software being broken because it only expects to receive the event VIR_DOMAIN_EVENT_ID_REBOOT. I think the difference in events, while annoying, is the right thing to have, becasue the fact that QEMU is shutoff & re-spawned can have implications for integration into the system & thus should be reflected. To make it slightly nicer though, we should make sure that the lifecycle event "reason" detail, reports "REBOOTED" as the cause. That would let control plane software understand that these events are from a fake reboot. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
By "REBOOTED", do you mean VIR_DOMAIN_EVENT_STARTED_REBOOTED ? If yes, do you suggest adding this detail/reason to each lifecycle event caused by a reboot ? Then, we will also have : - VIR_DOMAIN_EVENT_SHUTDOWN_REBOOTED - VIR_DOMAIN_EVENT_STOPPED_REBOOTED - VIR_DOMAIN_EVENT_RESUMED_REBOOTED Best regards, On Mon, Oct 21, 2024 at 2:40 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Mon, Oct 21, 2024 at 12:34:23PM -0000, hector.cao@canonical.com wrote: > > Hello Zhenzhong and Daniel, > > > > With this implementation, upon TD reboot, some events > VIR_DOMAIN_EVENT_ID_LIFECYCLE are emitted (STARTED, STOPPED and probably > SHUTDOWN and RESUMED). > > > > For normal VM, only the event VIR_DOMAIN_EVENT_ID_REBOOT is emitted. > > > > Do you think it is good to align the API for TD VM and normal VM ? So > the reboot of a TD VM will be transparent to existing control plane > software ? > > > > I would like to ask because we have a complaint about control plane > software being broken because it only expects to receive the event > VIR_DOMAIN_EVENT_ID_REBOOT. > > I think the difference in events, while annoying, is the right thing > to have, becasue the fact that QEMU is shutoff & re-spawned can have > implications for integration into the system & thus should be reflected. > > To make it slightly nicer though, we should make sure that the lifecycle > event "reason" detail, reports "REBOOTED" as the cause. That would let > control plane software understand that these events are from a fake > reboot. > > With regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :| > > -- Hector CAO Software Engineer – Partner Engineering Team hector.cao@canonical.com https://launc <https://launchpad.net/~hectorcao>hpad.net/~hectorcao <https://launchpad.net/~hectorcao> <https://launchpad.net/~hectorcao>
On Mon, Oct 21, 2024 at 03:14:13PM +0200, Hector Cao wrote: > By "REBOOTED", do you mean VIR_DOMAIN_EVENT_STARTED_REBOOTED ? > > If yes, do you suggest adding this detail/reason to each lifecycle event > caused by a reboot ? > > Then, we will also have : > - VIR_DOMAIN_EVENT_SHUTDOWN_REBOOTED > - VIR_DOMAIN_EVENT_STOPPED_REBOOTED > - VIR_DOMAIN_EVENT_RESUMED_REBOOTED Yeah, that's what I meant. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, May 24, 2024 at 02:21:24PM +0800, Zhenzhong Duan wrote:
> Utilize the existing fake reboot mechanism to do reboot for TDX guest.
>
> Different from normal guest, TDX guest doesn't support system_reset,
> so have to kill the old guest and start a new one to simulate the reboot.
>
> Co-developed-by: Chenyi Qiang <chenyi.qiang@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> src/qemu/qemu_process.c | 74 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index bd8624e3f6..35758d882f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -441,6 +441,75 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED,
> }
>
>
> +static void
> +qemuProcessSecFakeReboot(void *opaque)
> +{
snip
> +}
> +
> +
> /*
> * Since we have the '-no-shutdown' flag set, the
> * QEMU process will currently have guest OS shutdown
> @@ -459,6 +528,11 @@ qemuProcessFakeReboot(void *opaque)
> int ret = -1, rc;
>
> VIR_DEBUG("vm=%p", vm);
> +
> + if (vm->def->sec &&
> + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_TDX)
> + return qemuProcessSecFakeReboot(opaque);
> +
> virObjectLock(vm);
> if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
> goto cleanup;
So the 'qemuProcessFakeReboot' currently fakes reboot via a
machine CPU reset. This new code fakes reboot via QEMU
re-creation.
I'd suggest that the current method gets renamed to
qemuProcessFakeRebootViaReset(), then your new
qemuProcessSecFakeReboot() gets renamed to
qemuProcessFakeRebootViaRecreate().
Then create a qemuProcessFakeReboot, that calls into either
qemuProcessFakeRebootViaReset or qemuProcessFakeRebootViaRecreate
as appropriate.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.