[libvirt] qemu_migration: Precreate missing storage breaks with network drives
Hi Michal, We're testing libvirt-1.2.13 and found it failed to live migrate domains with attached network drives. The change to libvirt was made in commit cf54c606, announced here: https://www.redhat.com/archives/libvir-list/2014-November/msg01053.html It's not necessary to precreate network drives. They're created during nova live migration in openstack. How about this patch? diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c 2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c 2015-03-18 12:18:14.137990147 -0700 @@ -1510,6 +1510,9 @@ case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, Cheers, -- Noel Burton-Krahn Sr, Distributed Systems Engineer Piston Cloud Computing
Actually I messed that up slightly on the case. My C is a bit rusty: diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c 2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c 2015-03-18 13:55:45.873322477 -0700 @@ -1507,9 +1507,12 @@ flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; break; + case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: - case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, On Wed, Mar 18, 2015 at 1:42 PM, Noel Burton-Krahn <noel@pistoncloud.com> wrote:
Hi Michal,
We're testing libvirt-1.2.13 and found it failed to live migrate domains with attached network drives. The change to libvirt was made in commit cf54c606, announced here: https://www.redhat.com/archives/libvir-list/2014-November/msg01053.html
It's not necessary to precreate network drives. They're created during nova live migration in openstack. How about this patch?
diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c 2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c 2015-03-18 12:18:14.137990147 -0700 @@ -1510,6 +1510,9 @@ case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR,
Cheers, -- Noel Burton-Krahn Sr, Distributed Systems Engineer Piston Cloud Computing
On 18.03.2015 22:00, Nick Bartos wrote:
Actually I messed that up slightly on the case. My C is a bit rusty:
diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c2015-03-18 13:55:45.873322477 -0700 @@ -1507,9 +1507,12 @@ flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; break;
+ case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: - case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR,
Sounds reasonable to me. Although I'm worried that there might be users using network disk, supplying different XML on migration, where the disk is switched to yet non-existent network disk, expecting libvirt/qemu to copy the data. The other option is: libvirt won't copy shared or read only disks. So OpenStack would mark network disks as shared. One way or another - do you want to propose a patch or should I do that? Michal
That sounds like a reasonable option. How far back is the shared disk type supported? As of Icehouse (the oldest supported version I believe), the minimum supported libvirt version is 0.9.6. It would be simpler if it could be a simple change that doesn't have to be if/else'd for different libvirt versions. On Thu, Mar 19, 2015 at 3:45 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 18.03.2015 22:00, Nick Bartos wrote:
Actually I messed that up slightly on the case. My C is a bit rusty:
diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c2015-03-18 13:55:45.873322477 -0700 @@ -1507,9 +1507,12 @@ flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; break;
+ case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: - case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR,
Sounds reasonable to me. Although I'm worried that there might be users using network disk, supplying different XML on migration, where the disk is switched to yet non-existent network disk, expecting libvirt/qemu to copy the data.
The other option is: libvirt won't copy shared or read only disks. So OpenStack would mark network disks as shared.
One way or another - do you want to propose a patch or should I do that?
Michal
On 19.03.2015 16:29, Nick Bartos wrote:
That sounds like a reasonable option. How far back is the shared disk type supported? As of Icehouse (the oldest supported version I believe), the minimum supported libvirt version is 0.9.6. It would be simpler if it could be a simple change that doesn't have to be if/else'd for different libvirt versions.
The <shareable/> element was added back in 0.4.2 release (we're talking about mid 2008). So I wouldn't fear of it. Here's an example XML taken from one of our unit tests to show you how to use it: <disk type='block' device='disk'> <driver name='qemu' type='qcow2'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> <serial>XYZXYZXYZYXXYZYZYXYZY</serial> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> Michal
That does sound like the best option then. Would you mind proposing a nova patch? On Thu, Mar 19, 2015 at 8:37 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 19.03.2015 16:29, Nick Bartos wrote:
That sounds like a reasonable option. How far back is the shared disk type supported? As of Icehouse (the oldest supported version I believe), the minimum supported libvirt version is 0.9.6. It would be simpler if it could be a simple change that doesn't have to be if/else'd for different libvirt versions.
The <shareable/> element was added back in 0.4.2 release (we're talking about mid 2008). So I wouldn't fear of it. Here's an example XML taken from one of our unit tests to show you how to use it:
<disk type='block' device='disk'> <driver name='qemu' type='qcow2'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> <serial>XYZXYZXYZYXXYZYZYXYZY</serial> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
Michal
Hi Michal, I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate. Could you propose this patch to libvirt? -- Noel On Thu, Mar 19, 2015 at 3:45 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 18.03.2015 22:00, Nick Bartos wrote:
Actually I messed that up slightly on the case. My C is a bit rusty:
diff -U3 -r libvirt-1.2.13.orig/src/qemu/qemu_migration.c libvirt-1.2.13/src/qemu/qemu_migration.c --- libvirt-1.2.13.orig/src/qemu/qemu_migration.c2015-02-23 22:04:12.000000000 -0800 +++ libvirt-1.2.13/src/qemu/qemu_migration.c2015-03-18 13:55:45.873322477 -0700 @@ -1507,9 +1507,12 @@ flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; break;
+ case VIR_STORAGE_TYPE_NETWORK: + ret = 0; + goto cleanup; + break; case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: - case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR,
Sounds reasonable to me. Although I'm worried that there might be users using network disk, supplying different XML on migration, where the disk is switched to yet non-existent network disk, expecting libvirt/qemu to copy the data.
The other option is: libvirt won't copy shared or read only disks. So OpenStack would mark network disks as shared.
One way or another - do you want to propose a patch or should I do that?
Michal
On Fri, 20 Mar 2015, Noel Burton-Krahn wrote:
Hi Michal,
I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate.
Could you propose this patch to libvirt?
I need to migrate VMs from non-network disks to network disks, and this use case is what led me to post a patch for this to the list a week ago: https://www.redhat.com/archives/libvir-list/2015-March/msg00651.html It'd be nice to have the patch reviewed. :-) - Michael
On 20.03.2015 20:23, Noel Burton-Krahn wrote:
Hi Michal,
I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate.
That's how it was back in the old days. Then libvirt introduced storage migration, but requiring users to pre-create storage on destination themselves. And just recently I taught libvirt to automatically pre-create the storage. However, not for all disk types. If a disk on destination is missing, it's likely broken guest ABI libvirt should have not allowed migration in the first place. How come the migration was allowed? Michal
In our case the network disk is available on the dest before we call migration. It's an openstack volume mounted via ceph. If we disable the copy-check in qemu_migration.c the migration works fine. On Mon, Mar 23, 2015 at 12:17 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 20.03.2015 20:23, Noel Burton-Krahn wrote:
Hi Michal,
I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate.
That's how it was back in the old days. Then libvirt introduced storage migration, but requiring users to pre-create storage on destination themselves. And just recently I taught libvirt to automatically pre-create the storage. However, not for all disk types.
If a disk on destination is missing, it's likely broken guest ABI libvirt should have not allowed migration in the first place. How come the migration was allowed?
Michal
Hi Michal, How can we move this along as a patch to libvirt? Can we submit a patch ourselves? -- Noel On Tue, Mar 24, 2015 at 10:21 AM, Noel Burton-Krahn <noel@pistoncloud.com> wrote:
In our case the network disk is available on the dest before we call migration. It's an openstack volume mounted via ceph. If we disable the copy-check in qemu_migration.c the migration works fine.
On Mon, Mar 23, 2015 at 12:17 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 20.03.2015 20:23, Noel Burton-Krahn wrote:
Hi Michal,
I think issuing a libvirt migrate to a host where the network disks don't already exist would be a prequisite failure. Libvirt can never copy a network disk, but it shouldn't fail trying to migrate an existing domain that contains a network disk. If a libvirt user wishes to migrate a domain that contains a network disk, it's their responsibility to ensure the disk exists before calling migrate.
That's how it was back in the old days. Then libvirt introduced storage migration, but requiring users to pre-create storage on destination themselves. And just recently I taught libvirt to automatically pre-create the storage. However, not for all disk types.
If a disk on destination is missing, it's likely broken guest ABI libvirt should have not allowed migration in the first place. How come the migration was allowed?
Michal
On Thu, 26 Mar 2015, Noel Burton-Krahn wrote:
Hi Michal,
How can we move this along as a patch to libvirt? Can we submit a patch ourselves?
Hi Noel, Michal applied my patch for this as commit a1b1805155. It looks like we've been trying to solve the same problem in different threads; perhaps you didn't see my thread. Michal, thanks for the patch review! - Michael
participants (4)
-
Michael Chapman -
Michal Privoznik -
Nick Bartos -
Noel Burton-Krahn