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

[Martin Kletzander]

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 at 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
either.

>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.

>Jirka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200826/1e4793ba/attachment-0001.sig>