10:24 a.m.
On 10/13/2011 04:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange(a)redhat.com>
I finally got fed up of typing URIs when using virsh....
This adds support for a libvirt client configuration file
either /etc/libvirt/libvirt.conf for privileged clients,
or $HOME/.libvirt/libvirt.conf for unprivileged clients.
Cool idea!
Potential problem - our testsuite uses -c test:///default (or similar);
there's a case where we _don't_ want to use alias lookup. But I guess
if valid alias names cannot contain ':', then the presence of ':' in the
target name is sufficient to prove that we don't want to use aliases.
[This is a review as I go reply, so I'll see what the code actually does...]
+<h2>
+<a name="URI_config">Configuring URI aliases</a>
+</h2>
+
+<p>
+To simplify live for administrators, it is possible to setup URI aliases in a
s/live/life/
+libvirt client configuration file. The configuration file
is<code>/etc/libvirt/libvirt.conf</code>
+for the root user, or<code>$HOME/.libvirt/libvirt.conf</code> for any
unprivileged user.
Really? Shouldn't it instead be that /etc is for qemu:///system, and
$HOME/.libvirt is for any user, _including root_, for qemu:///session.
+<p>
+ A URI alias should be a string made up from the characters
+<code>a-Z, 0-9, _, -</code>. Following the<code>=</code>
+ can be any libvirt URI string, including arbitrary URI parameters.
+ URI aliases will apply to any application opening a libvirt
+ connection, unless it has explicitly passed
the<code>VIR_CONNECT_NO_ALIASES</code>
+ parameter to<code>virConnectOpenAuth</code>.
Should we document that aliases are not consulted if the non-NULL
connection name includes ':'?
+++ b/include/libvirt/libvirt.h.in
@@ -843,7 +843,8 @@ typedef virNodeMemoryStats *virNodeMemoryStatsPtr;
* Flags when opening a connection to a hypervisor
*/
typedef enum {
- VIR_CONNECT_RO = 1, /* A readonly connection */
+ VIR_CONNECT_RO = (1<< 0), /* A readonly connection */
+ VIR_CONNECT_NO_ALIASES = (1<< 1), /* Don't try to resolve URI aliases */
At first glance, I didn't see a use for this bit: it seems like the
decision to use aliases is unambiguous - if the name contains ':', there
is no alias, and if it lacks ':', then the only way it can succeed is if
an alias lookup is successful. Oh, I see - you use
VIR_CONNECT_NO_ALIASES to force failure rather than attempt an alias
lookup for the case where name has no ':'. Okay, it makes sense.
}
+static char *
+virConnectConfigFile(void)
+{
+ char *path;
+ if (geteuid() == 0) {
+ if (virAsprintf(&path, "%s/libvirt/libvirt.conf",
+ SYSCONFDIR)< 0)
+ goto no_memory;
Hmm, while I asked above whether qemu:///session for root should use
~root/.libvirt, the code matches your documentation. So if you like my
idea, then this needs a tweak (probably pass in a parameter for whether
the connection itself is system vs. session).
+static int
+virConnectOpenFindURIAliasMatch(virConfValuePtr value, const char *alias, char **uri)
+{
+ virConfValuePtr entry;
+ if (value->type != VIR_CONF_LIST) {
+ virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s",
Fair enough - only a coding error would trigger this error.
+ _("Expected a list for
'uri_aliases' config parameter"));
+ return -1;
+ }
+
+ entry = value->list;
+ while (entry) {
+ char *offset;
+ size_t safe;
+
+ if (entry->type != VIR_CONF_STRING) {
+ virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s",
Wouldn't VIR_ERR_CONF_SYNTAX go better here?
+ _("Expected a string for
'uri_aliases' config parameter list entry"));
+ return -1;
+ }
+
+ if (!(offset = strchr(entry->str, '='))) {
+ virLibConnError(VIR_ERR_INTERNAL_ERROR,
and here
+ _("Malformed 'uri_aliases'
config entry '%s', expected 'alias=uri://host/path'"),
+ entry->str);
+ return -1;
+ }
+
+ safe = strspn(entry->str,
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-");
+ if (safe< (offset - entry->str)) {
+ virLibConnError(VIR_ERR_INTERNAL_ERROR,
and here
+static int
+virConnectOpenResolveURIAlias(const char *alias, char **uri)
+{
+ char *config = NULL;
+ int ret = -1;
+ virConfPtr conf = NULL;
+ virConfValuePtr value = NULL;
+
+ *uri = NULL;
+
+ /* Short circuit to avoid doing URI alias resolution
+ * when it clearly isn't an alias */
+ if (strchr(alias, '/') ||
+ strchr(alias, ':'))
+ return 0;
Aha, I was right - no alias lookup on full URIs.
diff --git a/src/libvirt.conf b/src/libvirt.conf
new file mode 100644
index 0000000..ffe8c21
--- /dev/null
+++ b/src/libvirt.conf
@@ -0,0 +1,13 @@
+
Why a leading blank line?
ACK with nits fixed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org