On 05.12.2016 17:25, Martin Kletzander wrote:
> On Mon, Dec 05, 2016 at 11:31:56AM +0100, Michal Privoznik wrote:
>> So far the NSS module looks up only hostnames as provided by
>> guests themselves. However, there are some cases where this is
>> not enough: e.g. when there's a fresh new guest being installed
>> (with some generic hostname) say from a live ISO image; or some
>> (older) systems don't advertise their hostname in DHCP
>> transactions at all.
>> In cases like that it would be helpful if we translate domain
>> name as seen by libvirt too so that users can:
>>
>> # virsh start $dom && ssh $dom
>>
>> In order to achieve that new libvirt-guest module is introduced,
>> while older libvirt module maintains its current behaviour (that
>> is translating guest provided names into IP addresses).
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> docs/news.html.in | 4 ++
>> docs/nss.html.in | 58 ++++++++++++++++++++++---
>> src/Makefile.am | 8 ++++
>> tests/Makefile.am | 22 +++++++++-
>> tests/nssdata/virbr0.macs | 23 ++++++++++
>> tests/nssdata/virbr0.status | 5 +++
>> tests/nssdata/virbr1.macs | 21 +++++++++
>> tests/nssdata/virbr1.status | 5 +++
>> tests/nssmock.c | 21 +++++++++
>> tests/nsstest.c | 5 +++
>> tools/Makefile.am | 46 ++++++++++++++++++--
>> tools/nss/libvirt_guest_nss.syms | 12 ++++++
>> tools/nss/libvirt_nss.c | 92
>> ++++++++++++++++++++++++++++++++++------
>> tools/nss/libvirt_nss.h | 6 ++-
>> 14 files changed, 305 insertions(+), 23 deletions(-)
>> create mode 100644 tests/nssdata/virbr0.macs
>> create mode 100644 tests/nssdata/virbr1.macs
>> create mode 100644 tools/nss/libvirt_guest_nss.syms
>>
>> diff --git a/docs/news.html.in b/docs/news.html.in
>> index 12dcb0141..2ea409894 100644
>> --- a/docs/news.html.in
>> +++ b/docs/news.html.in
>> @@ -15,6 +15,10 @@
>> <h3>v3.0.0 (<i>unreleased</i>)</h3>
>> <ul>
>> <li><strong>New features</strong>
>> + <ul>
>> + <li>New <code>libvirt-guest</code> nss module that
>> translates libvirt
>> + guest names into IP addresses</li>
>
> It would be nice to have the "summary<br/>description" here. Also
> you're missing newline before </li>.
>
>> + </ul>
>> </li>
>> <li><strong>Improvements</strong>
>> </li>
>> diff --git a/docs/nss.html.in b/docs/nss.html.in
>> index 84ea8df6d..ee18c9b78 100644
>> --- a/docs/nss.html.in
>> +++ b/docs/nss.html.in
>> @@ -62,6 +62,48 @@ hosts: files libvirt dns
>> lookup given host name.
>> </p>
>>
>> + <h2><a name="Sources">Sources of information</a></h2>
>> +
>> + <p>
>> + As of <code>v3.0.0</code> release, libvirt offers two NSS modules
>> + implementing two different methods of hostname translation. The
>> first and
>> + older method is implemented by <code>libvirt</code> plugin and
>> + basically looks up the hostname to IP address translation in DHCP
>> server
>> + records. Therefore this is dependent on hostname provided by
>> guests. Thing
>> + is, not all the guests out there provide one in DHCP
>> transactions, nor
>
> I am being said (often) that "nor" can only be combined with "neither"
> and not even always (because I do that mistake often as well) and that
> "or" should be used in this context instead. Native speaker should
> probably correct me or you.
>
>> + every sysadmin out there believes all the guests. Hence libvirt
>> implements
>> + second method in <code>libvirt-guest</code> module which does
>> libvirt guest
>> + name to IP address translation (regardless of hostname set in the
>> guest).
>> + </p>
>> +
>> + <p>
>> + To enable either of the modules put their name into the
>> + <code>nsswitch.conf</code> file. For instance, to enable
>> + <code>libvirt-guest</code> module:
>> + </p>
>> +
>> +<pre>
>> +$ cat /etc/nsswitch.conf
>> +# /etc/nsswitch.conf:
>> +hosts: files libvirt-guest dns
>> +# ...
>> +</pre>
>> + <p>Or users can enable both at the same time:</p>
>> +<pre>
>> +$ cat /etc/nsswitch.conf
>> +# /etc/nsswitch.conf:
>> +hosts: files libvirt libvirt-guest dns
>> +# ...
>> +</pre>
>> +
>> + <p>
>> + This configuration will mean that if hostname is not found by the
>> + <code>libvirt</code> module (e.g. because a guest did not sent
>> hostname
>> + during DHCP transaction), the <code>libvirt-guest</code> module is
>> + consulted (and if the hostname matches libvirt guest name it will be
>> + resolved).
>> + </p>
>> +
>> <h2><a name="Internals">How does it work?</a></h2>
>>
>> <p>
>> @@ -100,15 +142,18 @@ hosts: files libvirt dns
>> <h2><a name="Limitations">Limitations</a></h2>
>>
>> <ol>
>> - <li>The libvirt NSS module matches only hostnames provided by
>> guest. If
>> - the libvirt name and one advertised by guest differs, the
>> latter is
>> - matched.</li>
>> + <li><del>The libvirt NSS module matches only hostnames provided
>> by guest.
>> + If the libvirt name and one advertised by guest differs, the
>> latter is
>> + matched.</del> As of <code>v3.0.0</code> there are two
>> libvirt NSS modules
>> + translating both hostnames provided by guest and libvirt
>> guest names.</li>
>
> Is <del/> just strike-through text? What's the point in that? Just
> make the documentation reflect the newest, current, state.
>
>> <li>The module works only in that cases where IP addresses are
>> assigned by
>> dnsmasq spawned by libvirt. Libvirt NATed networks are typical
>> example.</li>
>> </ol>
>>
>> <p>
>> + <i>The following paragraph describes implementation limitation of
>> the
>> + <code>libvirt</code> NSS module.</i>
>> These limitation are result of libvirt's internal implementation.
>> While
>> libvirt can report IP addresses regardless of their origin, a
>> public API
>> must be used to obtain those. However, for the API a connection
>> object is
>> @@ -134,8 +179,11 @@ virsh domifaddr --source lease $domain
>> </pre>
>>
>> <p>
>> - If there's no record for either of the aforementioned commands,
>> it's very
>> - likely that NSS module won't find anything and vice versa.
>> + <del>If there's no record for either of the aforementioned
>> commands, it's
>> + very likely that NSS module won't find anything and vice
>> versa.</del>
>> + As of <code>v3.0.0</code> libvirt provides
>> <code>libvirt-guest</code> NSS
>> + module that doesn't have this limitation. However, the statement
>> is still
>> + true for the <code>libvirt</code> NSS module.
>
> Same here.
>
>> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
>> index 979bf81e1..6cadf8c43 100644
>> --- a/tools/nss/libvirt_nss.c
>> +++ b/tools/nss/libvirt_nss.c
>> @@ -267,6 +284,54 @@ findLease(const char *name,
>> goto cleanup;
>> }
>>
>> +#else /* defined(LIBVIRT_NSS_GUEST) */
>> +
>> + for (i = 0; i < nMacmaps; i++) {
>> + const char **macs = (const char **)
>> virMACMapMgrLookup(macmaps[i], name);
>> + size_t j;
>> +
>> + if (!macs)
>> + continue;
>> +
>> + for (j = 0; j < nleases; j++) {
>> + virJSONValuePtr lease =
>> virJSONValueArrayGet(leases_array, j);
>> + const char *macAddr;
>> +
>> + if (!lease) {
>> + /* This should never happen (TM) */
>> + ERROR("Unable to get element %zd of %zd", j, nleases);
>> + goto cleanup;
>> + }
>> +
>> + macAddr = virJSONValueObjectGetString(lease, "mac-address");
>> + if (!macAddr)
>> + continue;
>> +
>> + if (!virStringListHasString(macs, macAddr))
>> + continue;
>> +
>> + if (virJSONValueObjectGetNumberLong(lease, "expiry-time",
>> &expirytime) < 0) {
>> + /* A lease cannot be present without expiry-time */
>> + ERROR("expiry-time field missing for %s", name);
>> + goto cleanup;
>> + }
>> +
>> + /* Do not report expired lease */
>> + if (expirytime < (long long) currtime) {
>> + DEBUG("Skipping expired lease for %s", name);
>> + continue;
>> + }
>> +
>> + DEBUG("Found record for %s", name);
>> + *found = true;
>> +
>> + if (appendAddr(&tmpAddress, &ntmpAddress, lease, af) < 0)
>> + goto cleanup;
>> + }
>> + }
>> +
>> +#endif /* defined(LIBVIRT_NSS_GUEST) */
>> +
>
> I feel like lot of this code is duplicated, but that can be fixed later.
> Also it might look more complicated and this is the best way to keep it.
> Did you try any other approach by any chance (just out of curiosity).
Hm.. good point. Let me see if I can fix that.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
