[libvirt] [PATCH 0/4] Sanitize restrictions imposed by VIR_DOMAIN_START_AUTODESTROY
Migration to a different host is the only reasonable restriction that should apply. Fix the wording in the docs and remove the restrictions of saving the VM, taking a snapshot and taking a checkpoint. Peter Krempa (4): lib: Lessen restrictions on VIR_DOMAIN_START_AUTODESTROY qemu: migration: Forbid only remote migration if autodestroy is active for VM qemu: snapshot: Don't forbid snapshot if autodestroy is registered qemu: checkpoint: Don't forbid checkpoint when VM is marked for autodestroy src/libvirt-domain.c | 9 ++++++--- src/qemu/qemu_driver.c | 12 ------------ src/qemu/qemu_migration.c | 3 ++- 3 files changed, 8 insertions(+), 16 deletions(-) -- 2.21.0
Apart from migrating the VM to a remote host where we can't honour the VIR_DOMAIN_START_AUTODESTROY flag properly restricting APIs which just modify the state of the VM does not make much sense. Change the wording of the documentation for VIR_DOMAIN_START_AUTODESTROY so that snapshots and saving to a file may be permitted as they semantically don't clash with the flag itself. Otherwise we'd have to forbid other APIs, such as virDomainDestroy as well. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt-domain.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index bbd2dc2e6e..e200dcc7d0 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -152,7 +152,8 @@ virDomainGetConnect(virDomainPtr dom) * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block save-to-file, + * or snapshots. * * virDomainFree should be used to free the resources after the * domain object is no longer needed. @@ -217,7 +218,8 @@ virDomainCreateXML(virConnectPtr conn, const char *xmlDesc, * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block + * save-to-file, or snapshots. * * virDomainFree should be used to free the resources after the * domain object is no longer needed. @@ -6565,7 +6567,8 @@ virDomainCreate(virDomainPtr domain) * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block save-to-file, + * or snapshots. * * If the VIR_DOMAIN_START_BYPASS_CACHE flag is set, and there is a * managed save file for this domain (created by virDomainManagedSave()), -- 2.21.0
On 9/24/19 8:17 AM, Peter Krempa wrote:
Apart from migrating the VM to a remote host where we can't honour the VIR_DOMAIN_START_AUTODESTROY flag properly restricting APIs which just
s/properly/properly,/
modify the state of the VM does not make much sense.
Change the wording of the documentation for VIR_DOMAIN_START_AUTODESTROY so that snapshots and saving to a file may be permitted as they semantically don't clash with the flag itself. Otherwise we'd have to forbid other APIs, such as virDomainDestroy as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt-domain.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index bbd2dc2e6e..e200dcc7d0 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -152,7 +152,8 @@ virDomainGetConnect(virDomainPtr dom) * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block save-to-file, + * or snapshots. * * virDomainFree should be used to free the resources after the * domain object is no longer needed. @@ -217,7 +218,8 @@ virDomainCreateXML(virConnectPtr conn, const char *xmlDesc, * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block + * save-to-file, or snapshots. * * virDomainFree should be used to free the resources after the * domain object is no longer needed. @@ -6565,7 +6567,8 @@ virDomainCreate(virDomainPtr domain) * object is finally released. This will also happen if the * client application crashes / loses its connection to the * libvirtd daemon. Any domains marked for auto destroy will - * block attempts at migration, save-to-file, or snapshots. + * block attempts at migration. Hypervisors may also block save-to-file, + * or snapshots. * * If the VIR_DOMAIN_START_BYPASS_CACHE flag is set, and there is a * managed save file for this domain (created by virDomainManagedSave()),
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Semantically we can't guarantee that we'll be able to destroy the VM on the remote host, thus we can't allow remote migration. All other forms of migration (e.g. saving to file) are okay though as they don't clash with semantics of the flag. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3c45ba35e6..a98ec2d55a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1155,7 +1155,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver, /* following checks don't make sense for offline migration */ if (!(flags & VIR_MIGRATE_OFFLINE)) { - if (qemuProcessAutoDestroyActive(driver, vm)) { + if (remote && + qemuProcessAutoDestroyActive(driver, vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is marked for auto destroy")); return false; -- 2.21.0
On 9/24/19 8:17 AM, Peter Krempa wrote:
Semantically we can't guarantee that we'll be able to destroy the VM on the remote host, thus we can't allow remote migration. All other forms of migration (e.g. saving to file) are okay though as they don't clash with semantics of the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3c45ba35e6..a98ec2d55a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1155,7 +1155,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
/* following checks don't make sense for offline migration */ if (!(flags & VIR_MIGRATE_OFFLINE)) { - if (qemuProcessAutoDestroyActive(driver, vm)) { + if (remote && + qemuProcessAutoDestroyActive(driver, vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is marked for auto destroy")); return false;
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Semantically VIR_DOMAIN_START_AUTODESTROY doesn't really clash with snapshot operations as the VM stays on the same host and thus bound to the same connection. Saving the state also doesn't differ from modifying the state of the VM which is allowed. Remove the check as it doesn't make much sense. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7f059b6d6..823aa558ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15917,12 +15917,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (qemuProcessAutoDestroyActive(driver, vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto cleanup; - } - if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot halt after transient domain snapshot")); -- 2.21.0
On 9/24/19 8:17 AM, Peter Krempa wrote:
Semantically VIR_DOMAIN_START_AUTODESTROY doesn't really clash with snapshot operations as the VM stays on the same host and thus bound to the same connection. Saving the state also doesn't differ from modifying the state of the VM which is allowed.
Remove the check as it doesn't make much sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7f059b6d6..823aa558ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15917,12 +15917,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup;
- if (qemuProcessAutoDestroyActive(driver, vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto cleanup; - } - if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot halt after transient domain snapshot"));
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The check was copied from the snapshot code and makes even less sense here. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 823aa558ac..db4b39dc9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17314,12 +17314,6 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (qemuProcessAutoDestroyActive(driver, vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto cleanup; - } - if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot create checkpoint for inactive domain")); -- 2.21.0
On 9/24/19 8:17 AM, Peter Krempa wrote:
The check was copied from the snapshot code and makes even less sense here.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 823aa558ac..db4b39dc9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17314,12 +17314,6 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup;
- if (qemuProcessAutoDestroyActive(driver, vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is marked for auto destroy")); - goto cleanup; - } - if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot create checkpoint for inactive domain"));
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa