<div dir="ltr">On Mon, Jun 6, 2016 at 11:26 PM, Matt Riedemann <span dir="ltr"><<a href="mailto:mriedem@linux.vnet.ibm.com" target="_blank">mriedem@linux.vnet.ibm.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On 6/6/2016 12:15 PM, Matt Riedemann wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
On 1/8/2016 12:28 PM, Mark McLoughlin wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
On Fri, 2016-01-08 at 14:11 +0000, Daniel P. Berrange wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
On Thu, Jan 07, 2016 at 09:07:00PM +0000, Mark McLoughlin wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
On Thu, 2016-01-07 at 12:23 +0100, Sahid Orentino Ferdjaoui<br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
On Mon, Jan 04, 2016 at 09:12:06PM +0000, Mark McLoughlin<br>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Hi<br>
<br>
commit 8ecf93e[1] got me thinking - the live_migration_flag<br>
config option unnecessarily allows operators choose arbitrary<br>
behavior of the migrateToURI() libvirt call, to the extent<br>
that we allow the operator to configure a behavior that can<br>
result in data loss[1].<br>
<br>
I see that danpb recently said something similar:<br>
<br>
<a href="https://review.openstack.org/171098" rel="noreferrer" target="_blank">https://review.openstack.org/171098</a><br>
<br>
"Honestly, I wish we'd just kill off  'live_migration_flag'<br>
and 'block_migration_flag' as config options. We really<br>
should not be exposing low level libvirt API flags as admin<br>
tunable settings.<br>
<br>
Nova should really be in charge of picking the correct set of<br>
flags for the current libvirt version, and the operation it<br>
needs to perform. We might need to add other more sensible<br>
config options in their place [..]"<br>
</blockquote>
<br>
Nova should really handle internal flags and this serie is<br>
running in the right way.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
...<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
4) Add a new config option for tunneled versus native:<br>
<br>
[libvirt] live_migration_tunneled = true<br>
<br>
This enables the use of the VIR_MIGRATE_TUNNELLED flag. We<br>
have historically defaulted to tunneled mode because it<br>
requires the least configuration and is currently the only<br>
way to have a secure migration channel.<br>
<br>
danpb's quote above continues with:<br>
<br>
"perhaps a "live_migration_secure_channel" to indicate that<br>
migration must use encryption, which would imply use of<br>
TUNNELLED flag"<br>
<br>
So we need to discuss whether the config option should<br>
express the choice of tunneled vs native, or whether it<br>
should express another choice which implies tunneled vs<br>
native.<br>
<br>
<a href="https://review.openstack.org/263434" rel="noreferrer" target="_blank">https://review.openstack.org/263434</a><br>
</blockquote>
<br>
We probably have to consider that operator does not know much<br>
about internal libvirt flags, so options we are exposing for<br>
him should reflect benefice of using them. I commented on your<br>
review we should at least explain benefice of using this option<br>
whatever the name is.<br>
</blockquote>
<br>
As predicted, plenty of discussion on this point in the review<br>
:)<br>
<br>
You're right that we don't give the operator any guidance in the<br>
help message about how to choose true or false for this:<br>
<br>
Whether to use tunneled migration, where migration data is<br>
transported over the libvirtd connection. If True, we use the<br>
VIR_MIGRATE_TUNNELLED migration flag<br>
<br>
libvirt's own docs on this are here:<br>
<br>
<a href="https://libvirt.org/migration.html#transport" rel="noreferrer" target="_blank">https://libvirt.org/migration.html#transport</a><br>
<br>
which emphasizes:<br>
<br>
- the data copies involved in tunneling - the extra configuration<br>
steps required for native - the encryption support you get when<br>
tunneling<br>
<br>
The discussions I've seen on this topic wrt Nova have revolved<br>
around:<br>
<br>
- that tunneling allows for an encrypted transport[1] - that<br>
qemu's NBD based drive-mirror block migration isn't supported<br>
using tunneled mode, and that danpb is working on fixing this<br>
limitation in libvirt - "selective" block migration[2] won't work<br>
with the fallback qemu block migration support, and so won't<br>
currently work in tunneled mode<br>
</blockquote>
<br>
I'm not working on fixing it, but IIRC some other dev had proposed<br>
patches.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
So, the advise to operators would be:<br>
<br>
- You may want to choose tunneled=False for improved block<br>
migration capabilities, but this limitation will go away in<br>
future. - You may want to choose tunneled=False if you wish to<br>
trade and encrypted transport for a (potentially negligible)<br>
performance improvement.<br>
<br>
Does that make sense?<br>
<br>
As for how to name the option, and as I said in the review, I<br>
think it makes sense to be straightforward here and make it<br>
clearly about choosing to disable libvirt's tunneled transport.<br>
<br>
If we name it any other way, I think our explanation for<br>
operators will immediately jump to explaining (a) that it<br>
influences the TUNNELLED flag, and (b) the differences between<br>
the tunneled and native transports. So, if we're going to have to<br>
talk about tunneled versus native, why obscure that detail?<br>
</blockquote>
<br>
Ultimately we need to recognise that libvirt's tunnelled mode was<br>
added as a hack, to work around fact that QEMU lacked any kind of<br>
native security capabilities & didn't appear likely to ever get<br>
them at that time.  As well as not working with modern NBD based<br>
block device encryption, it really sucks for performance because it<br>
introduces many extra data copies. So it is going to be quite poor<br>
for large VMs with heavy rate of data dirtying.<br>
<br>
The only long term relative "benefit" of tunnelled mode is that it<br>
avoids the need to open extra firewall ports.<br>
<br>
IMHO, the long term future is to *never* use tunnelled mode for<br>
QEMU. This will be viable when my support for native TLS support in<br>
QEMU migration + NBD protocols is merged. I'm hopeful this wil be<br>
for QEMU 2.6<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
But, Pawel strongly disagrees.<br>
<br>
One last point I'd make is this isn't about adding a *new*<br>
configuration capability for operators. As we deprecate and<br>
remove these configuration options, we need to be careful not to<br>
remove a capability that operators are currently depending on for<br>
arguably reasonable reasons.<br>
</blockquote>
<br>
My view is that "live_migration_tunneled" is a reasonable<br>
parameter to add, because there is a genuine need to let admins<br>
choose this behaviour. We should make sure it is correctly done as<br>
a tri-state flag though, when it is 'None', Nova should pick what<br>
it things is the best approach based on QEMU version. Probably to<br>
use QEMU native when it supports TLS, otherwise use tunnelled if<br>
possible to get security.<br>
</blockquote>
<br>
Great feedback. I buy that.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
[1] - <a href="https://review.openstack.org/#/c/171098/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/171098/</a> [2] -<br>
<a href="https://review.openstack.org/#/c/227278" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/227278</a><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
5) Add a new config option for additional migration flags:<br>
<br>
[libvirt] live_migration_extra_flags =<br>
VIR_MIGRATE_COMPRESSED<br>
<br>
This allows operators to continue to experiment with libvirt<br>
behaviors in safe ways without each use case having to be<br>
accounted for.<br>
<br>
<a href="https://review.openstack.org/263435" rel="noreferrer" target="_blank">https://review.openstack.org/263435</a><br>
<br>
We would disallow setting the following flags via this<br>
option:<br>
<br>
VIR_MIGRATE_LIVE VIR_MIGRATE_PEER2PEER VIR_MIGRATE_TUNNELLED<br>
VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE<br>
VIR_MIGRATE_NON_SHARED_INC VIR_MIGRATE_NON_SHARED_DISK<br>
<br>
which would allow the following currently available flags to<br>
be set:<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
VIR_MIGRATE_PAUSED VIR_MIGRATE_CHANGE_PROTECTION<br>
VIR_MIGRATE_UNSAFE VIR_MIGRATE_OFFLINE<br>
VIR_MIGRATE_COMPRESSED VIR_MIGRATE_ABORT_ON_ERROR<br>
VIR_MIGRATE_AUTO_CONVERGE VIR_MIGRATE_RDMA_PIN_ALL<br>
</blockquote>
<br>
We can probably consider to provide VIR_MIGRATE_PAUSED and<br>
VIR_MIGRATE_COMPRESSED as dedicated options too ?<br>
</blockquote>
<br>
Yes, I think any options we see regularly added to extra_flags<br>
by operators, and as we understand the use cases better, then we<br>
can add dedicated options for them.<br>
</blockquote>
<br>
I really don't see a case for letting the admin set<br>
VIR_MIGRATE_PAUSED at a host level. If we want the ability to force<br>
a running VM to end up paused after migration, this is a feature to<br>
be added to the Nova migration API.<br>
<br>
The VIR_MIGRATE_COMPRESSED is not as simple as just enabling a<br>
flag, there are other associated runtime tunables that need<br>
setting. There was a spec discussing this which was not approved as<br>
a suitable strategy for using it could not be agreed.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
In the review, Pawel is making a case for not allowing the<br>
operator to enable COMPRESSED or AUTO_CONVERGE.<br>
</blockquote>
<br>
I agree really. As per my comments, I in fact struggle to see a<br>
credible case for allowing any of the remaining flags to be<br>
enabled. They are all cases that Nova should be made todo the right<br>
thing, possibly in relation to API parameters or other deployment<br>
choices.<br>
</blockquote>
<br>
Fair enough. I figured it would be a necessary safety valve against<br>
operators who value the flexibility of the current configuration<br>
options, but you make a good case.<br>
<br>
I'll drop the extra_flags option and make tunneled a tri-state.<br>
<br>
Thanks, Mark.<br>
<br>
__________________________________________________________________________<br>
<br>
<br>
<br>
</blockquote>
OpenStack Development Mailing List (not for usage questions)<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Unsubscribe:<br>
<a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
</blockquote>
<br>
I'm going to be a jackass and skip this entire thread with a question.<br>
<br>
The live migration CI job we have in the experimental queue is blocked<br>
on this failure right now [1].<br>
<br>
"Live Migration failure: Operation not supported: Selecting disks to<br>
migrate is not implemented for tunnelled migration"<br>
<br>
That's running with ubuntu 16.04 and libvirt 1.2.17.<br>
<br>
I was looking at the deprecated live_migration_flag which tells you to<br>
use the live_migration_tunnelled flag, which defaults to None and is a<br>
no-op, even though the help text on the option says if it's omitted<br>
we'll do our best to figure out if you want to use tunneling based on<br>
the hypervisor's encryption support.  That doesn't actually happen by<br>
default [2].<br>
<br>
So what should happen? It seems some of this series was stalled or<br>
incomplete?<br>
<br>
We could change the configuration of our live migration CI job, but I'd<br>
like to see the job work with the defaults that we ship.<br>
<br>
I'm not entirely familiar with the thing that's causing the failure<br>
except I believe we're calling migrateToURI3 with the tunnelled flag<br>
(because it's in the default live_migration_flag option) and a list of<br>
devices [3][4].<br>
<br>
Would it be better to use migrateToURI2 if migrate_disks is in params<br>
but the VIR_MIGRATE_TUNNELLED flag is set?<br>
<br>
[1] <a href="https://bugs.launchpad.net/nova/+bug/1589591" rel="noreferrer" target="_blank">https://bugs.launchpad.net/nova/+bug/1589591</a><br>
[2]<br>
<a href="https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L559" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L559</a><br>
<br>
[3]<br>
<a href="https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L5740-5751" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L5740-5751</a><br>
<br>
[4]<br>
<a href="https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/guest.py#L523" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/guest.py#L523</a><br>
<br>
<br>
</blockquote>
<br></div></div>
Timofey is working on a fix here:<br>
<br>
<a href="https://review.openstack.org/#/c/326111/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/326111/</a><br>
<br>
I've left some comments inline, but I'm mostly wondering about this now:<br>
<br>
<a href="https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L5360" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/f5c9ebd56075f8eb04f9f0e683f85bacdcd68c38/nova/virt/libvirt/driver.py#L5360</a><br>
<br>
The code there says that if there are BDMs and libvirt<1.2.17 and you're doing block migration, it's unsupported and fails. The comment also says that it can't do that in tunneled method either - which is the bug we're hitting.<br>
<br>
But should we add the block_migration_flag check in check_can_live_migrate_source? Or does it work to live migrate in tunneled mode as long as we don't pass 'migrate_disks'?</blockquote><div>So we got 2 options:</div><div><ul><li> raise exception in case of tunneled block migration with migrate_disks param </li><ul><li> as migrate flags are supposed to be nova internals this is bad option</li></ul><li> get rid of one of these option: <br></li><ul><li>don't use VIR_MIGRATE_TUNNELLED - looks most preferable for now</li><li>don't pass migrate disks param (it will force block live migration to fail for instances with attached volumes, and volume-backed instances with config-drive(vfat))  </li></ul></ul></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">  </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">
<br>
-- <br>
<br>
Thanks,<br>
<br>
Matt Riedemann<br>
<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div></div>