[libvirt] [PATCH] qemu: snapshot: post error when create internal system checkpoint snapshot with rbd backend

When create internel system checkpoint snapshot with source file based on rbd backend, libvirt miss to check if the format of source file support internal snapshot. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179533 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26fc6a2..6dd0a5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13423,8 +13423,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup; if (dom_disk->src->type == VIR_STORAGE_TYPE_NETWORK && - (dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG || - dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG) { break; } if (vm->def->disks[i]->src->format > 0 && -- 2.1.0

On 11.02.2015 16:57, Shanzhi Yu wrote:
When create internel system checkpoint snapshot with source file based on rbd backend, libvirt miss to check if the format of source file support internal snapshot.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179533 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26fc6a2..6dd0a5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13423,8 +13423,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup;
if (dom_disk->src->type == VIR_STORAGE_TYPE_NETWORK && - (dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG || - dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG) { break; } if (vm->def->disks[i]->src->format > 0 &&
This check was introduced in b57e0153. I wonder what has changed since than. I'm not that familiar with snapshots nor ceph. But the whole if() block seems redundant to me. I mean, whether we know how to create a snapshot for given disk is checked a few lines above in qemuDomainSnapshotPrepareDiskInternal(). Therefore I think it can be removed whole, the if() block. Michal

On 02/19/2015 10:35 PM, Michal Privoznik wrote:
On 11.02.2015 16:57, Shanzhi Yu wrote:
When create internel system checkpoint snapshot with source file based on rbd backend, libvirt miss to check if the format of source file support internal snapshot.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179533 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26fc6a2..6dd0a5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13423,8 +13423,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, goto cleanup;
if (dom_disk->src->type == VIR_STORAGE_TYPE_NETWORK && - (dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG || - dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { + dom_disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_SHEEPDOG) { break; } if (vm->def->disks[i]->src->format > 0 &&
This check was introduced in b57e0153. I wonder what has changed since than. I'm not that familiar with snapshots nor ceph. But the whole if() block seems redundant to me. I mean, whether we know how to create a snapshot for given disk is checked a few lines above in qemuDomainSnapshotPrepareDiskInternal(). Therefore I think it can be removed whole, the if() block.
Thanks for review, will try to a v2
Michal
-- Regards shyu
participants (2)
-
Michal Privoznik
-
Shanzhi Yu