[libvirt] [PATCH] libxl: support domainReset
Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET option, but domainReset can be implemented in the libxl driver by forcibly destroying the domain and starting it again. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 67fd7bc6..08018d4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -979,6 +979,62 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } static int +libxlDomainReset(virDomainPtr dom, unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + libxlDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = libxlDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainResetEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + + /* + * The semantics of reset can be achieved by forcibly destroying + * the domain and starting it again. + */ + priv = vm->privateData; + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reset domain '%d'"), vm->def->id); + goto cleanup; + } + + if (!libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to cleanup domain '%d' after reset"), + vm->def->id); + vm = NULL; + goto cleanup; + } + + if (libxlDomainStart(driver, vm, false, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to start domain '%d' after reset"), + vm->def->id); + goto cleanup; + } + + ret = 0; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { @@ -4758,6 +4814,7 @@ static virDriver libxlDriver = { .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ + .domainReset = libxlDomainReset, /* 1.2.8 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */ -- 1.8.4.5
On Mon, 2014-08-04 at 20:09 -0600, Jim Fehlig wrote:
Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET option,
There's a case in the switch within libxl_send_trigger which at least purports to do so: int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid, libxl_trigger trigger, uint32_t vcpuid) { [...] case LIBXL_TRIGGER_RESET: rc = xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcpu break; Do you mean to say that it is broken or perhaps that it doesn't match the required semantics of domainReset? Ian.
On Tue, Aug 05, Ian Campbell wrote:
case LIBXL_TRIGGER_RESET: rc = xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcpu break;
Do you mean to say that it is broken or perhaps that it doesn't match the required semantics of domainReset?
This op is not implemented. And would it work for PV and HVM anyway? Olaf
On Tue, 2014-08-05 at 10:41 +0200, Olaf Hering wrote:
On Tue, Aug 05, Ian Campbell wrote:
case LIBXL_TRIGGER_RESET: rc = xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcpu break;
Do you mean to say that it is broken or perhaps that it doesn't match the required semantics of domainReset?
This op is not implemented.
You mean in libxc/Xen?
And would it work for PV and HVM anyway?
Aren't these triggers HVM only? They turn into ACPI events and things, don't they? I'm not sure what the intended behaviour of libvirt domainReset is, but perhaps libxl_domain_{reboot,shutdown} is closer to what is needed? Ian.
On Tue, Aug 05, Ian Campbell wrote:
On Tue, 2014-08-05 at 10:41 +0200, Olaf Hering wrote:
On Tue, Aug 05, Ian Campbell wrote:
case LIBXL_TRIGGER_RESET: rc = xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcpu break;
Do you mean to say that it is broken or perhaps that it doesn't match the required semantics of domainReset?
This op is not implemented.
You mean in libxc/Xen?
In Xen itself.
And would it work for PV and HVM anyway?
Aren't these triggers HVM only? They turn into ACPI events and things, don't they?
Most likely yes.
I'm not sure what the intended behaviour of libvirt domainReset is, but perhaps libxl_domain_{reboot,shutdown} is closer to what is needed?
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others. Olaf
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
On Tue, Aug 05, Ian Campbell wrote:
On Tue, 2014-08-05 at 10:41 +0200, Olaf Hering wrote:
On Tue, Aug 05, Ian Campbell wrote:
case LIBXL_TRIGGER_RESET: rc = xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcpu break;
Do you mean to say that it is broken or perhaps that it doesn't match the required semantics of domainReset?
This op is not implemented.
You mean in libxc/Xen?
In Xen itself.
And would it work for PV and HVM anyway?
Aren't these triggers HVM only? They turn into ACPI events and things, don't they?
Most likely yes.
I'm not sure what the intended behaviour of libvirt domainReset is, but perhaps libxl_domain_{reboot,shutdown} is closer to what is needed?
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger. Ian.
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset http://libvirt.org/html/libvirt-libvirt.html#virDomainReset destroy/start seems the correct way to implement this. Regards, Jim
On Tue, 2014-08-05 at 08:06 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset
http://libvirt.org/html/libvirt-libvirt.html#virDomainReset
destroy/start seems the correct way to implement this.
Yes, given that requirement it is. Sorry for the noise. Would some sort of hard reset API be useful in libxl? Ian.
Ian Campbell wrote:
On Tue, 2014-08-05 at 08:06 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset
http://libvirt.org/html/libvirt-libvirt.html#virDomainReset
destroy/start seems the correct way to implement this.
Yes, given that requirement it is. Sorry for the noise.
Would some sort of hard reset API be useful in libxl?
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided. In the absence of libxl_domain_reset(), do folks think the destroy/start approach is acceptable? As Olaf mentioned, it allows "Force Reset" to work via virt-manager. Regards, Jim
On Tue, 2014-08-05 at 09:10 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 08:06 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset
http://libvirt.org/html/libvirt-libvirt.html#virDomainReset
destroy/start seems the correct way to implement this.
Yes, given that requirement it is. Sorry for the noise.
Would some sort of hard reset API be useful in libxl?
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided.
I think this ought to become pretty easy once Wei's patches to record the guest cfg in libxl are completed. Wei -- what do you think?
In the absence of libxl_domain_reset(), do folks think the destroy/start approach is acceptable?
FWIW I believe it is.
As Olaf mentioned, it allows "Force Reset" to work via virt-manager.
Ian.
On Tue, Aug 05, 2014 at 04:30:47PM +0100, Ian Campbell wrote:
On Tue, 2014-08-05 at 09:10 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 08:06 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
The original report was that 'Reset' does not work from GUI, like virt-manager or virsh. I think the expected outcome is like pushing the reset button on a physical board. Xen doesnt do it that way, no idea about others.
Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset
http://libvirt.org/html/libvirt-libvirt.html#virDomainReset
destroy/start seems the correct way to implement this.
Yes, given that requirement it is. Sorry for the noise.
Would some sort of hard reset API be useful in libxl?
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided.
I think this ought to become pretty easy once Wei's patches to record the guest cfg in libxl are completed. Wei -- what do you think?
I don't think this "reset" API will need to record any state, i.e. this feature looks unrelated to my work. What do I miss? Wei.
On Tue, 2014-08-05 at 17:12 +0100, Wei Liu wrote:
On Tue, Aug 05, 2014 at 04:30:47PM +0100, Ian Campbell wrote:
On Tue, 2014-08-05 at 09:10 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 08:06 -0600, Jim Fehlig wrote:
Ian Campbell wrote:
On Tue, 2014-08-05 at 10:55 +0200, Olaf Hering wrote:
> The original report was that 'Reset' does not work from GUI, like > virt-manager or virsh. I think the expected outcome is like pushing the > reset button on a physical board. Xen doesnt do it that way, no idea > about others. > > Sounds like you want libxl_domain_reboot then, perhaps with a fallback on ERROR_NOPARAVIRT for an HVM guest to sending a trigger.
Hrm, I don't think that's right . It should be a hard reset
http://libvirt.org/html/libvirt-libvirt.html#virDomainReset
destroy/start seems the correct way to implement this.
Yes, given that requirement it is. Sorry for the noise.
Would some sort of hard reset API be useful in libxl?
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided.
I think this ought to become pretty easy once Wei's patches to record the guest cfg in libxl are completed. Wei -- what do you think?
I don't think this "reset" API will need to record any state, i.e. this feature looks unrelated to my work. What do I miss?
It's a forced reboot, so the API would need to destroy and then recreate the domain. Recreate would need to use the state your patches arrange for libxl to store. Ian.
On Tue, Aug 05, 2014 at 07:45:48PM +0100, Ian Campbell wrote: [...]
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided.
I think this ought to become pretty easy once Wei's patches to record the guest cfg in libxl are completed. Wei -- what do you think?
I don't think this "reset" API will need to record any state, i.e. this feature looks unrelated to my work. What do I miss?
It's a forced reboot, so the API would need to destroy and then recreate the domain. Recreate would need to use the state your patches arrange for libxl to store.
Oh you were talking about pesisting state across hard reset, that's of course achievable. I think hard reset is more or less the same as reboot. That's still somewhat orthogonal to my work though. Not having the capability to presist state across in libxl doesn't prevent us from introducing "reset". I think this is the status quo of "reboot" API, isn't it? Wei.
Ian.
On Tue, 2014-08-05 at 20:30 +0100, Wei Liu wrote:
On Tue, Aug 05, 2014 at 07:45:48PM +0100, Ian Campbell wrote: [...]
Sure. I think having an API that emulates a power reset button would be a nice addition to libxl's domain operations. The destroy/start approach incurs a small bit of overhead, which would be avoided with such an API. Clients (perhaps incorrectly) implementing their own notion of reset would also be avoided.
I think this ought to become pretty easy once Wei's patches to record the guest cfg in libxl are completed. Wei -- what do you think?
I don't think this "reset" API will need to record any state, i.e. this feature looks unrelated to my work. What do I miss?
It's a forced reboot, so the API would need to destroy and then recreate the domain. Recreate would need to use the state your patches arrange for libxl to store.
Oh you were talking about pesisting state across hard reset, that's of course achievable. I think hard reset is more or less the same as reboot.
That's still somewhat orthogonal to my work though. Not having the capability to presist state across in libxl doesn't prevent us from introducing "reset". I think this is the status quo of "reboot" API, isn't it?
There is no "reboot" API in the sense we are talking about. libxl_domain_reboot() asks the guest to reboot itself. The resulting actual reboot is handled by the toolstack receiving LIBXL_EVENT_TYPE_DOMAIN_DEATH and using libxl_domain_destroy +libxl_domain_create to recreate the domain, prior to your changes only the toolstack app could do this because only xl/libvirt knew the actual domain cfg. With your changes a new libxl_domain_hard_reboot could, I think, be written which does the reboot. Ian.
Jim Fehlig wrote:
Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET option, but domainReset can be implemented in the libxl driver by forcibly destroying the domain and starting it again.
Any other comments on this from a libvirt perspective? Regards, Jim
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 67fd7bc6..08018d4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -979,6 +979,62 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) }
static int +libxlDomainReset(virDomainPtr dom, unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + libxlDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = libxlDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainResetEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + + /* + * The semantics of reset can be achieved by forcibly destroying + * the domain and starting it again. + */ + priv = vm->privateData; + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reset domain '%d'"), vm->def->id); + goto cleanup; + } + + if (!libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to cleanup domain '%d' after reset"), + vm->def->id); + vm = NULL; + goto cleanup; + } + + if (libxlDomainStart(driver, vm, false, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to start domain '%d' after reset"), + vm->def->id); + goto cleanup; + } + + ret = 0; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { @@ -4758,6 +4814,7 @@ static virDriver libxlDriver = { .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ + .domainReset = libxlDomainReset, /* 1.2.8 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
Jim Fehlig wrote:
Currently, libxl_send_trigger() does not implement the LIBXL_TRIGGER_RESET option, but domainReset can be implemented in the libxl driver by forcibly destroying the domain and starting it again.
Any objections to pushing this patch? I have several patches on the list getting stale... Regards, Jim
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 67fd7bc6..08018d4 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -979,6 +979,62 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) }
static int +libxlDomainReset(virDomainPtr dom, unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + libxlDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = libxlDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainResetEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + + /* + * The semantics of reset can be achieved by forcibly destroying + * the domain and starting it again. + */ + priv = vm->privateData; + if (libxl_domain_destroy(priv->ctx, vm->def->id, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reset domain '%d'"), vm->def->id); + goto cleanup; + } + + if (!libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to cleanup domain '%d' after reset"), + vm->def->id); + vm = NULL; + goto cleanup; + } + + if (libxlDomainStart(driver, vm, false, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to start domain '%d' after reset"), + vm->def->id); + goto cleanup; + } + + ret = 0; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { @@ -4758,6 +4814,7 @@ static virDriver libxlDriver = { .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ + .domainReset = libxlDomainReset, /* 1.2.8 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */
participants (5)
-
Ian Campbell -
Ian Campbell
-
Jim Fehlig -
Olaf Hering -
Wei Liu