Re: [libvirt] [PATCH v2] qemu-migration: Disallow migration of read only disk
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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu-migration: Disallow migration of read only disk
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu-migration: Disallow migration of read only disk
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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu-migration: Disallow migration of read only disk
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_MIGRATE_DISKS
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
> Reviewed-by: Jason J. Herne
> Reviewed-by: Boris Fiuczynski
> ---
> 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
Reviewed-by: Jason J. Herne
Reviewed-by: Boris Fiuczynski
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
