Re: [Gen-art] Gen-ART Telechat review of draft-ietf-opsawg-vmm-mib-03

Jari Arkko <jari.arkko@piuha.net> Wed, 24 June 2015 21:10 UTC

Return-Path: <jari.arkko@piuha.net>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id DAF881B2E2E; Wed, 24 Jun 2015 14:10:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.91
X-Spam-Level:
X-Spam-Status: No, score=-1.91 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, T_RP_MATCHES_RCVD=-0.01] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WWKqvBL5t4XV; Wed, 24 Jun 2015 14:09:56 -0700 (PDT)
Received: from p130.piuha.net (p130.piuha.net [IPv6:2a00:1d50:2::130]) by ietfa.amsl.com (Postfix) with ESMTP id E9A061B2E31; Wed, 24 Jun 2015 14:09:55 -0700 (PDT)
Received: from localhost (localhost [127.0.0.1]) by p130.piuha.net (Postfix) with ESMTP id 639442CEE4; Thu, 25 Jun 2015 00:09:54 +0300 (EEST) (envelope-from jari.arkko@piuha.net)
X-Virus-Scanned: amavisd-new at piuha.net
Received: from p130.piuha.net ([127.0.0.1]) by localhost (p130.piuha.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 24HPzOX9eOQb; Thu, 25 Jun 2015 00:09:52 +0300 (EEST)
Received: from [127.0.0.1] (p130.piuha.net [IPv6:2a00:1d50:2::130]) by p130.piuha.net (Postfix) with ESMTP id 962842CC49; Thu, 25 Jun 2015 00:09:50 +0300 (EEST) (envelope-from jari.arkko@piuha.net)
Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\))
Content-Type: multipart/signed; boundary="Apple-Mail=_C2E459FB-2A76-43A2-B18A-3EBAE0635498"; protocol="application/pgp-signature"; micalg="pgp-sha512"
X-Pgp-Agent: GPGMail 2.5
From: Jari Arkko <jari.arkko@piuha.net>
In-Reply-To: <55719BEC.9080905@alum.mit.edu>
Date: Wed, 24 Jun 2015 18:09:48 -0300
Message-Id: <7FE8A0F4-E629-4D51-93D2-50D9B218D32D@piuha.net>
References: <554FA03F.1040707@alum.mit.edu> <99F0B974-FF7F-40B6-952A-B1D094842A67@hongo.wide.ad.jp> <555F30C5.3070408@bogus.com> <D0F6AE88-F9DC-4916-9551-1716C29632E3@hongo.wide.ad.jp> <55672EA8.4010709@alum.mit.edu> <20150528154814.GA272@elstar.local> <55719BEC.9080905@alum.mit.edu>
To: Paul Kyzivat <pkyzivat@alum.mit.edu>
X-Mailer: Apple Mail (2.1878.6)
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/wPowzPf-9Z64txKFhU1RnACuyqI>
Cc: Hirochika Asai <panda@hongo.wide.ad.jp>, General Area Review Team <gen-art@ietf.org>, draft-ietf-opsawg-vmm-mib.all@ietf.org
Subject: Re: [Gen-art] Gen-ART Telechat review of draft-ietf-opsawg-vmm-mib-03
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 24 Jun 2015 21:10:01 -0000

Paul: thank you very much for the review.

Jari

On 05 Jun 2015, at 09:54, Paul Kyzivat <pkyzivat@alum.mit.edu> wrote:

> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at
> < http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
> 
> Please wait for direction from your document shepherd
> or AD before posting a new version of the draft.
> 
> Document: draft-ietf-opsawg-vmm-mib-03
> Reviewer: Paul Kyzivat
> Review Date: 2014-06-05
> IETF LC End Date: 2015-05-18
> IESG Telechat date: 2015-06-11
> 
> Summary: This draft is ready for publication as a proposed standard. All of my previous comments have been addressed in correspondence.
> 
> Major issues:
> 
> None
> 
> Minor issues:
> 
> none
> 
> Nits/editorial comments:
> 
> In the new changes for -03:
> 
> In Section 4 I see one change to use normative language in this section, as I suggested, that seems problematic:
> 
>   An implementation MUST support both, either of, or none
>   of per-VM notifications and bulk notifications.
> 
> This *looks* normative, but with all the "or"s it ends up requiring *nothing*. I suggest replacing "MUST" with "can".
> 
> In Section 6.1:
> 
> There is one occurrence of "SHOULD not" that ought to be "SHOULD NOT".
> 
> There are two occurrences of "in byte" that should be "in bytes".
> 
> In Section 6.2, in the definition of IANAStorageMediaType:
> 
> I wonder why there isn't a category for Flash Drives.
> 
> 
> On 5/28/15 11:48 AM, Juergen Schoenwaelder wrote:
>> And the proper way to state conformance requirements in MIB modules
>> is to use the conformance definitions.
>> 
>> /js
>> 
>> On Thu, May 28, 2015 at 11:05:12AM -0400, Paul Kyzivat wrote:
>>> Hirochika,
>>> 
>>> Thanks for addressing my comments. I have one comment on -03:
>>> 
>>> I see a change to use normative language in this section. But this
>>> particular one seems problematic:
>>> 
>>>    An implementation MUST support both, either of, or none
>>>    of per-VM notifications and bulk notifications.
>>> 
>>> This *looks* normative, but with all the "or"s it ends up requiring
>>> *nothing*. I suggest replacing "MUST" with "can".
>>> 
>>> 	Thanks,
>>> 	Paul
>>> 
>>> On 5/28/15 6:42 AM, Hirochika Asai wrote:
>>>> Hi,
>>>> 
>>>> We have submitted -03 after the revision addressing Dan’s comments.
>>>> 
>>>> Best,
>>>> Hirochika Asai
>>>> 
>>>> 
>>>>> On May 22, 2015, at 10:36 PM, joel jaeggli <joelja@bogus.com> wrote:
>>>>> 
>>>>> Hi, please do submitt 03
>>>>> 
>>>>> Also take a look at Dan Romascanu's
>>>>> 
>>>>> opsdir reivew.
>>>>> 
>>>>> On 5/14/15 1:08 AM, Hirochika Asai wrote:
>>>>>> Dear Paul Kyzivat,
>>>>>> 
>>>>>> 
>>>>>> Thank you for your review.
>>>>>> 
>>>>>> Since the Last call is in process, we do not submit the (current)
>>>>>> revised version but reply with inline comments and the revised version
>>>>>> attached in this mail.
>>>>>> 
>>>>>> 
>>>>>>> * Figure 2: A few things are fuzzy about this figure:
>>>>>>> 
>>>>>>> -- The meaning/purpose of the part above the "====", and its
>>>>>>> relationship to the rest of the diagram, isn't clear to me. Is it a
>>>>>>> legend, explaining the notation for transient vs. finite states?
>>>>>> My bad.  Thank you for your indication.  In that figure, we expect to
>>>>>> explain the notation of two types of boxes and a symbol of “!”.  So
>>>>>> we have modified the figure to explicitly denote they are the legend.
>>>>>> 
>>>>>> NEW:
>>>>>> 
>>>>>> Notation:
>>>>>> 
>>>>>>    +-------------+
>>>>>>    | vmOperState | : Finite state; the first line presents the
>>>>>>    |             |   `vmOperState', and the second line presents a
>>>>>>    +-------------+   notification generated if applicable.
>>>>>> 
>>>>>>    + - - - - - - +
>>>>>>    | vmOperState | : Transient state; first line presents the
>>>>>>    |             |   `vmOperState', and the second line presents a
>>>>>>    + - - - - - - +   notification generated if applicable.
>>>>>> 
>>>>>>    !               : Notification; a text followed by the symbol "!"
>>>>>>                      denotes a notification generated.
>>>>>> 
>>>>>> =====================================================================
>>>>>> 
>>>>>> +--------------+   + - - - - - - - +     +-------------+
>>>>>> |  suspended   |<--|  suspending   |     |   paused    |
>>>>>> | !vmSuspended |   | !vmSuspending |     |  !vmPaused  |
>>>>>> +--------------+   + - - - - - - - +     +-------------+
>>>>>>      |                ^                    ^
>>>>>>      |                |                    |
>>>>>>      v                |                    |
>>>>>> + - - - - - - +   +-------------+<----------+    + - - - - - - -+
>>>>>> |  resuming   |-->|   running   |<-------------->|  migrating   |
>>>>>> | !vmResuming |   |  !vmRunning |                | !vmMigrating |
>>>>>> + - - - - - - +   +-------------+                + - - - - - - -+
>>>>>>                       |      ^                        ^
>>>>>>                       |      |                        |
>>>>>>                       |      +-------------------+    |
>>>>>>                       |                          |    |
>>>>>>                       v                          v    v
>>>>>>                + - - - - - - - - +          +-------------+
>>>>>>                |  shuttingdown   |--------->|  shutdown   |
>>>>>>                | !vmShuttingdown |          | !vmShutdown |
>>>>>>                + - - - - - - - - +          +-------------+
>>>>>>                                                 ^      |
>>>>>>                                                 |      v !vmDeleted
>>>>>> + - - - - - -+   +------------+     + - - - - - - +    (Deleted from
>>>>>> |  blocked   |   |  crashed   |     |  preparing  |     vmTable)
>>>>>> | !vmBlocked |   | !vmCrashed |     |             |
>>>>>> + - - - - - -+   +------------+     + - - - - - - +
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> -- what is the point of the 'preparing' state? There is no way in, and
>>>>>>> the only way out is to shutdown. (I could understand it as a starting
>>>>>>> state if there was a path to running.) While it is described later, in
>>>>>>> this figure it seems to have no purpose.
>>>>>>> -- the 'blocked' and 'crashed' states have no way either in or out.
>>>>>>> Surely there must be some path into these states, and some path out (at
>>>>>>> least to shutdown or deleted.)
>>>>>>> 
>>>>>> 
>>>>>>> I see from the later definitions that arbitrary state transitions can
>>>>>>> be represented. Is Figure 2 intended to normatively constrain the state
>>>>>>> transitions? Or does it only provide an overview of "expected"
>>>>>>> transitions?
>>>>>>> 
>>>>>>> I don't feel I understand the intent sufficiently to suggest changes to
>>>>>>> remedy my confusion.
>>>>>> 
>>>>>> We modified the caption of Figure 2 to denote it is the overview, and
>>>>>> added the explanation that the detailed state transition is summarized
>>>>>> in Appendix A.  Although there is no way from/to blocked and crashed as
>>>>>> well as one way from preparing as you pointed out, we consider adding it
>>>>>> makes the figure complicated.  Thus, thanks to your suggestion, we
>>>>>> changed it to the overview and promotes readers to refer to Appendix A.
>>>>>> 
>>>>>> The caption of Figure 2:
>>>>>> 
>>>>>> OLD:
>>>>>> The state transition of a virtual machine
>>>>>> 
>>>>>> NEW:
>>>>>> The overview of the state transition of a virtual machine
>>>>>> 
>>>>>> The paragraph referring to Figure 2:
>>>>>> 
>>>>>> OLD:
>>>>>> The transition of `vmOperState' by the write access to `vmAdminState'
>>>>>> and the notifications generated by the operational state changes are
>>>>>> summarized in Figure 2.
>>>>>> 
>>>>>> NEW:
>>>>>> The overview of the transition of `vmOperState' by the write access to
>>>>>> `vmAdminState' and the notifications generated by the operational state
>>>>>> changes are illustrated in Figure 2.  The detailed state transition is
>>>>>> summarized in Appendix A.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> * Section 5
>>>>>>> 
>>>>>>> This says "Hypervisors *shall* implement HOST-RESOURCES-MIB." This
>>>>>>> sounds normative. If so, 'shall' should be replaced with MUST.
>>>>>> The MIB module imports HOST-RESOURCES-MIB, so we replace it with MUST.
>>>>>> 
>>>>>> OLD:
>>>>>> HOST-RESOURCES-MIB defines the MIB objects for managing host systems.
>>>>>> Hypervisors shall implement HOST-RESOURCES-MIB.  On systems implementing
>>>>>> HOST-RESOURCES-MIB, the objects of HOST-RESOURCES-MIB indicate resources
>>>>>> of a hypervisor.   Some objects of HOST-RESOURCES-MIB shall also be used
>>>>>> to indicate physical resources through indexes.
>>>>>> 
>>>>>> NEW:
>>>>>> HOST-RESOURCES-MIB defines the MIB objects for managing host systems.
>>>>>> Hypervisors MUST implement HOST-RESOURCES-MIB.  On systems implementing
>>>>>> HOST-RESOURCES-MIB, the objects of HOST-RESOURCES-MIB indicate resources
>>>>>> of a hypervisor.  Some objects of HOST-RESOURCES-MIB are used to
>>>>>> indicate physical resources through indexes.
>>>>>> 
>>>>>> 
>>>>>>> The same issue with 'shall' is present in the 2nd paragraph refering to
>>>>>>> virtual machines.
>>>>>> 
>>>>>>> 
>>>>>>> Also in the 2nd paragraph I can't parse or fully understand the last
>>>>>>> sentence. ("This document defines the objects of these information.")
>>>>>>> Changing 'these' to 'this' would make it grammatical, but still not
>>>>>>> very clear. I guess you mean something like: "This document defines the
>>>>>>> relationship between the objects visible to virtual machine operators
>>>>>>> and those visible to hypervisor operators.”
>>>>>> We figured that this paragraph was not appropriate to be included here,
>>>>>> so removed it.  Note that it explains an operational difference between
>>>>>> this document and HOST-RESOURCES-MIB that may be individually
>>>>>> implemented in systems on "virtual machines”, i.e., guest OS.  No need
>>>>>> to mention it in this document.
>>>>>> 
>>>>>> 
>>>>>>> * Section 8 - Security Considerations:
>>>>>>> 
>>>>>>> I see no 2119 language in this section, but I see language that sounds
>>>>>>> normative to me. E.g., "When SNMPv3 strong security is not used, these
>>>>>>> objects ***should*** have access of read-only, not read-write." If
>>>>>>> these statements are intended to be normative then please use 2119
>>>>>>> language.
>>>>>> We change *should* to *SHOULD* as 2119 language.  Also, we capitalize
>>>>>> 2119 language in this document.
>>>>>> As for RFC 2119 language, we modified several words to more appropriate
>>>>>> ones in the new version.
>>>>>> 
>>>>>> 
>>>>>>> The rest leaves me concerned about security. But I will leave it to a
>>>>>>> security review to dig into.
>>>>>>> 
>>>>>>> Nits/editorial comments:
>>>>>>> 
>>>>>>> * The introduction says that this has been derived from "enterprise
>>>>>>> specific" MIB modules. But the examples sound more "product-specific"
>>>>>>> than enterprise-specific. I guess you mean modules created by the
>>>>>>> enterprise producing the product, so maybe it is ok, but it struck me
>>>>>>> as odd. (Please feel free to leave this as-is if the usage is
>>>>>>> appropriate in context.)
>>>>>> I agree that the examples except for VMware are product-specific than
>>>>>> enterprise specific.
>>>>>> 
>>>>>> OLD:
>>>>>> The design of this MIB module has been derived from enterprise specific
>>>>>> MIB modules, namely a MIB module for managing guests of the Xen
>>>>>> hypervisor, a MIB module for managing virtual machines controlled by the
>>>>>> VMware hypervisor, and a MIB module using the libvirt programming
>>>>>> interface to access different hypervisors.
>>>>>> 
>>>>>> NEW:
>>>>>> The design of this MIB module has been derived from product-specific MIB
>>>>>> modules, namely a MIB module for managing guests of the Xen hypervisor,
>>>>>> a MIB module for managing virtual machines controlled by the VMware
>>>>>> hypervisor, and a MIB module using the libvirt programming interface to
>>>>>> access different hypervisors.
>>>>>> 
>>>>>> 
>>>>>>> * Page 22, DESCRIPTION of vmHvSoftware:
>>>>>>> 
>>>>>>> This says "This value should not include its version, and it should be
>>>>>>> included in `vmHvVersion'." IIUC 'and' is the wrong word to use here -
>>>>>>> 'as' would convey the intended meaning.
>>>>>> 
>>>>>> Thank you.  ‘as’ is what we intended.
>>>>>> 
>>>>>> 
>>>>>> Best regards,
>>>>>> Hirochika Asai
>>>>>> 
>>>>>> 
>>>>>>> On May 11, 2015, at 3:15 AM, Paul Kyzivat <pkyzivat@alum.mit.edu> wrote:
>>>>>>> 
>>>>>>> I am the assigned Gen-ART reviewer for this draft. For background on
>>>>>>> Gen-ART, please see the FAQ at
>>>>>>> 
>>>>>>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>>>>>>> 
>>>>>>> Please resolve these comments along with any other Last Call comments
>>>>>>> you may receive.
>>>>>>> 
>>>>>>> Document: draft-ietf-opsawg-vmm-mib-02
>>>>>>> Reviewer: Paul Kyzivat
>>>>>>> Review Date:
>>>>>>> IETF LC End Date: 2015-05-18
>>>>>>> IESG Telechat date: (if known)
>>>>>>> 
>>>>>>> Summary: Ready with minor issues.
>>>>>>> 
>>>>>>> Major issues:
>>>>>>> 
>>>>>>> None.
>>>>>>> 
>>>>>>> Minor issues:
>>>>>>> 
>>>>>>> * Figure 2: A few things are fuzzy about this figure:
>>>>>>> 
>>>>>>> -- The meaning/purpose of the part above the "====", and its
>>>>>>> relationship to the rest of the diagram, isn't clear to me. Is it a
>>>>>>> legend, explaining the notation for transient vs. finite states?
>>>>>>> 
>>>>>>> -- what is the point of the 'preparing' state? There is no way in, and
>>>>>>> the only way out is to shutdown. (I could understand it as a starting
>>>>>>> state if there was a path to running.) While it is described later, in
>>>>>>> this figure it seems to have no purpose.
>>>>>>> 
>>>>>>> -- the 'blocked' and 'crashed' states have no way either in or out.
>>>>>>> Surely there must be some path into these states, and some path out (at
>>>>>>> least to shutdown or deleted.)
>>>>>>> 
>>>>>>> I see from the later definitions that arbitrary state transitions can
>>>>>>> be represented. Is Figure 2 intended to normatively constrain the state
>>>>>>> transitions? Or does it only provide an overview of "expected"
>>>>>>> transitions?
>>>>>>> 
>>>>>>> I don't feel I understand the intent sufficiently to suggest changes to
>>>>>>> remedy my confusion.
>>>>>>> 
>>>>>>> * Section 5
>>>>>>> 
>>>>>>> This says "Hypervisors *shall* implement HOST-RESOURCES-MIB." This
>>>>>>> sounds normative. If so, 'shall' should be replaced with MUST.
>>>>>>> 
>>>>>>> The same issue with 'shall' is present in the 2nd paragraph refering to
>>>>>>> virtual machines.
>>>>>>> 
>>>>>>> Also in the 2nd paragraph I can't parse or fully understand the last
>>>>>>> sentence. ("This document defines the objects of these information.")
>>>>>>> Changing 'these' to 'this' would make it grammatical, but still not
>>>>>>> very clear. I guess you mean something like: "This document defines the
>>>>>>> relationship between the objects visible to virtual machine operators
>>>>>>> and those visible to hypervisor operators."
>>>>>>> 
>>>>>>> * Section 8 - Security Considerations:
>>>>>>> 
>>>>>>> I see no 2119 language in this section, but I see language that sounds
>>>>>>> normative to me. E.g., "When SNMPv3 strong security is not used, these
>>>>>>> objects ***should*** have access of read-only, not read-write." If
>>>>>>> these statements are intended to be normative then please use 2119
>>>>>>> language.
>>>>>>> 
>>>>>>> The rest leaves me concerned about security. But I will leave it to a
>>>>>>> security review to dig into.
>>>>>>> 
>>>>>>> Nits/editorial comments:
>>>>>>> 
>>>>>>> * The introduction says that this has been derived from "enterprise
>>>>>>> specific" MIB modules. But the examples sound more "product-specific"
>>>>>>> than enterprise-specific. I guess you mean modules created by the
>>>>>>> enterprise producing the product, so maybe it is ok, but it struck me
>>>>>>> as odd. (Please feel free to leave this as-is if the usage is
>>>>>>> appropriate in context.)
>>>>>>> 
>>>>>>> * Page 22, DESCRIPTION of vmHvSoftware:
>>>>>>> 
>>>>>>> This says "This value should not include its version, and it should be
>>>>>>> included in `vmHvVersion'." IIUC 'and' is the wrong word to use here -
>>>>>>> 'as' would convey the intended meaning.
>>>>>> 
>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art