[libvirt] [PATCH] qemu: update disk chain in vm->newDef when block job compeleted
A bug will be hitted with libvirt+qemu+kvm when doing follow steps: 1.initial status :vm = base+z(z is a snapshot) 2. call snapshotCreateXML( ): vm = base+z +z' 3. call blockRebase( ): vm = base+z''(z rebase to z') 4.(after rebasing work done) vm shutdown and then start an error occured like this: "unable to set user and group to '0:0' on ' /path/to/disk': No such file or directory A second start may sucess. The error message reported by virSecurityDACSetSecurityDiskLabel when the disk chain is misused(that is base+z +z', after rebase, but z' was deleted yet).virSecurityDACSetSecurityDiskLabel used an old disk chain after rebasing. In qemuProcessHandleBlockJob when rebase complete, the disk chain in vm->def is updated but vm->newDef is not(when vm->newDef is not null). In shutdown( actrully in qemuProcessStop ), if vm->newDef is not null , vm->def will be replaced by vm->newDef, which contains a wrong disk chain!!! Then the next start will failed with above message. To fix it, update disk chain in vm->newDef in qemuProcessHandleBlockJob. Signed-off-by: Weiwei Li <nuonuoli@tencent.com> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 24e5f0f..0491249 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1033,6 +1033,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainDiskDefPtr persistDisk = NULL; + virDomainDiskDefPtr newDisk = NULL; bool save = false; virObjectLock(vm); @@ -1101,6 +1102,32 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true, true)); + /* Chang vm->newDef disk chain */ + if (!persistDisk && vm->newDef){ + int indx = virDomainDiskIndexByName(vm->newDef, + disk->dst, + false); + virStorageSourcePtr copy = NULL; + + if (indx >=0){ + newDisk = vm->newDef->disks[indx]; + copy = virStorageSourceCopy(disk->src, false); + if (virStorageSourceInitChainElement(copy, + newDisk->src, + false) < 0) { + VIR_WARN("Unable to update newDef definition " + "on vm %s after block job", + vm->def->name); + virStorageSourceFree(copy); + copy = NULL; + newDisk = NULL; + } + } + if (copy){ + virStorageSourceFree(newDisk->src); + newDisk->src = copy; + } + } } else if (disk->mirror && (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) { -- 1.7.1
On 11/10/14 05:15, weiwei li wrote:
A bug will be hitted with libvirt+qemu+kvm when doing follow steps: 1.initial status :vm = base+z(z is a snapshot) 2. call snapshotCreateXML( ): vm = base+z +z' 3. call blockRebase( ): vm = base+z''(z rebase to z') 4.(after rebasing work done) vm shutdown and then start
an error occured like this: "unable to set user and group to '0:0' on ' /path/to/disk': No such file or directory
A second start may sucess.
The error message reported by virSecurityDACSetSecurityDiskLabel when the disk chain is misused(that is base+z +z', after rebase, but z' was deleted yet).virSecurityDACSetSecurityDiskLabel used an old disk chain after rebasing. In qemuProcessHandleBlockJob when rebase complete, the disk chain in vm->def is updated but vm->newDef is not(when vm->newDef is not null). In shutdown( actrully in qemuProcessStop ), if vm->newDef is not null , vm->def will be replaced by vm->newDef, which contains a wrong disk chain!!! Then the next start will failed with above message. To fix it, update disk chain in vm->newDef in qemuProcessHandleBlockJob.
Signed-off-by: Weiwei Li <nuonuoli@tencent.com <mailto:nuonuoli@tencent.com>> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
Since commit: commit fe7ef7b112b3b4d6f9c9edf499a79683fb0b7edb Author: Peter Krempa <pkrempa@redhat.com> AuthorDate: Thu Sep 25 17:30:28 2014 +0200 Commit: Peter Krempa <pkrempa@redhat.com> CommitDate: Fri Sep 26 09:35:33 2014 +0200 qemu: Always re-detect backing chain Since 363e9a68 we track backing chain metadata when creating snapshots the right way even for the inactive configuration. As we did not yet update other code paths that modify the backing chain (blockpull) the newDef backing chain gets out of sync. After stopping of a VM the new definition gets copied to the next start one. The new VM then has incorrect backing chain info. This patch switches the backing chain detector to always purge the existing backing chain and forces re-detection to avoid this issue until we'll have full backing chain tracking support. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1144922 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 11145d1..76fccce 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2540,7 +2540,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virFileExists(virDomainDiskGetSource(disk))) continue; - if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) >= 0) + if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0) continue; if (disk->startupPolicy && thus your symptoms shouldn't show up. Could you please verify that the situation you've described happens
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 24e5f0f..0491249 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1033,6 +1033,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainDiskDefPtr persistDisk = NULL; + virDomainDiskDefPtr newDisk = NULL; bool save = false;
virObjectLock(vm); @@ -1101,6 +1102,32 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true, true)); + /* Chang vm->newDef disk chain */ + if (!persistDisk && vm->newDef){ + int indx = virDomainDiskIndexByName(vm->newDef, + disk->dst,
+ false);
+ virStorageSourcePtr copy = NULL; + + if (indx >=0){ + newDisk = vm->newDef->disks[indx]; + copy = virStorageSourceCopy(disk->src, false); + if (virStorageSourceInitChainElement(copy, + newDisk->src,
+ false) < 0) {
+ VIR_WARN("Unable to update newDef definition " + "on vm %s after block job", + vm->def->name); + virStorageSourceFree(copy); + copy = NULL; + newDisk = NULL; + } + } + if (copy){ + virStorageSourceFree(newDisk->src); + newDisk->src = copy; + } + } } else if (disk->mirror && (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) { -- 1.7.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 12/02/14 17:05, Peter Krempa wrote:
On 11/10/14 05:15, weiwei li wrote:
A bug will be hitted with libvirt+qemu+kvm when doing follow steps: 1.initial status :vm = base+z(z is a snapshot) 2. call snapshotCreateXML( ): vm = base+z +z' 3. call blockRebase( ): vm = base+z''(z rebase to z') 4.(after rebasing work done) vm shutdown and then start
an error occured like this: "unable to set user and group to '0:0' on ' /path/to/disk': No such file or directory
A second start may sucess.
The error message reported by virSecurityDACSetSecurityDiskLabel when the disk chain is misused(that is base+z +z', after rebase, but z' was deleted yet).virSecurityDACSetSecurityDiskLabel used an old disk chain after rebasing. In qemuProcessHandleBlockJob when rebase complete, the disk chain in vm->def is updated but vm->newDef is not(when vm->newDef is not null). In shutdown( actrully in qemuProcessStop ), if vm->newDef is not null , vm->def will be replaced by vm->newDef, which contains a wrong disk chain!!! Then the next start will failed with above message. To fix it, update disk chain in vm->newDef in qemuProcessHandleBlockJob.
Signed-off-by: Weiwei Li <nuonuoli@tencent.com <mailto:nuonuoli@tencent.com>> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
Since commit:
commit fe7ef7b112b3b4d6f9c9edf499a79683fb0b7edb Author: Peter Krempa <pkrempa@redhat.com> AuthorDate: Thu Sep 25 17:30:28 2014 +0200 Commit: Peter Krempa <pkrempa@redhat.com> CommitDate: Fri Sep 26 09:35:33 2014 +0200
qemu: Always re-detect backing chain
Since 363e9a68 we track backing chain metadata when creating snapshots the right way even for the inactive configuration. As we did not yet update other code paths that modify the backing chain (blockpull) the newDef backing chain gets out of sync.
After stopping of a VM the new definition gets copied to the next start one. The new VM then has incorrect backing chain info. This patch switches the backing chain detector to always purge the existing backing chain and forces re-detection to avoid this issue until we'll have full backing chain tracking support.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1144922
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 11145d1..76fccce 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2540,7 +2540,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virFileExists(virDomainDiskGetSource(disk))) continue;
- if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) >= 0) + if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0) continue;
if (disk->startupPolicy &&
thus your symptoms shouldn't show up. Could you please verify that the situation you've described happens
Oops I've hit send before finishing the message. I want to ask whether this still reproduces with current upstream as the commit mentioned above should avoid issue like you've described. Additionally the patch does not apply cleanly now, so please repost a rebased version. Thanks. Peter
participants (2)
-
Peter Krempa -
weiwei li