On Wed, Sep 02, 2020 at 11:53:37AM +0200, Jiri Denemark wrote:
On Tue, Sep 01, 2020 at 16:36:59 +0200, Martin Kletzander wrote:
> This allows:
>
> a) migration without access to network
>
> b) complete control of the migration stream
>
> c) easy migration between containerised libvirt daemons on the same host
>
> Resolves:
https://bugzilla.redhat.com/1638889
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> docs/manpages/virsh.rst | 15 +++-
> docs/migration.html.in | 33 ++++++++
> src/qemu/qemu_migration.c | 138 +++++++++++++++++++++++--------
> src/qemu/qemu_migration_params.c | 9 ++
> src/qemu/qemu_migration_params.h | 3 +
> src/qemu/qemu_monitor.c | 15 ++++
> src/qemu/qemu_monitor.h | 4 +
> 7 files changed, 179 insertions(+), 38 deletions(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 4d66019e750b..e3aeaa5c44ea 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -3242,7 +3242,7 @@ with the qemu driver you could use this:
>
> .. code-block::
>
> - qemu+unix://?socket=/path/to/socket
> + qemu+unix:///system?socket=/path/to/socket
>
> When *migrateuri* is not specified, libvirt will automatically determine the
> hypervisor specific URI. Some hypervisors, including QEMU, have an optional
Ah, again I didn't notice the incorrect URI until I saw this patch...
This hunk should go into the previous patch.
My bad, I though I did that.
> @@ -3270,6 +3270,14 @@ There are a few scenarios where specifying
*migrateuri* may help:
> might be specified to choose a specific port number outside the default range in
> order to comply with local firewall policies.
>
> +* The *desturi* uses UNIX transport method. In this advanced case libvirt
> + should not guess a *migrateuri* and it should be specified using
> + UNIX socket path URI:
> +
> +.. code-block::
> +
> + unix:///path/to/socket
> +
> See `https://libvirt.org/migration.html#uris
<
https://libvirt.org/migration.html#uris>`_ for more details on
> migration URIs.
>
> @@ -3296,7 +3304,10 @@ specific parameters separated by '&'. Currently
recognized parameters are
> Optional *listen-address* sets the listen address that hypervisor on the
> destination side should bind to for incoming migration. Both IPv4 and IPv6
> addresses are accepted as well as hostnames (the resolving is done on
> -destination). Some hypervisors do not support this feature and will return an
> +destination). In niche scenarios you can also use UNIX socket to make the
> +hypervisor connection over UNIX socket in which case you must make sure the
> +source can connect to the destination using the socket path provided by you.
> +Some hypervisors do not support specifying the listen address and will return an
> error if this parameter is used.
>
> Optional *disks-port* sets the port that hypervisor on destination side should
> diff --git a/docs/migration.html.in b/docs/migration.html.in
> index e95ee9de6f1b..8585dcab6863 100644
> --- a/docs/migration.html.in
> +++ b/docs/migration.html.in
> @@ -201,6 +201,9 @@
> numbers. In the latter case the management application may wish
> to choose a specific port number outside the default range in order
> to comply with local firewall policies.</li>
> + <li>The second URI uses UNIX transport method. In this advanced case
> + libvirt should not guess a *migrateuri* and it should be specified using
> + UNIX socket path URI:
<code>unix:///path/to/socket</code>.</li>
> </ol>
>
> <h2><a id="config">Configuration file
handling</a></h2>
> @@ -628,5 +631,35 @@ virsh migrate --p2p --tunnelled web1 qemu+ssh://desthost/system
qemu+ssh://10.0.
> Supported by QEMU driver
> </p>
>
> +
> + <h3><a id="scenariounixsocket">Migration using only UNIX
sockets</a></h3>
> +
> + <p>
> + In niche scenarios where libvirt daemon does not have access to the
> + network (e.g. running in a restricted container on a host that has
> + accessible network), when a management application wants to have complete
> + control over the transfer or when migrating between two containers on the
> + same host all the communication can be done using UNIX sockets. This
> + includes connecting to non-standard socket path for the destination
> + daemon, using UNIX sockets for hypervisor's communication or for the NBD
> + data transfer. All of that can be used with both peer2peer and direct
> + migration options.
> + </p>
> +
> + <p>
> + Example using <code>/tmp/migdir</code> as a directory representing
the
> + same path visible from both libvirt daemons. That can be achieved by
> + bind-mounting the same directory to different containers running separate
> + daemons or forwarding connections to these sockets manually
> + (using <code>socat</code>, <code>netcat</code> or a
custom piece of
> + software):
> + <pre>
> +virsh migrate web1 [--p2p] --copy-storage-all
'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver'
'unix:///tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu]
--disks-uri unix:///tmp/migdir/test-sock-nbd
The listen address is a bit strange. With TCP this is the IP address
QEMU on the destination host will be listening for incoming migration.
But do we need this at all for UNIX sockets? I think coming up with a
socket path which would be available on both hosts shouldn't be too
hard. Ability to use different sockets on each side would provide
greater flexibility, but I'm not sure it's worth the effort. Since
normally listen address is just an IP address, using socket path here
seems to be OK as it is the address of a UNIX socket, but it looks
confusing. I don't think forcing unix:///socket/path as the listen
address would make things significantly better. And creating a new
listen URI parameter doesn't sound like a good idea either.
That said, I would either drop support for UNIX sockets in listen
address or keep it the way you implemented it. If you decide to keep it,
proper documented would be appreciated. Mentioning it only as an
optional (and redundant in this case) argument in the example doesn't
seem optimal at all.
And after reading the rest of the patch, I can see we gained the support
for socket path in listen address for free. Forbidding it would require
an explicit check for a valid listen IP address. So I think we could
just enhance the documentation of the listen address to mention UNIX
sockets and keep it supported.
Nope, it gets replaced by the URI path if STREQ(uri->scheme, "unix"), so it
gets
silently ignored. I dropped the hunk adding docs about it and removed the
mention from `docs/migration.html.in`. Now I can either document that it is
ignored for desturi with unix transports or document that it is mutually
exclusive and explicitly forbid it. I already did the former, but due to future
corner cases I should probably do the latter, I think. Let me know which one
would you rather see in v3 (of this and the next patch only, the previous ones
are correct, so I'll push them as is).
Thanks for the review.
> + </pre>
> +
> + <p>
> + Supported by QEMU driver
> + </p>
> +
> </body>
> </html>
...
The rest of the patch is OK.
Jirka