migration: Don't activate block devices if using -S

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
  migration/migration.c | 22 +++++++++++++++-------
  1 file changed, 15 insertions(+), 7 deletions(-)

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

---
 migration/migration.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

migration/migration.c +15 -7

index 52a5092add..58bd382730 100644
@@ -306,13 +306,21 @@ static void process_incoming_migration_bh(void *opaque)
     Error *local_err = NULL;
     MigrationIncomingState *mis = opaque;
 
-    /* Make sure all file formats flush their mutable metadata.
-     * If we get an error here, just don't restart the VM yet. */
-    bdrv_invalidate_cache_all(&local_err);
-    if (local_err) {
-        error_report_err(local_err);
-        local_err = NULL;
-        autostart = false;
+    /* Only fire up the block code now if we're going to restart the
+     * VM, else 'cont' will do it.
+     * This causes file locking to happen; so we don't want it to happen
+     * unless we really are starting the VM.
+     */
+    if (autostart && (!global_state_received() ||
+        global_state_get_runstate() == RUN_STATE_RUNNING)) {
+        /* Make sure all file formats flush their mutable metadata.
+         * If we get an error here, just don't restart the VM yet. */
+        bdrv_invalidate_cache_all(&local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            local_err = NULL;
+            autostart = false;
+        }
     }
 
     /*

Signature · 1 line

2.14.3

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

Kevin

Quoted 27 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

Quoted 4 lines

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 33 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 31 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Quoted 3 lines

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

I wonder if they can't already grant the destination QEMU the necessary permission in the pre-switchover phase. Just a thought, I don't know how this works in detail, so it might not possible after all.

Quoted 8 lines

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.

Kevin

Quoted 34 lines

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 33 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 31 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Quoted 7 lines

Quoted 3 lines

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

I wonder if they can't already grant the destination QEMU the necessary permission in the pre-switchover phase. Just a thought, I don't know how this works in detail, so it might not possible after all.

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

Quoted 16 lines

Quoted 8 lines

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

Quoted 7 lines

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 52 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 34 lines

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 33 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 31 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Quoted 7 lines

Quoted 3 lines

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

I wonder if they can't already grant the destination QEMU the necessary permission in the pre-switchover phase. Just a thought, I don't know how this works in detail, so it might not possible after all.

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

Quoted 24 lines

Quoted 23 lines

Quoted 8 lines

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

And yes, you're probably right that we would have to be more careful to catch inactive images without crashing. On the other hand, it would become a state that is easier to test because it can be directly influenced via QMP rather than being only a side effect of migration.

Kevin

Quoted 59 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 52 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 34 lines

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 33 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 31 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Quoted 7 lines

Quoted 3 lines

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

I wonder if they can't already grant the destination QEMU the necessary permission in the pre-switchover phase. Just a thought, I don't know how this works in detail, so it might not possible after all.

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

Quoted 8 lines

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

I'll leave Jiri to reply to this; I think this is a case of the source realising libvirt has restarted, then trying to recover all of it's VMs without being in the position of being able to check on the destination.

Quoted 29 lines

Quoted 28 lines

Quoted 16 lines

Quoted 8 lines

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

Quoted 7 lines

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Quoted 3 lines

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

I'd only want the 'block-activate' in the case of this new block-job you're suggesting; not in the case of normal migrates - they'd still get it when they do 'cont' - so the change in behaviour is only with that block-job case that must start before the end of migrate.

Quoted 4 lines

And yes, you're probably right that we would have to be more careful to catch inactive images without crashing. On the other hand, it would become a state that is easier to test because it can be directly influenced via QMP rather than being only a side effect of migration.

Yes; but crashing is really bad, so we should really really stopping asserting all over.

Am 09.04.2018 um 16:04 hat Dr. David Alan Gilbert geschrieben:

Quoted 106 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 59 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 52 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 34 lines

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 33 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 31 lines

Am 28.03.2018 um 19:02 hat Dr. David Alan Gilbert (git) geschrieben:

Quoted 7 lines

From: "Dr. David Alan Gilbert" <[email protected]>

Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken.

Quoted 5 lines

Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854

Signed-off-by Dr. David Alan Gilbert <[email protected]>

I'm not sure that this is a good idea. Going back to my old writeup of the migration phases...

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07917.html

...the phase between migration completion and 'cont' is described like this:

b) Migration converges:
Both VMs are stopped (assuming -S is given on the destination, otherwise this phase is skipped), the destination is in control of the resources

This patch changes the definition of the phase so that neither side is in control of the resources. We lose the phase where the destination is in control, but the VM isn't running yet. This feels like a problem to me.

Quoted 7 lines

Quoted 3 lines

But see Jiri's writeup on that bz; libvirt is hitting the opposite
problem; in this corner case they can't have the destination taking
control yet.

I wonder if they can't already grant the destination QEMU the necessary permission in the pre-switchover phase. Just a thought, I don't know how this works in detail, so it might not possible after all.

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

Quoted 8 lines

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

I'll leave Jiri to reply to this; I think this is a case of the source realising libvirt has restarted, then trying to recover all of it's VMs without being in the position of being able to check on the destination.

Quoted 32 lines

Quoted 24 lines

Quoted 16 lines

Quoted 8 lines

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

Quoted 7 lines

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

Quoted 4 lines

I'd only want the 'block-activate' in the case of this new block-job you're suggesting; not in the case of normal migrates - they'd still get it when they do 'cont' - so the change in behaviour is only with that block-job case that must start before the end of migrate.

I'm not aware of having suggested a new block job?

Quoted 7 lines

Quoted 4 lines

And yes, you're probably right that we would have to be more careful to catch inactive images without crashing. On the other hand, it would become a state that is easier to test because it can be directly influenced via QMP rather than being only a side effect of migration.

Yes; but crashing is really bad, so we should really really stopping asserting all over.

Are you aware of any wrong assertions currently?

The thing is, inactive images can only happen in a fairly restricted set of scenarios today - either on the source after migration completed, or on the destination before it completed. If you get any write I/O requests in these states, that's a QEMU bug, so assertions to catch these bugs feel right to me.

Kevin

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Quoted 5 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Quoted 3 lines

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Am 10.04.2018 um 09:36 hat Jiri Denemark geschrieben:

Quoted 22 lines · Kevin Wolf

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 8 lines

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Hm, okay...

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

Quoted 3 lines

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Yeah, that's exactly the thing that we wouldn't need if we could use 'block-inactivate' on the source instead. It feels a bit wrong to design a more involved QEMU interface around the libvirt internals,

but
as long as we implement both sides for symmetry and libvirt just happens to pick the destination side for now, I think it's okay.

By the way, are block devices the only thing that need to be explicitly activated? For example, what about qemu_announce_self() for network cards, do we need to delay that, too?

In any case, I think this patch needs to be reverted for 2.12 because it's wrong, and then we can create the proper solution in the 2.13 timefrage.

Kevin

Quoted 47 lines

Am 10.04.2018 um 09:36 hat Jiri Denemark geschrieben:

Quoted 22 lines · Kevin Wolf

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 8 lines

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Hm, okay...

Quoted 12 lines

Quoted 8 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Quoted 3 lines

Yeah, that's exactly the thing that we wouldn't need if we could use 'block-inactivate' on the source instead. It feels a bit wrong to design a more involved QEMU interface around the libvirt internals,

It's not necessarily 'libvirt internals' - it's a case of them having to cope with recovering from failures that happen around migration; it's not an easy problem, and if they've got a way to stop both sides running at the same time that's pretty important.

Quoted 8 lines

but
as long as we implement both sides for symmetry and libvirt just happens to pick the destination side for now, I think it's okay.

By the way, are block devices the only thing that need to be explicitly activated? For example, what about qemu_announce_self() for network cards, do we need to delay that, too?

Quoted 3 lines

In any case, I think this patch needs to be reverted for 2.12 because it's wrong, and then we can create the proper solution in the 2.13 timefrage.

what case does this break?
I'm a bit wary of reverting this, which fixes a known problem, on the basis that it causes a theoretical problem.

Am 10.04.2018 um 10:45 hat Dr. David Alan Gilbert geschrieben:

Quoted 48 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 47 lines

Am 10.04.2018 um 09:36 hat Jiri Denemark geschrieben:

Quoted 22 lines · Kevin Wolf

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 8 lines

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Hm, okay...

Quoted 12 lines

Quoted 8 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Quoted 8 lines

Quoted 3 lines

Yeah, that's exactly the thing that we wouldn't need if we could use 'block-inactivate' on the source instead. It feels a bit wrong to design a more involved QEMU interface around the libvirt internals,

It's not necessarily 'libvirt internals' - it's a case of them having to cope with recovering from failures that happen around migration; it's not an easy problem, and if they've got a way to stop both sides running at the same time that's pretty important.

The 'libvirt internals' isn't that it needs an additional state where neither source nor destination QEMU own the images, but that it has to be between migration completion and image activation on the destination rather than between image inactivation on the source and migration completion. The latter would be much easier for qemu, but apparently it doesn't work for libvirt because of how it works internally.

But as I said, I'd just implement both for symmetry and then management tools can pick whatever makes their life easier.

Quoted 8 lines

Quoted 8 lines

but
as long as we implement both sides for symmetry and libvirt just happens to pick the destination side for now, I think it's okay.

By the way, are block devices the only thing that need to be explicitly activated? For example, what about qemu_announce_self() for network cards, do we need to delay that, too?

Quoted 7 lines

Quoted 3 lines

In any case, I think this patch needs to be reverted for 2.12 because it's wrong, and then we can create the proper solution in the 2.13 timefrage.

what case does this break?
I'm a bit wary of reverting this, which fixes a known problem, on the basis that it causes a theoretical problem.

It breaks the API. And the final design we're having in mind now is compatible with the old API, not with the new one exposed by this patch, so that switch would break the API again to get back to the old state.

Do you know all the scripts that people are using around QEMU? I don't, but I know that plenty of them exist, so I don't think we can declare this API breakage purely theoretical.

Yes, the patch fixes a known problem, but also a problem that is a rare corner case error that you can only hit with really bad timing. Do we really want to risk unconditionally breaking success cases for fixing a mostly theoretical corner case error path (with the failure mode that the guest is paused when it shouldn't be)?

Kevin

Quoted 58 lines

Am 10.04.2018 um 10:45 hat Dr. David Alan Gilbert geschrieben:

Quoted 48 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 47 lines

Am 10.04.2018 um 09:36 hat Jiri Denemark geschrieben:

Quoted 22 lines · Kevin Wolf

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 8 lines

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Hm, okay...

Quoted 12 lines

Quoted 8 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Quoted 8 lines

Quoted 3 lines

Yeah, that's exactly the thing that we wouldn't need if we could use 'block-inactivate' on the source instead. It feels a bit wrong to design a more involved QEMU interface around the libvirt internals,

It's not necessarily 'libvirt internals' - it's a case of them having to cope with recovering from failures that happen around migration; it's not an easy problem, and if they've got a way to stop both sides running at the same time that's pretty important.

Quoted 6 lines

The 'libvirt internals' isn't that it needs an additional state where neither source nor destination QEMU own the images, but that it has to be between migration completion and image activation on the destination rather than between image inactivation on the source and migration completion. The latter would be much easier for qemu, but apparently it doesn't work for libvirt because of how it works internally.

I suspect this is actually a fundamental requirement to ensuring that we don't end up with a QEMU running on both sides rather than how libvirt is structured.

Quoted 27 lines

But as I said, I'd just implement both for symmetry and then management tools can pick whatever makes their life easier.

Quoted 8 lines

Quoted 8 lines

but
as long as we implement both sides for symmetry and libvirt just happens to pick the destination side for now, I think it's okay.

By the way, are block devices the only thing that need to be explicitly activated? For example, what about qemu_announce_self() for network cards, do we need to delay that, too?

Quoted 7 lines

Quoted 3 lines

In any case, I think this patch needs to be reverted for 2.12 because it's wrong, and then we can create the proper solution in the 2.13 timefrage.

what case does this break?
I'm a bit wary of reverting this, which fixes a known problem, on the basis that it causes a theoretical problem.

It breaks the API. And the final design we're having in mind now is compatible with the old API, not with the new one exposed by this patch, so that switch would break the API again to get back to the old state.

Do you know all the scripts that people are using around QEMU? I don't, but I know that plenty of them exist, so I don't think we can declare this API breakage purely theoretical.

Quoted 5 lines

Yes, the patch fixes a known problem, but also a problem that is a rare corner case error that you can only hit with really bad timing. Do we really want to risk unconditionally breaking success cases for fixing a mostly theoretical corner case error path (with the failure mode that the guest is paused when it shouldn't be)?

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Can you explain the way you see it working?

Am 10.04.2018 um 12:40 hat Dr. David Alan Gilbert geschrieben:

Quoted 66 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 58 lines

Am 10.04.2018 um 10:45 hat Dr. David Alan Gilbert geschrieben:

Quoted 48 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 47 lines

Am 10.04.2018 um 09:36 hat Jiri Denemark geschrieben:

Quoted 22 lines · Kevin Wolf

Quoted 21 lines

Am 09.04.2018 um 12:27 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines

It's a fairly hairy failure case they had; if I remember correctly it's:
a) Start migration
b) Migration gets to completion point
c) Destination is still paused
d) Libvirt is restarted on the source
e) Since libvirt was restarted it fails the migration (and hence knows the destination won't be started)
f) It now tries to resume the qemu on the source

Quoted 5 lines

(f) fails because (b) caused the locks to be taken on the destination; hence this patch stops doing that. It's a case we don't really think about - i.e. that the migration has actually completed and all the data is on the destination, but libvirt decides for some other reason to abandon migration.

If you do remember correctly, that scenario doesn't feel tricky at all. libvirt needs to quit the destination qemu, which will inactivate the images on the destination and release the lock, and then it can continue the source.

Quoted 8 lines

Quoted 3 lines

In fact, this is so straightforward that I wonder what else libvirt is doing. Is the destination qemu only shut down after trying to continue the source? That would be libvirt using the wrong order of steps.

There's no connection between the two libvirt daemons in the case we're talking about so they can't really synchronize the actions. The destination daemon will kill the new QEMU process and the source will resume the old one, but the order is completely random.

Hm, okay...

Quoted 12 lines

Quoted 8 lines

Quoted 4 lines

Yes it was a 'block-activate' that I'd wondered about. One complication is that if this now under the control of the management layer then we should stop asserting when the block devices aren't in the expected state and just cleanly fail the command instead.

Requiring an explicit 'block-activate' on the destination would be an incompatible change, so you would have to introduce a new option for that. 'block-inactivate' on the source feels a bit simpler.

As I said in another email, the explicit block-activate command could depend on a migration capability similarly to how pre-switchover state works.

Quoted 8 lines

Quoted 3 lines

Yeah, that's exactly the thing that we wouldn't need if we could use 'block-inactivate' on the source instead. It feels a bit wrong to design a more involved QEMU interface around the libvirt internals,

It's not necessarily 'libvirt internals' - it's a case of them having to cope with recovering from failures that happen around migration; it's not an easy problem, and if they've got a way to stop both sides running at the same time that's pretty important.

Quoted 6 lines

The 'libvirt internals' isn't that it needs an additional state where neither source nor destination QEMU own the images, but that it has to be between migration completion and image activation on the destination rather than between image inactivation on the source and migration completion. The latter would be much easier for qemu, but apparently it doesn't work for libvirt because of how it works internally.

Quoted 3 lines

I suspect this is actually a fundamental requirement to ensuring that we don't end up with a QEMU running on both sides rather than how libvirt is structured.

I don't think so. In theory, both options can provide the same. If anything, it's related specifically to the phases that Jirka described that libvirt uses to implement migration.

Quoted 40 lines

Quoted 27 lines

But as I said, I'd just implement both for symmetry and then management tools can pick whatever makes their life easier.

Quoted 8 lines

Quoted 8 lines

but
as long as we implement both sides for symmetry and libvirt just happens to pick the destination side for now, I think it's okay.

By the way, are block devices the only thing that need to be explicitly activated? For example, what about qemu_announce_self() for network cards, do we need to delay that, too?

Quoted 7 lines

Quoted 3 lines

In any case, I think this patch needs to be reverted for 2.12 because it's wrong, and then we can create the proper solution in the 2.13 timefrage.

what case does this break?
I'm a bit wary of reverting this, which fixes a known problem, on the basis that it causes a theoretical problem.

It breaks the API. And the final design we're having in mind now is compatible with the old API, not with the new one exposed by this patch, so that switch would break the API again to get back to the old state.

Do you know all the scripts that people are using around QEMU? I don't, but I know that plenty of them exist, so I don't think we can declare this API breakage purely theoretical.

Quoted 5 lines

Yes, the patch fixes a known problem, but also a problem that is a rare corner case error that you can only hit with really bad timing. Do we really want to risk unconditionally breaking success cases for fixing a mostly theoretical corner case error path (with the failure mode that the guest is paused when it shouldn't be)?

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Quoted 1 line

Can you explain the way you see it working?

The key is making the delayed activation of block devices (and probably delayed announcement of NICs? - you didn't answer that part) optional instead of making it the default.

We can use Jirka's suggestion of adding a migration capability that enables it, or I suppose a new option to -incoming could work, too. It doesn't really matter what the syntax is, but the management tool must request it explicitly.

Kevin

Quoted 10 lines

Am 10.04.2018 um 12:40 hat Dr. David Alan Gilbert geschrieben:

Quoted 8 lines

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Can you explain the way you see it working?

Quoted 3 lines

The key is making the delayed activation of block devices (and probably delayed announcement of NICs? - you didn't answer that part) optional instead of making it the default.

NIC announcments are broken in similar but slightly different ways; we did have a series on list to help a while ago but it never got merged; I'd like to keep that mess separate.

Quoted 4 lines

We can use Jirka's suggestion of adding a migration capability that enables it, or I suppose a new option to -incoming could work, too. It doesn't really matter what the syntax is, but the management tool must request it explicitly.

A new capability is easy to gate the change in behaviour that this patch added; I'll do that first thing for 2.13 (given today is rc3 tag it's too late).

However, once we turn this on, to cope with the situation of a block user that must start prior to the 'cont' when this behaviour is active, we'd also need the 'block-activate' command.

Am 10.04.2018 um 16:22 hat Dr. David Alan Gilbert geschrieben:

Quoted 15 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 13 lines

Am 10.04.2018 um 12:40 hat Dr. David Alan Gilbert geschrieben:

Quoted 8 lines

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Can you explain the way you see it working?

The key is making the delayed activation of block devices (and probably delayed announcement of NICs? - you didn't answer that part) optional instead of making it the default.

Quoted 3 lines

NIC announcments are broken in similar but slightly different ways; we did have a series on list to help a while ago but it never got merged; I'd like to keep that mess separate.

Okay. I just thought that it would make sense to have clear migration phases that are the same for all external resources that the QEMU processes use.

Quoted 16 lines

Am 10.04.2018 um 16:22 hat Dr. David Alan Gilbert geschrieben:

Quoted 15 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 13 lines

Am 10.04.2018 um 12:40 hat Dr. David Alan Gilbert geschrieben:

Quoted 8 lines

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Can you explain the way you see it working?

The key is making the delayed activation of block devices (and probably delayed announcement of NICs? - you didn't answer that part) optional instead of making it the default.

Quoted 7 lines

Quoted 3 lines

NIC announcments are broken in similar but slightly different ways; we did have a series on list to help a while ago but it never got merged; I'd like to keep that mess separate.

Okay. I just thought that it would make sense to have clear migration phases that are the same for all external resources that the QEMU processes use.

I don't think NIC announcements should be delayed in this specific case since we're dealing with a failure recovery which should be rare in comparison to successful migration when we want NIC announcements to be send early. In other words, any NIC issues should be solved separately and Laine would likely be a better person for discussing them since he has a broader knowledge of all the fancy network stuff which libvirt needs to coordinate with.

Am 11.04.2018 um 12:01 hat Jiri Denemark geschrieben:

Quoted 25 lines · Kevin Wolf

Quoted 16 lines

Am 10.04.2018 um 16:22 hat Dr. David Alan Gilbert geschrieben:

Quoted 15 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 13 lines

Am 10.04.2018 um 12:40 hat Dr. David Alan Gilbert geschrieben:

Quoted 8 lines

Hmm; having chatted to Jiri I'm OK with reverting it, on the condition that I actually understand how this alternative would work first.

I can't currently see how a block-inactivate would be used. I also can't see how a block-activate unless it's also with the change that you're asking to revert.

Can you explain the way you see it working?

The key is making the delayed activation of block devices (and probably delayed announcement of NICs? - you didn't answer that part) optional instead of making it the default.

Quoted 7 lines

Quoted 3 lines

NIC announcments are broken in similar but slightly different ways; we did have a series on list to help a while ago but it never got merged; I'd like to keep that mess separate.

Okay. I just thought that it would make sense to have clear migration phases that are the same for all external resources that the QEMU processes use.

Quoted 7 lines

I don't think NIC announcements should be delayed in this specific case since we're dealing with a failure recovery which should be rare in comparison to successful migration when we want NIC announcements to be send early. In other words, any NIC issues should be solved separately and Laine would likely be a better person for discussing them since he has a broader knowledge of all the fancy network stuff which libvirt needs to coordinate with.

Well, if I were the migration maintainer, I would insist on a properly designed phase model that solves the problem once and for all because it would be clear where everything belongs. We could still have bugs in the future, but that would be internal implementation bugs with no effect on the API.

But I'm not the maintainer and Dave prefers to deal with it basically as a bunch of one-off fixes, and that will work, too. It will probably clutter up the external API a bit (because the management tool will have to separately address migration of block devices, network devices and possibly other things in the future), but that shouldn't matter much for libvirt. Maybe what we do need is some documentation of the recommended process for performing a live migration so that management tools know which QMP commands they need to issue when.

Kevin

Am 03.04.2018 um 22:52 hat Dr. David Alan Gilbert geschrieben:

Quoted 9 lines · * Kevin Wolf ([email protected]) wrote:

Quoted 8 lines

Consider a case where the management tool keeps a mirror job with sync=none running to expose all I/O requests to some external process. It needs to shut down the old block job on the source in the 'pre-switchover' state, and start a new block job on the destination when the destination controls the images, but the VM doesn't run yet (so that it doesn't miss an I/O request). This patch removes the migration phase that the management tool needs to implement this correctly.

Quoted 7 lines

Quoted 3 lines

If we need a "neither side has control" phase, we might need to introduce it in addition to the existing phases rather than replacing a phase that is still needed in other cases.

This is yet another phase to be added.
IMHO this needs the managment tool to explicitly take control in the case you're talking about.

What kind of mechanism do you have in mind there?

Maybe what could work would be separate QMP commands to inactivate (and possibly for symmetry activate) all block nodes. Then the management tool could use the pre-switchover phase to shut down its block jobs etc., inactivate all block nodes, transfer its own locks and then call migrate-continue.