6:46 a.m.
Bobo Du (1):
qemu:Fix snapshot creating with vm xml persistent
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
--
1.8.3.1
6:46 a.m.
New subject: [libvirt] [PATCH] qemu:Fix snapshot creating with vm xml persistent
the vm xml will be existed when the vm is undefined after started.
blockcommit interface also has the bug with above.
Step1:prepare a vm，eg:test1,start it and undefined
Step2: virsh snapshot-create-as test1 --disk-only --no-metadata
Step3:ls /etc/libvirt/qemu/test1.xml,then it will be exist here
Signed-off-by: Bobo Du <dubobo(a)didichuxing.com
---
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb0d4a8..d977922 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7570,6 +7570,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
if (!virDomainObjIsActive(vm))
qemuDomainRemoveInactive(driver, vm);
+ virDomainDefFree(vm->newDef);
ret = 0;
endjob:
qemuDomainObjEndJob(driver, vm);
--
1.8.3.1
10:01 a.m.
New subject: [libvirt] [PATCH] qemu:Fix snapshot creating with vm xml persistent
On 08/07/2018 01:46 PM, Bobo Du wrote:
the vm xml will be existed when the vm is undefined after started.
blockcommit interface also has the bug with above.
Step1:prepare a vm，eg:test1,start it and undefined
Step2: virsh snapshot-create-as test1 --disk-only --no-metadata
Step3:ls /etc/libvirt/qemu/test1.xml,then it will be exist here
Signed-off-by: Bobo Du <dubobo(a)didichuxing.com
---
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb0d4a8..d977922 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7570,6 +7570,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
if (!virDomainObjIsActive(vm))
qemuDomainRemoveInactive(driver, vm);
+ virDomainDefFree(vm->newDef);
ret = 0;
endjob:
qemuDomainObjEndJob(driver, vm);
This doesn't feel right. Firstly, vm->newDef becomes a stale pointer
after this patch. But more importantly, if snapshot-create-as saves the
xml it is clearly not testing vm->persistent flag and the fix should
focus on that.
Michal
3:25 a.m.
New subject: [libvirt] [PATCH] qemu:Fix snapshot creating with vm xml persistent
On Thu, Aug 09, 2018 at 17:01:49 +0200, Michal Privoznik wrote:
On 08/07/2018 01:46 PM, Bobo Du wrote:
> the vm xml will be existed when the vm is undefined after started.
> blockcommit interface also has the bug with above.
> Step1:prepare a vm，eg:test1,start it and undefined
> Step2: virsh snapshot-create-as test1 --disk-only --no-metadata
> Step3:ls /etc/libvirt/qemu/test1.xml,then it will be exist here
>
> Signed-off-by: Bobo Du <dubobo(a)didichuxing.com
> ---
> src/qemu/qemu_driver.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index fb0d4a8..d977922 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7570,6 +7570,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
> if (!virDomainObjIsActive(vm))
> qemuDomainRemoveInactive(driver, vm);
>
> + virDomainDefFree(vm->newDef);
> ret = 0;
> endjob:
> qemuDomainObjEndJob(driver, vm);
>
This doesn't feel right. Firstly, vm->newDef becomes a stale pointer
This is obviously true ...
after this patch. But more importantly, if snapshot-create-as saves
the
xml it is clearly not testing vm->persistent flag and the fix should
focus on that.
That was the previous approach which is wrong in my opinion. If
vm->newDef exists we should modify it. The snapshot code should not care
if the VM is persistent or not. We need to modify the definition and
afterwards it's saved.
If the vm->newDef gets used in a different way we'd need to change a lot
of code just because of that.
The real bug is that for transient vms currently there should be no
vm->newDef as this is a fact in many places.
Also the commit message is obviously misleading as it's pointing out the
symptom but not the real bug.
If vm->newDef gets cleared and the commit message points out to the real
problem this should be the preferred solution.
2293
days inactive
2299
days old
3 comments
3 participants
participants (3)
-
Bobo Du -
Michal Privoznik -
Peter Krempa