[libvirt] [PATCH v3] qemu-migration: Disallow migration of read only disk

Currently Libvirt allows attempts to migrate read only disks. Qemu cannot handle this as read only disks cannot be written to on the destination system. The end result is a cryptic error message and a failed migration. This patch causes migration to fail earlier and provides a meaningful error message stating that migrating read only disks is not supported. Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..c8fb7ec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, /* check whether disk should be migrated */ if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; + + if (disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate read-only disk %s"), disk->dst); + goto cleanup; + } VIR_FREE(diskAlias); if (!(diskAlias = qemuAliasFromDisk(disk))) -- 2.7.4

Polite ping? :) On 09/26/2016 01:16 PM, Corey S. McQuay wrote:
Currently Libvirt allows attempts to migrate read only disks. Qemu cannot handle this as read only disks cannot be written to on the destination system. The end result is a cryptic error message and a failed migration.
This patch causes migration to fail earlier and provides a meaningful error message stating that migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..c8fb7ec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, /* check whether disk should be migrated */ if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; + + if (disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate read-only disk %s"), disk->dst); + goto cleanup; + }
VIR_FREE(diskAlias); if (!(diskAlias = qemuAliasFromDisk(disk)))
-- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)

Very polite double ping... On 10/05/2016 03:38 PM, Jason J. Herne wrote:
Polite ping? :)
On 09/26/2016 01:16 PM, Corey S. McQuay wrote:
Currently Libvirt allows attempts to migrate read only disks. Qemu cannot handle this as read only disks cannot be written to on the destination system. The end result is a cryptic error message and a failed migration.
This patch causes migration to fail earlier and provides a meaningful error message stating that migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..c8fb7ec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, /* check whether disk should be migrated */ if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; + + if (disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate read-only disk %s"), disk->dst); + goto cleanup; + }
VIR_FREE(diskAlias); if (!(diskAlias = qemuAliasFromDisk(disk)))
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Mon, Sep 26, 2016 at 13:16:00 -0400, Corey S. McQuay wrote:
Currently Libvirt allows attempts to migrate read only disks. Qemu cannot handle this as read only disks cannot be written to on the destination system. The end result is a cryptic error message and a failed migration.
This patch causes migration to fail earlier and provides a meaningful error message stating that migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..c8fb7ec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, /* check whether disk should be migrated */ if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; + + if (disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate read-only disk %s"), disk->dst);
This is misaligned.
+ goto cleanup; + }
VIR_FREE(diskAlias); if (!(diskAlias = qemuAliasFromDisk(disk)))
This patch fails syntax check due to trailing whitespace: trailing_blank src/qemu/qemu_migration.c:1767: src/qemu/qemu_migration.c:1769: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, maint.mk: found trailing blank(s) make: *** [maint.mk:748: sc_trailing_blank] Error 1 Please make sure you run "make check" and "make syntax-check" prior to submitting patches. I'll adjust the mistakes and push this as the patch is rather trivial. Peter

On 10/12/2016 10:13 AM, Peter Krempa wrote:
On Mon, Sep 26, 2016 at 13:16:00 -0400, Corey S. McQuay wrote:
Currently Libvirt allows attempts to migrate read only disks. Qemu cannot handle this as read only disks cannot be written to on the destination system. The end result is a cryptic error message and a failed migration.
This patch causes migration to fail earlier and provides a meaningful error message stating that migrating read only disks is not supported.
Signed-off-by: Corey S. McQuay <csmcquay@linux.vnet.ibm.com> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..c8fb7ec 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1764,6 +1764,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, /* check whether disk should be migrated */ if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; + + if (disk->src->readonly) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate read-only disk %s"), disk->dst);
This is misaligned.
+ goto cleanup; + }
VIR_FREE(diskAlias); if (!(diskAlias = qemuAliasFromDisk(disk)))
This patch fails syntax check due to trailing whitespace:
trailing_blank src/qemu/qemu_migration.c:1767: src/qemu/qemu_migration.c:1769: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, maint.mk: found trailing blank(s) make: *** [maint.mk:748: sc_trailing_blank] Error 1
Please make sure you run "make check" and "make syntax-check" prior to submitting patches.
I'll adjust the mistakes and push this as the patch is rather trivial.
Thanks for adjusting the syntax errors and pushing the patch. I enjoyed reading the "rather trivial" comment as this is version 3 of the patch. Isn't it good if things become rather trivial? ;-) -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (4)
-
Boris Fiuczynski
-
Corey S. McQuay
-
Jason J. Herne
-
Peter Krempa