On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote: [...]
+++ b/src/remote/remote_driver.c +VIR_ENUM_IMPL(remoteDriverTransport, + REMOTE_DRIVER_TRANSPORT_LAST, + "tls", "unix", "ssh", "libssh2", "ext", "tcp", "libssh");
One line per enum value, please. [...]
@@ -174,10 +192,17 @@ static int remoteSplitURIScheme(virURIPtr uri, + p = *transport; + while (*p) { + *p = c_tolower(*p); + p++; + }
I can't believe we don't have a virString helper for this. Oh well. [...]
- if (STRCASEEQ(transport_str, "tls")) { - transport = trans_tls; [...] - } else if (STRCASEEQ(transport_str, "libssh")) { - transport = trans_libssh; - } else { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("remote_open: transport in URL not recognised " - "(should be tls|unix|ssh|ext|tcp|libssh2|libssh)")); + if ((transport = remoteDriverTransportTypeFromString(transport_str)) < 0) + return VIR_DRV_OPEN_ERROR;
You're no longer calling virReportError() when the user attempts to use an unknown transport. While I don't think hardcoding the list of valid transport in the error message is a good idea, neither is failing without telling the user what they did wrong. Please restore the virReportError() call. [...]
@@ -966,7 +966,7 @@ doRemoteOpen(virConnectPtr conn, VIR_DEBUG("Connecting with transport %d", transport); /* Connect to the remote service. */ switch (transport) {
Idea for a follow-up patch: "transport" could be cast to remoteDriverTransport here, so that the compiler will ensure we're covering all possible values. Anyway, with the VIR_ENUM_IMPL() arguments formatted correctly and some error message reported on unknown transport, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization