[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt PATCH 6/9] qemu: Allow NBD migration over UNIX socket

On Tue, Aug 25, 2020 at 06:01:28PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 16:28:12 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:12AM +0200, Martin Kletzander wrote:
> Adds new typed param for migration and uses this as a UNIX socket path that
> should be used for the NBD part of migration.  And also adds virsh support.
> Partially resolves: https://bugzilla.redhat.com/1638889
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  docs/manpages/virsh.rst          |   8 +++
>  include/libvirt/libvirt-domain.h |  12 ++++
>  src/qemu/qemu_domain.h           |   1 +
>  src/qemu/qemu_driver.c           |  33 ++++++++--
>  src/qemu/qemu_migration.c        | 110 ++++++++++++++++++++++---------
>  src/qemu/qemu_migration.h        |   3 +
>  src/qemu/qemu_migration_cookie.c |  22 +++++--
>  src/qemu/qemu_migration_cookie.h |   1 +
>  tools/virsh-domain.c             |  12 ++++
>  9 files changed, 160 insertions(+), 42 deletions(-)
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 9187037a5615..75f475eea6ad 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -3113,6 +3113,7 @@ migrate
>        [--postcopy-bandwidth bandwidth]
>        [--parallel [--parallel-connections connections]]
>        [--bandwidth bandwidth] [--tls-destination hostname]
> +      [--disks-socket socket-path]

For the primary migration connection we have a proper URI, so we can
support new schemes without adding new parameters for each one.

For the NBD connetion we have the "disk port" parameter only, not a
full URI.

Adding "disks socket" makes this design mistake worse.

IMHO we should add a "disks uri" accepting the exact same syntax
as the migration URI.

Oh yeah, thanks for mentioning this. I originally thought about generic
disks-uri too, but forgot about it when getting back to the review after
lunch :-/

I wanted to do it that way, but I did not want to be arguing for that.  So
here's what I thought others would use to argue against the URI: If we allow URI
it can then be used for non-socket connections as well, but in that case we
might need different IP address for source and destination, the same reason we
have listenAddress for.  I guess my counter-argument would be that we already
have the listenAddress for that.  That might have been countered by saying that
someone might want to migrate the disk over different data path.  At that point
I might say to forward it yourself using a proxy.  I have few more following
arguments in my head.

I'm very much fine with adding URI instead, but I am not going to be arguing for

Just beware the migration URI is basically passed through to QEMU, while
we use nbd or nbd+unix URIs for disks when talking to QEMU and we should
not use these in the API. Just tcp:// or unix:// and translate them to
the correct QEMU nbd URI internally.

yeah, well, qemu should accept both nbd+unix and unix: I think.  I'll see what I
can do.


Attachment: signature.asc
Description: PGP signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]