From pemensik@redhat.com Mon Apr 3 11:07:53 2017 X-Scanned-BY: MIMEDefang 2.79 on 10.5.11.11 Dkim-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com CF655C0567A1 MIME-Version: 1.0 In-Reply-To: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD autolearn=ham autolearn_force=no version=3.4.0 X-RT-Interface: API Dmarc-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CF655C0567A1 References: <317307465.487433.1489162746275.JavaMail.zimbra@redhat.com> <960257277.509834.1489165546786.JavaMail.zimbra@redhat.com> <20170310174641.GB92236@isc.org> <1dee09e0-c1a7-3d7f-0c71-9368e04ae1be@redhat.com> Message-ID: content-type: text/plain; charset="utf-8" X-RT-Original-Encoding: utf-8 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pemensik@redhat.com Received: from mx.pao1.isc.org (mx.pao1.isc.org [149.20.64.53]) by bugs.isc.org (Postfix) with ESMTP id DDC3071B5A8 for ; Mon, 3 Apr 2017 11:07:52 +0000 (UTC) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx.pao1.isc.org (Postfix) with ESMTPS id C00D63493AC for ; Mon, 3 Apr 2017 11:07:50 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CF655C0567A1 for ; Mon, 3 Apr 2017 11:07:49 +0000 (UTC) Received: from menpad.brq.redhat.com (menpad.brq.redhat.com [10.34.4.127]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 76C596031C for ; Mon, 3 Apr 2017 11:07:49 +0000 (UTC) Delivered-To: bind9-review@bugs.isc.org User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 Subject: Re: [ISC-Bugs #44853] [PATCH] NZF files cannot be written to different directory Return-Path: X-Original-To: bind9-review@bugs.isc.org Date: Mon, 3 Apr 2017 13:07:47 +0200 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 03 Apr 2017 11:07:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mx.pao1.isc.org To: bind9-review@isc.org Content-Transfer-Encoding: 8bit From: "Petr Menšík" RT-Message-ID: Content-Length: 2094 Hello Evan, We are preparing a new release including bind soon. I would like to help you any way I can. If there is anything I can help you with, please do not hesitate to ask. Let me know when if you have anything to comment or review. Dne 25.3.2017 v 18:51 Evan Hunt via RT napsal(a): > I've reviewed your patch, and made a few changes: > > - reverted the changes to dns_view_addnewzones(), put the allow code back > into view.c. Yes, I thought you would not like that move. I rechecked why did I move it in the first place. It made some sense on bind 9.9, where it allowed me to reuse making hashed file name from different places. It moved to shared function isc_file_sanitize in bind 9.11, making such move unnecessary change. I agree it is not necessary or useful in recent version. > - moved your file_sanitize_legacy() function into view.c as nz_legacy() > - made new-zones-directory a view option instead of options-only I am not sure if that is really required. Every file in that directory is named after single view and has to be as unique as is name of a view. I think it would be more useful if you could override path to NZF/NZD file used by that view. It would allow to configure one file shared between views however, that is not wanted I think. Do you have any use case when would it help to configure it per view? > - added a dns_view_setnewzonedir() function. That's now used by nz_legacy() > to find the right place to put the files Sure, it seems better than keeping temporary allocated path in nz_legacy() > - added documentation of new-zones-directory > - noticed that the existing documentation of NZD and NZF files was rather > poor, so expanded them > - fixed the test to check for NZD instead of NZF when linked with LMDB > > (I want to thank you for writing a test, that was particularly helpful.) > > Since I changed your code a fair bit I'm going to ask one of my colleagues > to review it too, but I expect this will be merged to our master branch (for > 9.12.0) soon. > > Thank you again for the contribution. I am glad I could help. Petr Mensik