[libvirt] [PATCH] 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 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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Osier Yang