Re: [libvirt] [PATCH] virNetworkDefUpdateDNSHost: Require both IP and a hostname to match
On Wed, Nov 07, 2018 at 11:17:51AM -0500, Laine Stump wrote: > On 11/6/18 12:10 PM, W. Trevor King wrote: > > But HOST records can have the same hostname for multiple records > > (similar to TXT records with the same value). The value that > > needs to be distinct for HOST records is the IP address. > > You're going to force me to go dig out the decades-old DNS RFCs, > aren't you? The relevant RFCs are probably [1,2]. But as far as they're concened IP <-> domain-name mappings are many to many. That doesn't really impact libvirt's entries. For example: IP hostname 192.168.1.1 alice 192.168.1.1 bob 192.168.1.2 alice could be represented with unique IPs: alice bob alice or with repeated IPs: alice bob alice Depending on how generous you were feeling, you could also support overlaps: alice bob bob alice I know libvirt supports the unique-IP form. I haven't looked to see whether it supports the repeated IP form (with or without overlaps). > (but in the meantime, if something was working before (e.g. removing > an entry by hostname alone) then that needs to continue working, > otherwise some application depending on that behavior will now be > broken, and we've made promises about that not happening with > libvirt. I think continuing to support the IP/hostname-union logic makes things too complicated. Would you have to detect the "we match too many things and would ordinarily error out" case so you could switch on the "they must expect this to be an IP/hostname-intersection match" logic? How would you feel about a v2 function (a la dup2), which callers switch on with VIR_NETWORK_SECTION_DNS_HOST2 or something to opt in to the new logic? This would also give you a way to notice the old section variable and log deprecation notices (does the libary have hooks for logging?) to help get the word out, and help set the stage for possibly removing the old logic after a few years. > > 1. You can now delete entries from an existing network like: > > > > > > > > example > > > > > > example > > > > > > > > with input like: > > > > > > > > > > or: > > > > > > example > > > > > > Previously, only the former would work (the latter used to raise > > "multiple matching DNS HOST records were found in network"). > > > Without thinking too much about it, that (the "multiple matching > " error) sounds like a bug that can/should be fixed - since > both hostname and ip find only a single record, there shouldn't be > an error. Both elements in the original network are "example", so they are not unique. The current logic's IP/hostname-union logic matches both, and errors out. I don't think this is a bug we can fix without something ugly like "if IP/hostname-union gives multiple matches, implicitly switch to IP/hostname-intersection" or a way to explicitly select the more-specific logic. > > 3. You can now add multiple entries with a common hostname (as long as > >they have distinct IP addresses). Previously, adding: > > > > > >example > > > > > >to an existing: > > > > > >example > > > > > >would have raised "there is already at least one DNS HOST record > >with a matching field in network". > > --- > > > > I'm actually not clear on whether the 'ip' attribute is required > > to be unique or not. > > Well, *something* needs to be unique. Either one of the fields, or > the combination of some fields. If possible, decisions about that > should be based on the RFCs, and then if the backend implementation > (dnsmasq in this case) is any different, that should be treated as a > different kind of error. We've had no trouble creating networks where multiple IP share a common hostname in libvirt. The only problem I'm aware of (and the reason for this patch) is that libvirt doesn't allow you to *update* those networks. You currently have to delete them and then re-create them, after which you need to re-attach your domains to the new network. Cheers, Trevor [1]: https://tools.ietf.org/html/rfc1035 [2]: https://tools.ietf.org/html/rfc1794 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetworkDefUpdateDNSHost: Only require IP to match
I've filed [1] to track this in Bugzilla. Cheers, Trevor [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1647211 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virNetworkDefUpdateDNSHost: Only require IP to match
On Tue, Nov 06, 2018 at 09:10:39AM -0800, W. Trevor King wrote: > This commit update the matching logic to only consider the IP > address. Oops, I should have updated the subject to: virNetworkDefUpdateDNSHost: Only require IP to match Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH go] version: Add ParseVersion and a Version struct
Anything I can do to help this along? I've adjusted the subject to have "PATCH go" instead of just "PATCH"; sorry about that. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virNetworkDefUpdateDNSHost: Require both IP and a hostname to match
Since fc19a0059 (network: backend functions for updating network dns
host/srv/txt, 2012-11-12), the matching logic for various network
components has been:
1) for HOST records, it's considered a match if the IP address or any
of the hostnames of an existing record matches.
2) for SRV records, it's a match if all of
domain+service+protocol+target *which have been specified* are
matched.
3) for TXT records, there is only a single field to match - name
(value can be the same for multiple records, and isn't considered a
search term), so by definition there can be no ambiguous matches.
But HOST records can have the same hostname for multiple records
(similar to TXT records with the same value). The value that needs to
be distinct for HOST records is the IP address. This commit updates
the matching logic to only consider the IP address. Compared to the
previous HOST logic:
1. You can now delete entries from an existing network like:
example
example
with input like:
or:
example
Previously, only the former would work (the latter used to raise
"multiple matching DNS HOST records were found in network").
2. You can no longer remove entries by hostname alone. Previously,
you may have been able to remove an entry from an existing network
like:
example-1
example-2
with input like:
example-1
using the 'name' property to get through the partialOkay check in
virNetworkDHCPHostDefParseXML. Now that input will raise "Missing
IP address in network '%s' DNS HOST record".
3. You can now add multiple entries with a common hostname (as long as
they have distinct IP addresses). Previously, adding:
example
to an existing:
example
would have raised "there is already at least one DNS HOST record
with a matching field in network".
---
I'm actually not clear on whether the 'ip' attribute is required to be
unique or not. If not, maybe the logic should be:
* Deletes with just an IP remove all entries that match that
IP.
* Deletes with just a hostname remove all entries that
match that hostname.
* Deletes with an IP and a hostname remove matching entries
from entries which match the IP.
* If removal completely empties a , the is
also removed.
Thoughts?
src/conf/network_conf.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 39a13b4..8ed62ac 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -587,14 +587,14 @@ virNetworkDNSHostDefParseXML(const char *networkName,
xmlNodePtr cur;
char *ip;
-if (!(ip = virXMLPropString(node, "ip")) && !partialOkay) {
+if (!(ip = virXMLPropString(node, "ip"))) {
virReportError(VIR_ERR_XML_DETAIL,
_("Missing IP address in network '%s' DNS HOST record"),
networkName);
goto error;
}
-if (ip && (virSocketAddrParse(&def->ip, ip, AF_UNSPEC) < 0)) {
+if (virSocketAddrParse(&def->ip, ip, AF_UNSPEC) < 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("Invalid IP address in network '%s' DNS HOST record"),
networkName);
@@ -603,6 +603,13 @@ virNetworkDNSHostDefParseXML(const char *networkName,
}
VIR_FREE(ip);
+if (!VIR_SOCKET_ADDR_VALID(&def->ip)) {
+virReportError(VIR_ERR_XML_DETAIL,
+ _("Invalid IP address in network '%s' DNS HOST record"),
+ networkName);
+goto error;
+}
+
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE &&
@@ -631,13 +638,6 @@ virNetworkDNSHostDefParseXML(const char *networkName,
goto error;
}
-if (!VIR_SOCKET_ADDR_VALID(&def->ip) && def->nnames == 0) {
-virReportError(VIR_ERR_XML_DETAIL,
- _("Missing ip and hostname in network '%s' DNS HOST
record"),
- networkName);
-goto error;
-}
-
return 0;
error:
@@ -3334,18 +3334,7 @@ virNetworkDefUpdateDNSHost(virNetworkDefPtr def,
goto cleanup;
for (i = 0; i < dns->nhosts; i++) {
-bool foundThisTime = false;
-
-if (virSocketAddrEqual(&host.ip, &dns->hosts[i].ip))
-foundThisTime = true;
-
-for (j = 0; j < host.nnames && !foundThisTime; j++) {
-for (k = 0; k < dns->hosts[i].nnames && !foundThisTime; k++) {
-if (STREQ(host.names[j], dns->hosts[i].names[k]))
-foundThisTime = true;
-}
-}
-if (foundThisTime) {
+if virSocketAddrEqual(&host.ip, &dns->hosts[i].ip) {
foundCt++;
foundIdx = i;
}
[libvirt] [PATCH] version: Add ParseVersion and a Version struct
Make it easier to convert version integers to the more human-readable
major.minor.release format.
---
connect.go | 8
version.go | 52 ++
version_test.go | 64 +
3 files changed, 124 insertions(+)
create mode 100644 version.go
create mode 100644 version_test.go
diff --git a/connect.go b/connect.go
index 8cc7cc7..fac45da 100644
--- a/connect.go
+++ b/connect.go
@@ -46,6 +46,7 @@ func init() {
}
const (
+ // VERSION_NUMBER is the version of the linked libvirt.
VERSION_NUMBER = uint32(C.LIBVIR_VERSION_NUMBER)
)
@@ -306,7 +307,9 @@ func releaseConnectionData(c *Connect) {
delete(connections, c.ptr)
}
+// GetVersion returns the version of the linked libvirt.
// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+// and ParseVersion.
func GetVersion() (uint32, error) {
var version C.ulong
var err C.virError
@@ -540,7 +543,10 @@ func (c *Connect) GetHostname() (string, error) {
return hostname, nil
}
+// GetLibVersion returns the version of libvirt used by the daemon
+// running on the connected host.
// See also
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetLibVersion
+// and ParseVersion.
func (c *Connect) GetLibVersion() (uint32, error) {
var version C.ulong
var err C.virError
@@ -2272,7 +2278,9 @@ func (c *Connect) GetDomainCapabilities(emulatorbin
string, arch string, machine
return C.GoString(ret), nil
}
+// GetVersion returns the hypervisor version.
// See also
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetVersion
+// and ParseVersion.
func (c *Connect) GetVersion() (uint32, error) {
var hvVer C.ulong
var err C.virError
diff --git a/version.go b/version.go
new file mode 100644
index 000..1a07fa3
--- /dev/null
+++ b/version.go
@@ -0,0 +1,52 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ */
+
+package libvirt
+
+import (
+ "fmt"
+)
+
+// Version represents the libvirt version.
+// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+type Version struct {
+ Major uint32
+ Minor uint32
+ Release uint32
+}
+
+// String returns the "major.minor.release" version string.
+func (v *Version) String() string {
+ return fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Release)
+}
+
+// ParseVersion converts a version integer to a structured Version instance.
+// See also https://libvirt.org/html/libvirt-libvirt-host.html#virGetVersion
+func ParseVersion(version uint32) *Version {
+ return &Version{
+ Major: version / 100,
+ Minor: (version / 1000) % 1000,
+ Release: version % 1000,
+ }
+}
diff --git a/version_test.go b/version_test.go
new file mode 100644
index 000..ecafe5d
--- /dev/null
+++ b/version_test.go
@@ -0,0 +1,64 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
