2:30 a.m.
The managed state file is not useful anymore after the domain is
undefined, and perhaps cause confusion. E.g. define & start a domain
which has same name with previous undefined domain later.
---
src/qemu/qemu_driver.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f6e503a..874b455 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3739,6 +3739,7 @@ static int qemudDomainUndefine(virDomainPtr dom) {
virDomainObjPtr vm;
virDomainEventPtr event = NULL;
int ret = -1;
+ char *managed_save = NULL;
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -3766,6 +3767,13 @@ static int qemudDomainUndefine(virDomainPtr dom) {
if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0)
goto cleanup;
+ if (!(managed_save = qemuDomainManagedSavePath(driver, vm)))
+ goto cleanup;
+
+ if (virFileExists(managed_save) && (unlink(managed_save) < 0)) {
+ VIR_WARN("Failed to remove the managed state %s", managed_save);
+ }
+
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_UNDEFINED,
VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
--
1.7.4
8:17 a.m.
New subject: [libvirt] [PATCH] qemu: Unlink managed state file when a domain is undefined
On 04/20/2011 01:30 AM, Osier Yang wrote:
The managed state file is not useful anymore after the domain is
undefined, and perhaps cause confusion. E.g. define & start a domain
which has same name with previous undefined domain later.
---
src/qemu/qemu_driver.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f6e503a..874b455 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3739,6 +3739,7 @@ static int qemudDomainUndefine(virDomainPtr dom) {
virDomainObjPtr vm;
virDomainEventPtr event = NULL;
int ret = -1;
+ char *managed_save = NULL;
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -3766,6 +3767,13 @@ static int qemudDomainUndefine(virDomainPtr dom) {
if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) <
0)
goto cleanup;
+ if (!(managed_save = qemuDomainManagedSavePath(driver, vm)))
+ goto cleanup;
+
+ if (virFileExists(managed_save) && (unlink(managed_save) < 0)) {
+ VIR_WARN("Failed to remove the managed state %s", managed_save);
+ }
Hmm, I'm wondering if we should instead swap this to occur before
virDomainDeleteConfig, and fail the undefine operation if unlink() fails
(doing it after the deletion and only doing a VIR_WARN while still
succeeding seems rather weak).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4:41 a.m.
New subject: [libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
The managed state file is not useful anymore after the domain is
undefined, and perhaps cause confusion. E.g. define & start a domain
which has same name but different UUID with previous undefined
domain later.
v1 - v2:
* Try to delete the managed state file before delete domain
config file, and goto fail if it failed to delete it.
---
src/qemu/qemu_driver.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 771678e..4c5edd4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3738,6 +3738,7 @@ static int qemudDomainUndefine(virDomainPtr dom) {
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
virDomainEventPtr event = NULL;
+ char *managed_save = NULL;
int ret = -1;
qemuDriverLock(driver);
@@ -3763,6 +3764,15 @@ static int qemudDomainUndefine(virDomainPtr dom) {
goto cleanup;
}
+ if (!(managed_save = qemuDomainManagedSavePath(driver, vm)))
+ goto cleanup;
+
+ if (virFileExists(managed_save) && (unlink(managed_save) < 0)) {
+ virReportSystemError(errno, "%s",
+ _("Failed to delete managed state file"));
+ goto cleanup;
+ }
+
if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0)
goto cleanup;
--
1.7.4
9:29 a.m.
New subject: [libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
On 04/25/2011 03:41 AM, Osier Yang wrote:
The managed state file is not useful anymore after the domain is
undefined, and perhaps cause confusion. E.g. define & start a domain
which has same name but different UUID with previous undefined
domain later.
v1 - v2:
* Try to delete the managed state file before delete domain
config file, and goto fail if it failed to delete it.
---
src/qemu/qemu_driver.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
Ouch. virDomainUndefine doesn't have a flags argument. But this is
changing behavior in a user-visible manner (arguably for the better, but
any change is risky)
We have two options:
1. Proceed with the change: virDomainUndefine removes all associated
snapshot data; but this is a silent change in behavior
2. Add a new API: virDomainUndefineFlags; flag 0 leaves snapshot data
around (but warns), flag VIR_DOMAIN_UNDEFINE_ALL_ASSOCIATED_DATA (or
some better name) guarantees that snapshots are also removed
Being an API question, I think danpb might need to weigh in before we
make a change.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:45 a.m.
New subject: [libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
On 04/25/2011 08:29 AM, Eric Blake wrote:
On 04/25/2011 03:41 AM, Osier Yang wrote:
> The managed state file is not useful anymore after the domain is
> undefined, and perhaps cause confusion. E.g. define & start a domain
> which has same name but different UUID with previous undefined
> domain later.
>
> v1 - v2:
> * Try to delete the managed state file before delete domain
> config file, and goto fail if it failed to delete it.
> ---
> src/qemu/qemu_driver.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
Ouch. virDomainUndefine doesn't have a flags argument. But this is
changing behavior in a user-visible manner (arguably for the better, but
any change is risky)
We have two options:
1. Proceed with the change: virDomainUndefine removes all associated
snapshot data; but this is a silent change in behavior
2. Add a new API: virDomainUndefineFlags; flag 0 leaves snapshot data
around (but warns), flag VIR_DOMAIN_UNDEFINE_ALL_ASSOCIATED_DATA (or
some better name) guarantees that snapshots are also removed
Option 3: no silent deletion on undefine, and no new API, but make
virDomainUndefine fail if the managed state file exists. The user is
then responsible for using virDomainHasManagedSaveImage and
virDomainManagedSaveRemove prior to undefining a domain. But whereas
with option 1, people might complain that we removed the managed state
file, now with option 3 people might complain that we prevent the
undefine of a domain. Which leads to:
Option 4: Add virDomainUndefineFlags(). virDomainUndefineFlags(,0)
refuses to undefine a domain if state exists, while
virDomainUndefineFlags(,VIR_DOMAIN_UNDEFINE_FORCE) proceeds to undefine
the domain even if a managed state file exists. We'd still have to
decide whether virDomainUndefine maps to Flags(,0) or Flags(,FORCE), but
at least the default option is safer than in option 2 where you have to
use a flag to get the safe behavior.
At any rate, at the virsh level we can add a flag that maps into two API
calls as necessary to use virDomainManagedSaveRemove prior to
virDomainUndefine, regardless of what we do at the API level.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4960
days inactive
4965
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Osier Yang