[libvirt] [PATCH for 5.10.0] nss: Don't fail on empty files

Michal Privoznik posted 1 patch 4 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/c48dffbb1564cf632f224ce2957afbf4d6dc74ed.1574948428.git.mprivozn@redhat.com
tools/nss/libvirt_nss_leases.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[libvirt] [PATCH for 5.10.0] nss: Don't fail on empty files
Posted by Michal Privoznik 4 years, 4 months ago
Before we rewrote nss plugin so that it doesn't use libvirt's
internal functions it used virLeaseReadCustomLeaseFile() to parse
.status files. After the rewrite it's using read() + yajl_parse()
+ yajl_complete_parse(). There's one catch though,
virLeaseReadCustomLeaseFile() skipped over empty files.

An empty .status file is created when a network is started. This
is because we configure dnsmasq to use our leasehelper. So the
first thing it does it calls it as follows:

  DNSMASQ_INTERFACE=virbr0 /usr/libexec/libvirt_leaseshelper init

which causes the leasehelper to create empty virbr0.status file.
If there is only one libvirt network then that is no problem -
there are no other .status files to parse anyway. But if there
are two or more networks then the first empty .status file causes
whole parsing process and subsequently the whole name lookup
process to fail.

Reported-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/nss/libvirt_nss_leases.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/nss/libvirt_nss_leases.c b/tools/nss/libvirt_nss_leases.c
index e96e260da8..7c431e4d53 100644
--- a/tools/nss/libvirt_nss_leases.c
+++ b/tools/nss/libvirt_nss_leases.c
@@ -379,6 +379,7 @@ findLeases(const char *file,
     };
     yajl_handle parser = NULL;
     char line[1024];
+    ssize_t nreadTotal = 0;
     int rv;
 
     if ((fd = open(file, O_RDONLY)) < 0) {
@@ -398,6 +399,7 @@ findLeases(const char *file,
             goto cleanup;
         if (rv == 0)
             break;
+        nreadTotal += rv;
 
         if (yajl_parse(parser, (const unsigned char *)line, rv)  !=
             yajl_status_ok) {
@@ -409,7 +411,8 @@ findLeases(const char *file,
         }
     }
 
-    if (yajl_complete_parse(parser) != yajl_status_ok) {
+    if (nreadTotal > 0 &&
+        yajl_complete_parse(parser) != yajl_status_ok) {
         ERROR("Parse failed %s",
               yajl_get_error(parser, 1, NULL, 0));
         goto cleanup;
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for 5.10.0] nss: Don't fail on empty files
Posted by Michal Privoznik 4 years, 4 months ago
On 11/28/19 2:40 PM, Michal Privoznik wrote:
> Before we rewrote nss plugin so that it doesn't use libvirt's
> internal functions it used virLeaseReadCustomLeaseFile() to parse
> .status files. After the rewrite it's using read() + yajl_parse()
> + yajl_complete_parse(). There's one catch though,
> virLeaseReadCustomLeaseFile() skipped over empty files.
> 
> An empty .status file is created when a network is started. This
> is because we configure dnsmasq to use our leasehelper. So the
> first thing it does it calls it as follows:
> 
>    DNSMASQ_INTERFACE=virbr0 /usr/libexec/libvirt_leaseshelper init
> 
> which causes the leasehelper to create empty virbr0.status file.
> If there is only one libvirt network then that is no problem -
> there are no other .status files to parse anyway. But if there
> are two or more networks then the first empty .status file causes
> whole parsing process and subsequently the whole name lookup
> process to fail.

Forgot to add:

Fixes: v5.7.0-rc1~343

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for 5.10.0] nss: Don't fail on empty files
Posted by Daniel P. Berrangé 4 years, 4 months ago
On Thu, Nov 28, 2019 at 02:40:55PM +0100, Michal Privoznik wrote:
> Before we rewrote nss plugin so that it doesn't use libvirt's
> internal functions it used virLeaseReadCustomLeaseFile() to parse
> .status files. After the rewrite it's using read() + yajl_parse()
> + yajl_complete_parse(). There's one catch though,
> virLeaseReadCustomLeaseFile() skipped over empty files.
> 
> An empty .status file is created when a network is started. This
> is because we configure dnsmasq to use our leasehelper. So the
> first thing it does it calls it as follows:
> 
>   DNSMASQ_INTERFACE=virbr0 /usr/libexec/libvirt_leaseshelper init
> 
> which causes the leasehelper to create empty virbr0.status file.
> If there is only one libvirt network then that is no problem -
> there are no other .status files to parse anyway. But if there
> are two or more networks then the first empty .status file causes
> whole parsing process and subsequently the whole name lookup
> process to fail.
> 
> Reported-by: Pavel Hrdina <phrdina@redhat.com>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/nss/libvirt_nss_leases.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list