[libvirt] [PATCH] qemu: snapshot: Fix incorrect disk type for auto-generated disks
When changing the parsing and formatting functions in commit 43f2ccdc73090bd03f64de4d58d46ffa0134d705 I forgot to update the qemu disk alingnment function for snapshots that automatically adds snapshot configs for disks that were not mentioned in the XML. The function allocated a new disk snapshot definition but did not correctly initialize the snapshot disk source type variable. This resulted into the disks considered as block devices and invalid XML was generated. Reported by John Ferlan. --- Notes: Doing a unit test for this will require yet another addition to the testsuite so I'm sending this patch first. src/conf/snapshot_conf.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 5637975..55abd80 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -532,6 +532,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; disk->index = i; disk->snapshot = def->dom->disks[i]->snapshot; + disk->type = -1; if (!disk->snapshot) disk->snapshot = default_snapshot; } @@ -549,6 +550,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, const char *tmp; struct stat sb; + if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE || + disk->type != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate external snapshot name " + "for disk '%s' on a '%s' device"), + disk->name, virDomainDiskTypeToString(disk->type)); + goto cleanup; + } + if (!original) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name " -- 1.8.4.3
On 12/05/13 17:17, Peter Krempa wrote:
When changing the parsing and formatting functions in commit 43f2ccdc73090bd03f64de4d58d46ffa0134d705 I forgot to update the qemu disk alingnment function for snapshots that automatically adds snapshot configs for disks that were not mentioned in the XML. The function allocated a new disk snapshot definition but did not correctly initialize the snapshot disk source type variable. This resulted into the disks considered as block devices and invalid XML was generated.
Reported by John Ferlan. ---
Notes: Doing a unit test for this will require yet another addition to the testsuite so I'm sending this patch first.
src/conf/snapshot_conf.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 5637975..55abd80 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -532,6 +532,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; disk->index = i; disk->snapshot = def->dom->disks[i]->snapshot; + disk->type = -1; if (!disk->snapshot) disk->snapshot = default_snapshot; } @@ -549,6 +550,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, const char *tmp; struct stat sb;
+ if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE ||
I forgot to amend the patch before sending. Please consider && in place of || in the line above.
+ disk->type != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate external snapshot name " + "for disk '%s' on a '%s' device"), + disk->name, virDomainDiskTypeToString(disk->type)); + goto cleanup; + } + if (!original) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name "
Peter
On 12/05/2013 09:30 AM, Peter Krempa wrote:
On 12/05/13 17:17, Peter Krempa wrote:
When changing the parsing and formatting functions in commit 43f2ccdc73090bd03f64de4d58d46ffa0134d705 I forgot to update the qemu disk alingnment function for snapshots that automatically adds snapshot
s/alingnment/alignment/
configs for disks that were not mentioned in the XML. The function allocated a new disk snapshot definition but did not correctly initialize the snapshot disk source type variable. This resulted into the disks considered as block devices and invalid XML was generated.
+ if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE ||
I forgot to amend the patch before sending. Please consider && in place of || in the line above.
+ disk->type != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate external snapshot name " + "for disk '%s' on a '%s' device"),
ACK as amended. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/05/13 20:25, Eric Blake wrote:
On 12/05/2013 09:30 AM, Peter Krempa wrote:
On 12/05/13 17:17, Peter Krempa wrote:
When changing the parsing and formatting functions in commit 43f2ccdc73090bd03f64de4d58d46ffa0134d705 I forgot to update the qemu disk alingnment function for snapshots that automatically adds snapshot
s/alingnment/alignment/
configs for disks that were not mentioned in the XML. The function allocated a new disk snapshot definition but did not correctly initialize the snapshot disk source type variable. This resulted into the disks considered as block devices and invalid XML was generated.
+ if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE ||
I forgot to amend the patch before sending. Please consider && in place of || in the line above.
+ disk->type != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate external snapshot name " + "for disk '%s' on a '%s' device"),
ACK as amended.
I've fixed the commit message and pushed this patch. Thanks. Peter
On 12/05/2013 11:30 AM, Peter Krempa wrote:
On 12/05/13 17:17, Peter Krempa wrote:
When changing the parsing and formatting functions in commit 43f2ccdc73090bd03f64de4d58d46ffa0134d705 I forgot to update the qemu disk alingnment function for snapshots that automatically adds snapshot configs for disks that were not mentioned in the XML. The function allocated a new disk snapshot definition but did not correctly initialize the snapshot disk source type variable. This resulted into the disks considered as block devices and invalid XML was generated.
Reported by John Ferlan. ---
Notes: Doing a unit test for this will require yet another addition to the testsuite so I'm sending this patch first.
src/conf/snapshot_conf.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
ACK as amended - the virt-test's pass again too John
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 5637975..55abd80 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -532,6 +532,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; disk->index = i; disk->snapshot = def->dom->disks[i]->snapshot; + disk->type = -1; if (!disk->snapshot) disk->snapshot = default_snapshot; } @@ -549,6 +550,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, const char *tmp; struct stat sb;
+ if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE ||
I forgot to amend the patch before sending. Please consider && in place of || in the line above.
+ disk->type != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate external snapshot name " + "for disk '%s' on a '%s' device"), + disk->name, virDomainDiskTypeToString(disk->type)); + goto cleanup; + } + if (!original) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name "
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa