[libvirt] [PATCH v2] 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 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..011f349 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2157,6 +2157,13 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, 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; + } + if (!(diskAlias = qemuAliasFromDisk(disk)) || (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s", hoststr, port, diskAlias) < 0)) -- 2.7.4
On Tue, Sep 20, 2016 at 09:40:22 -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 is not necessarily true. Read only disks can sometimes be in fact backed by storage that is writable and it's desired to be migrated. I think you want to specify which storage to migrate using the VIR_MIGRATE_PARAM_MIGRATE_DISKS migration parameter for virDomainMigrate3. http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrate3 http://libvirt.org/html/libvirt-libvirt-domain.html#VIR_MIGRATE_PARAM_MIGRAT... Virsh also implements support for that flag.
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e451ef6..011f349 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2157,6 +2157,13 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, 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; + }
NACK this would break the above use case. You want to specify explicitly which storage to migrate using the above described API. Peter
On Tue, Sep 20, 2016 at 03:53:16PM +0200, Peter Krempa wrote:
On Tue, Sep 20, 2016 at 09:40:22 -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 is not necessarily true. Read only disks can sometimes be in fact backed by storage that is writable and it's desired to be migrated.
If 'def->readonly' is true, then the security drivers won't allow QEMU to write to the image, regardless of whether the underlying storage is writable. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Tue, Sep 20, 2016 at 15:21:16 +0100, Daniel Berrange wrote:
On Tue, Sep 20, 2016 at 03:53:16PM +0200, Peter Krempa wrote:
On Tue, Sep 20, 2016 at 09:40:22 -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 is not necessarily true. Read only disks can sometimes be in fact backed by storage that is writable and it's desired to be migrated.
If 'def->readonly' is true, then the security drivers won't allow QEMU to write to the image, regardless of whether the underlying storage is writable.
That definitely would be just a bug in the implementation rather than a reason to do this. In fact the problem is that qemu itself forbids to write the data to a readonly marked block backend, which indeed makes this impossible although I'm pretty certain that it worked for me in some configuration. At any rate, checking this on the source of migration is incorrect. The destination should do such check if there's currently no way to persuade qemu do do it. We still may later find a way and all hosts running older versions would be hosed. Peter
On Tue, Sep 20, 2016 at 05:49:17PM +0200, Peter Krempa wrote:
On Tue, Sep 20, 2016 at 15:21:16 +0100, Daniel Berrange wrote:
On Tue, Sep 20, 2016 at 03:53:16PM +0200, Peter Krempa wrote:
On Tue, Sep 20, 2016 at 09:40:22 -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 is not necessarily true. Read only disks can sometimes be in fact backed by storage that is writable and it's desired to be migrated.
If 'def->readonly' is true, then the security drivers won't allow QEMU to write to the image, regardless of whether the underlying storage is writable.
That definitely would be just a bug in the implementation rather than a reason to do this.
That's not a bug - that's a feature - if the disk is marked readonly, SELinux is right to prevent QEMU writing to the disk.
In fact the problem is that qemu itself forbids to write the data to a readonly marked block backend, which indeed makes this impossible although I'm pretty certain that it worked for me in some configuration.
That's a further layer of protection/complication, but I'm concerned primarily about fact that libvirt is intentionally blocking writes
At any rate, checking this on the source of migration is incorrect. The destination should do such check if there's currently no way to persuade qemu do do it. We still may later find a way and all hosts running older versions would be hosed.
So the check would belong in qemuMigrationStartNBDServer, rather than in the DriveMirror method Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Corey S. McQuay -
Daniel P. Berrange -
Peter Krempa