[libvirt] [PATCH] conf: Assign newDef of active domain as persistent conf if it is NULL

Libvirt loads the domain conf from status XML if it's running when starting up. The problem is there is no record of the original conf. (dom->newDef is NULL here). So libvirt won't be able to restore the domain conf to original one when destroying/shutdown. E.g. 1) attach a device without "--persistent" 2) restart libvirtd 3) destroy domain 4) start domain One will see the the disk still exists. This patch is to fix the peoblem by assigning persistent domain conf to dom->newDef if it's NULL and the domain is running. --- src/conf/domain_conf.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 00212db..cbb99d3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10533,9 +10533,15 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, if ((dom = virDomainFindByUUID(doms, def->uuid))) { dom->autostart = autostart; + if (virDomainObjIsActive(dom) && + !dom->newDef) { + virDomainObjAssignDef(dom, def, false); + } else { + virDomainDefFree(def); + } + VIR_FREE(configFile); VIR_FREE(autostartLink); - virDomainDefFree(def); return dom; } -- 1.7.6

于 2011年09月01日 21:44, Osier Yang 写道:
Libvirt loads the domain conf from status XML if it's running when starting up. The problem is there is no record of the original conf. (dom->newDef is NULL here).
So libvirt won't be able to restore the domain conf to original one when destroying/shutdown. E.g.
1) attach a device without "--persistent" 2) restart libvirtd 3) destroy domain 4) start domain
One will see the the disk still exists.
This patch is to fix the peoblem by assigning persistent domain conf to dom->newDef if it's NULL and the domain is running. --- src/conf/domain_conf.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 00212db..cbb99d3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10533,9 +10533,15 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, if ((dom = virDomainFindByUUID(doms, def->uuid))) { dom->autostart = autostart;
+ if (virDomainObjIsActive(dom) && + !dom->newDef) { + virDomainObjAssignDef(dom, def, false); + } else { + virDomainDefFree(def); + } + VIR_FREE(configFile); VIR_FREE(autostartLink); - virDomainDefFree(def); return dom; }
With my testing, this patch solve the problem well. Could someone review/ACK this? Thanks Osier

On 09/01/2011 07:44 AM, Osier Yang wrote:
Libvirt loads the domain conf from status XML if it's running when starting up. The problem is there is no record of the original conf. (dom->newDef is NULL here).
So libvirt won't be able to restore the domain conf to original one when destroying/shutdown. E.g.
1) attach a device without "--persistent" 2) restart libvirtd 3) destroy domain 4) start domain
One will see the the disk still exists.
This patch is to fix the peoblem by assigning persistent domain conf to dom->newDef if it's NULL and the domain is running.
I was hoping danpb would weigh in on this one, but it looks right to me, definitely solves the test in your commit message, and is appropriate for 0.9.5. ACK.
+++ b/src/conf/domain_conf.c @@ -10533,9 +10533,15 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, if ((dom = virDomainFindByUUID(doms, def->uuid))) { dom->autostart = autostart;
+ if (virDomainObjIsActive(dom)&& + !dom->newDef) { + virDomainObjAssignDef(dom, def, false); + } else { + virDomainDefFree(def); + } + VIR_FREE(configFile); VIR_FREE(autostartLink); - virDomainDefFree(def); return dom; }
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

于 2011年09月20日 04:58, Eric Blake 写道:
On 09/01/2011 07:44 AM, Osier Yang wrote:
Libvirt loads the domain conf from status XML if it's running when starting up. The problem is there is no record of the original conf. (dom->newDef is NULL here).
So libvirt won't be able to restore the domain conf to original one when destroying/shutdown. E.g.
1) attach a device without "--persistent" 2) restart libvirtd 3) destroy domain 4) start domain
One will see the the disk still exists.
This patch is to fix the peoblem by assigning persistent domain conf to dom->newDef if it's NULL and the domain is running.
I was hoping danpb would weigh in on this one, but it looks right to me, definitely solves the test in your commit message, and is appropriate for 0.9.5.
ACK.
+++ b/src/conf/domain_conf.c @@ -10533,9 +10533,15 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, if ((dom = virDomainFindByUUID(doms, def->uuid))) { dom->autostart = autostart;
+ if (virDomainObjIsActive(dom)&& + !dom->newDef) { + virDomainObjAssignDef(dom, def, false); + } else { + virDomainDefFree(def); + } + VIR_FREE(configFile); VIR_FREE(autostartLink); - virDomainDefFree(def); return dom; }
Pushed. Osier
participants (2)
-
Eric Blake
-
Osier Yang