Bug reports from users have shown diagnosing problems with the remote driver are quite tricky, so I've been looking at improving its error messages where possible. There's a number of places where we simply pass in strerror(errno) as the only data in virRaiseError. This doesn't let us narrow down just which call failed - we just get a generic message "Permission denied" or "Connection refused" with no clue what was being tried. This patch adds descriptive error messages whereever possible, fixes a place where we'd report an error to connect to IPv4 even if IPv6 succeeded, and adds some missing translations Before $ virsh --connect qemu:///system libvir: Remote error : Permission denied $ virsh --connect qemu+tcp:///system libvir: Remote error : Connection refused $ virsh --connect qemu+tcp://wibble/system libvir: Remote error : invalid argument in Name or service not known After $ ./src/virsh --connect qemu:///system libvir: Remote error : unable to connect to '/var/run/libvirt/libvirt-sock': Permission denied $ ./src/virsh --connect qemu+tcp:///system libvir: Remote error : unable to connect to 'localhost': Connection refused $ ./src/virsh --connect qemu+tcp://wibble/system libvir: Remote error : unable to resolve hostname 'wibble': Name or service not known Daniel Index: src/remote_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/remote_internal.c,v retrieving revision 1.80 diff -u -r1.80 remote_internal.c --- src/remote_internal.c 26 Jun 2008 09:37:51 -0000 1.80 +++ src/remote_internal.c 19 Aug 2008 10:36:32 -0000 @@ -371,13 +371,12 @@ priv->hostname = strdup (uri->server ? uri->server : "localhost"); - if (!priv->hostname) { - error (NULL, VIR_ERR_NO_MEMORY, _("allocating priv->hostname")); - goto failed; - } + if (!priv->hostname) + goto out_of_memory; if (uri->user) { username = strdup (uri->user); - if (!username) goto out_of_memory; + if (!username) + goto out_of_memory; } /* Get the variables from the query string. @@ -491,12 +490,15 @@ // http://people.redhat.com/drepper/userapi-ipv6.html struct addrinfo *res, *r; struct addrinfo hints; + int saved_errno = EINVAL; memset (&hints, 0, sizeof hints); hints.ai_socktype = SOCK_STREAM; hints.ai_flags = AI_ADDRCONFIG; int e = getaddrinfo (priv->hostname, port, &hints, &res); if (e != 0) { - error (conn, VIR_ERR_INVALID_ARG, gai_strerror (e)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to resolve hostname '%s': %s"), + priv->hostname, gai_strerror (e)); goto failed; } @@ -516,7 +518,7 @@ priv->sock = socket (r->ai_family, SOCK_STREAM, 0); if (priv->sock == -1) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (socket_errno ())); + saved_errno = socket_errno(); continue; } @@ -526,7 +528,7 @@ sizeof no_slow_start); if (connect (priv->sock, r->ai_addr, r->ai_addrlen) == -1) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (socket_errno ())); + saved_errno = socket_errno(); close (priv->sock); continue; } @@ -545,6 +547,9 @@ } freeaddrinfo (res); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to connect to '%s': %s"), + priv->hostname, strerror (saved_errno)); goto failed; tcp_connected: @@ -563,23 +568,23 @@ uid_t uid = getuid(); if (!(pw = getpwuid(uid))) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to lookup user '%d': %s"), + uid, strerror (errno)); goto failed; } if (asprintf (&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, pw->pw_dir) < 0) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); - goto failed; + sockname = NULL; + goto out_of_memory; } } else { if (flags & VIR_DRV_OPEN_REMOTE_RO) sockname = strdup (LIBVIRTD_PRIV_UNIX_SOCKET_RO); else sockname = strdup (LIBVIRTD_PRIV_UNIX_SOCKET); - if (sockname == NULL) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); - goto failed; - } + if (sockname == NULL) + goto out_of_memory; } } @@ -598,7 +603,9 @@ autostart_retry: priv->sock = socket (AF_UNIX, SOCK_STREAM, 0); if (priv->sock == -1) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to create socket %s"), + strerror (errno)); goto failed; } if (connect (priv->sock, (struct sockaddr *) &addr, sizeof addr) == -1) { @@ -620,7 +627,9 @@ goto autostart_retry; } } - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to connect to '%s': %s"), + sockname, strerror (errno)); goto failed; } @@ -634,17 +643,13 @@ if (no_tty) nr_args += 5; /* For -T -o BatchMode=yes -e none */ command = command ? : strdup ("ssh"); - if (command == NULL) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); - goto failed; - } + if (command == NULL) + goto out_of_memory; // Generate the final command argv[] array. // ssh -p $port [-l $username] $hostname $netcat -U $sockname [NULL] - if (VIR_ALLOC_N(cmd_argv, nr_args) < 0) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); - goto failed; - } + if (VIR_ALLOC_N(cmd_argv, nr_args) < 0) + goto out_of_memory; j = 0; cmd_argv[j++] = strdup (command); @@ -667,12 +672,9 @@ cmd_argv[j++] = strdup (sockname ? sockname : LIBVIRTD_PRIV_UNIX_SOCKET); cmd_argv[j++] = 0; assert (j == nr_args); - for (j = 0; j < (nr_args-1); j++) { - if (cmd_argv[j] == NULL) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (ENOMEM)); - goto failed; - } - } + for (j = 0; j < (nr_args-1); j++) + if (cmd_argv[j] == NULL) + goto out_of_memory; } /*FALLTHROUGH*/ @@ -685,28 +687,38 @@ * to faff around with two file descriptors (a la 'pipe(2)'). */ if (socketpair (PF_UNIX, SOCK_STREAM, 0, sv) == -1) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to create socket pair %s"), + strerror (errno)); goto failed; } pid = fork (); if (pid == -1) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("unable to fork external network transport: %s"), + strerror (errno)); goto failed; } else if (pid == 0) { /* Child. */ close (sv[0]); // Connect socket (sv[1]) to stdin/stdout. close (0); - if (dup (sv[1]) == -1) perror ("dup"); + if (dup (sv[1]) == -1) { + perror ("dup"); + _exit(1); + } close (1); - if (dup (sv[1]) == -1) perror ("dup"); + if (dup (sv[1]) == -1) { + perror ("dup"); + _exit(1); + } close (sv[1]); // Run the external process. if (!cmd_argv) { if (VIR_ALLOC_N(cmd_argv, 2) < 0) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); - goto failed; + perror("malloc"); + _exit(1); } cmd_argv[0] = command; cmd_argv[1] = 0; @@ -770,7 +782,7 @@ return retcode; out_of_memory: - error (NULL, VIR_ERR_NO_MEMORY, _("uri params")); + error (conn, VIR_ERR_NO_MEMORY, NULL); failed: /* Close the socket if we failed. */ @@ -882,10 +894,9 @@ { struct stat sb; if (stat(file, &sb) < 0) { - __virRaiseError (conn, NULL, NULL, VIR_FROM_REMOTE, VIR_ERR_RPC, - VIR_ERR_ERROR, LIBVIRT_CACERT, NULL, NULL, 0, 0, - _("Cannot access %s '%s': %s (%d)"), - type, file, strerror(errno), errno); + errorf(conn, VIR_ERR_RPC, + _("Cannot access %s '%s': %s (%d)"), + type, file, strerror(errno), errno); return -1; } return 0; @@ -905,7 +916,9 @@ /* X509 stuff */ err = gnutls_certificate_allocate_credentials (&x509_cred); if (err) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to allocate TLS credentials: %s"), + gnutls_strerror (err)); return -1; } @@ -923,7 +936,9 @@ gnutls_certificate_set_x509_trust_file (x509_cred, LIBVIRT_CACERT, GNUTLS_X509_FMT_PEM); if (err < 0) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to load CA certificate: %s"), + gnutls_strerror (err)); return -1; } @@ -936,7 +951,9 @@ LIBVIRT_CLIENTKEY, GNUTLS_X509_FMT_PEM); if (err < 0) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to load private key/certificate: %s"), + gnutls_strerror (err)); return -1; } @@ -963,21 +980,27 @@ */ err = gnutls_init (&session, GNUTLS_CLIENT); if (err) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to initialize TLS client: %s"), + gnutls_strerror (err)); return NULL; } /* Use default priorities */ err = gnutls_set_default_priority (session); if (err) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to set TLS algorithm priority: %s"), + gnutls_strerror (err)); return NULL; } err = gnutls_certificate_type_set_priority (session, cert_type_priority); if (err) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to set certificate priority: %s"), + gnutls_strerror (err)); return NULL; } @@ -985,7 +1008,9 @@ */ err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, x509_cred); if (err) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to set session credentials: %s"), + gnutls_strerror (err)); return NULL; } @@ -998,7 +1023,9 @@ if (err < 0) { if (err == GNUTLS_E_AGAIN || err == GNUTLS_E_INTERRUPTED) goto again; - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (err)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to complete TLS handshake: %s"), + gnutls_strerror (err)); return NULL; } @@ -1018,12 +1045,14 @@ if (len < 0 && len != GNUTLS_E_UNEXPECTED_PACKET_LENGTH) { if (len == GNUTLS_E_AGAIN || len == GNUTLS_E_INTERRUPTED) goto again_2; - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (len)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to complete TLS initialization: %s"), + gnutls_strerror (len)); return NULL; } if (len != 1 || buf[0] != '\1') { error (conn, VIR_ERR_RPC, - _("server verification (of our certificate or IP address) failed\n")); + _("server verification (of our certificate or IP address) failed\n")); return NULL; } @@ -1047,33 +1076,39 @@ time_t now; if ((ret = gnutls_certificate_verify_peers2 (session, &status)) < 0) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (ret)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to verify server certificate: %s"), + gnutls_strerror (ret)); return -1; } if ((now = time(NULL)) == ((time_t)-1)) { - error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + errorf (conn, VIR_ERR_SYSTEM_ERROR, + _("cannot get current time: %s"), + strerror (errno)); return -1; } if (status != 0) { - const char *reason = "Invalid certificate"; + const char *reason = _("Invalid certificate"); if (status & GNUTLS_CERT_INVALID) - reason = "The certificate is not trusted."; + reason = _("The certificate is not trusted."); if (status & GNUTLS_CERT_SIGNER_NOT_FOUND) - reason = "The certificate hasn't got a known issuer."; + reason = _("The certificate hasn't got a known issuer."); if (status & GNUTLS_CERT_REVOKED) - reason = "The certificate has been revoked."; + reason = _("The certificate has been revoked."); #ifndef GNUTLS_1_0_COMPAT if (status & GNUTLS_CERT_INSECURE_ALGORITHM) - reason = "The certificate uses an insecure algorithm"; + reason = _("The certificate uses an insecure algorithm"); #endif - error (conn, VIR_ERR_RPC, reason); + errorf (conn, VIR_ERR_RPC, + _("server certificate failed validation: %s"), + reason); return -1; } @@ -1092,13 +1127,17 @@ ret = gnutls_x509_crt_init (&cert); if (ret < 0) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (ret)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to initialize certificate: %s"), + gnutls_strerror (ret)); return -1; } ret = gnutls_x509_crt_import (cert, &certs[i], GNUTLS_X509_FMT_DER); if (ret < 0) { - error (conn, VIR_ERR_GNUTLS_ERROR, gnutls_strerror (ret)); + errorf (conn, VIR_ERR_GNUTLS_ERROR, + _("unable to import certificate: %s"), + gnutls_strerror (ret)); gnutls_x509_crt_deinit (cert); return -1; } @@ -1117,13 +1156,9 @@ if (i == 0) { if (!gnutls_x509_crt_check_hostname (cert, priv->hostname)) { - __virRaiseError - (conn, NULL, NULL, - VIR_FROM_REMOTE, VIR_ERR_RPC, - VIR_ERR_ERROR, priv->hostname, NULL, NULL, - 0, 0, - _("Certificate's owner does not match the hostname (%s)"), - priv->hostname); + errorf(conn, VIR_ERR_RPC, + _("Certificate's owner does not match the hostname (%s)"), + priv->hostname); gnutls_x509_crt_deinit (cert); return -1; } @@ -4721,8 +4756,9 @@ errmsg = __virErrorMsg (code, errorMessage); __virRaiseError (conn, NULL, NULL, VIR_FROM_REMOTE, - code, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", errmsg); + code, VIR_ERR_ERROR, + errmsg, errorMessage, NULL, -1, -1, + errmsg, errorMessage); } /* For errors generated on the server side and sent back to us. */ -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|