On Tue, Aug 25, 2020 at 05:35:52PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 06:34:05PM +0200, Jiri Denemark wrote:On Tue, Aug 25, 2020 at 17:12:59 +0100, Daniel P. Berrangé wrote: > On Tue, Aug 25, 2020 at 05:53:32PM +0200, Jiri Denemark wrote: > > On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote: > > > - /* RDMA and multi-fd migration requires QEMU to connect to the destination > > > - * itself. > > > - */ > > > - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) > > > - spec.destType = MIGRATION_DEST_HOST; > > > - else > > > - spec.destType = MIGRATION_DEST_CONNECT_HOST; > > > - spec.dest.host.protocol = uribits->scheme; > > > - spec.dest.host.name = uribits->server; > > > - spec.dest.host.port = uribits->port; > > > + if (STREQ(uribits->scheme, "unix")) { > > > + if (flags & VIR_MIGRATE_TLS) { > > > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > > > + _("Migration over UNIX socket with TLS is not supported")); > > > + return -1; > > > + } > > > + if (flags & VIR_MIGRATE_PARALLEL) { > > > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > > > + _("Parallel migration over UNIX socket is not supported")); > > > + return -1; > > > + } > > > > Is there a reason for not supporting TLS and multi-fd with unix sockets? > > >From libvirt's POV this should be fairly trivial (can be a follow-up > > patch, though): introduce MIGRATION_DEST_SOCKET in addition to > > MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect > > using unix: URI (unless this support was removed from QEMU). > > multi-fd is certainly desirable to support and I don't see any reason > to block that. > > TLS is more problematic, at least if you are using x509 credentials then > you need to tell QEMU what hostname to use for validation. This would > require us to accept a hostname parameter in the URI giving the UNIX > socket.We already support this generally regardless on the transport via VIR_MIGRATE_PARAM_TLS_DESTINATION parameter.Ah yes, I forgot. In that case, I don't see a obvious reason to block TLS.
I have got two reasons, both of which are kinda stupid, so I will remove the check. The reasons are: 1) Due to my limited knowledge I was not sure whether "just allowing it" is enough or whether I need to do something extra (like requiring VIR_MIGRATE_PARAM_TLS_DESTINATION) and testing this already took me enough time and 2) since the mgmt app (or user, if there's ever going to be anyone crazy enough) can (and actually needs to) manage the data path it can add its own encryption, compression etc. as they are in the full control of what happens with the data and they will probably not bother adding anything extra on top of making sure the sockets refer to the same file or simply proxying the data stream (I'm quite sure the mgmt app asking for this won't). I am fine with adding support for something nobody might use as we cannot ever be sure. This, however, changes when we are trying to be completely backward compatible as it could mean we need to support it forever. But if checking the one typed param is enough for TLS and multi-fd should "just work" I'm fine with rewriting the check.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
signature.asc
Description: PGP signature