Re: [PATCH v2 10/10] nss: Introduce libvirt-guest module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux