Re: [Gen-art] Gen-ART Last Call review of draft-ietf-opsawg-vmm-mib-02

Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de> Thu, 28 May 2015 15:53 UTC

Return-Path: <j.schoenwaelder@jacobs-university.de>
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 E3C881A9147; Thu, 28 May 2015 08:53:27 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.86
X-Spam-Level:
X-Spam-Status: No, score=-3.86 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HELO_EQ_DE=0.35, RCVD_IN_DNSWL_MED=-2.3, 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 uPSD-jU7Q-yk; Thu, 28 May 2015 08:53:24 -0700 (PDT)
Received: from atlas3.jacobs-university.de (atlas3.jacobs-university.de [212.201.44.18]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A664D1AD352; Thu, 28 May 2015 08:48:24 -0700 (PDT)
Received: from localhost (demetrius5.irc-it.jacobs-university.de [10.70.0.222]) by atlas3.jacobs-university.de (Postfix) with ESMTP id 60196192C; Thu, 28 May 2015 17:48:23 +0200 (CEST)
X-Virus-Scanned: amavisd-new at jacobs-university.de
Received: from atlas3.jacobs-university.de ([10.70.0.220]) by localhost (demetrius5.jacobs-university.de [10.70.0.222]) (amavisd-new, port 10030) with ESMTP id p_vUdolCjJNZ; Thu, 28 May 2015 17:48:11 +0200 (CEST)
Received: from hermes.jacobs-university.de (hermes.jacobs-university.de [212.201.44.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "hermes.jacobs-university.de", Issuer "Jacobs University CA - G01" (verified OK)) by atlas3.jacobs-university.de (Postfix) with ESMTPS; Thu, 28 May 2015 17:48:20 +0200 (CEST)
Received: from localhost (demetrius1.jacobs-university.de [212.201.44.46]) by hermes.jacobs-university.de (Postfix) with ESMTP id 8BC2920037; Thu, 28 May 2015 17:48:20 +0200 (CEST)
X-Virus-Scanned: amavisd-new at jacobs-university.de
Received: from hermes.jacobs-university.de ([212.201.44.23]) by localhost (demetrius1.jacobs-university.de [212.201.44.32]) (amavisd-new, port 10024) with ESMTP id tNJ0t-Lf6EP7; Thu, 28 May 2015 17:48:16 +0200 (CEST)
Received: from elstar.local (elstar.jacobs.jacobs-university.de [10.50.231.133]) by hermes.jacobs-university.de (Postfix) with ESMTP id 1349420033; Thu, 28 May 2015 17:48:15 +0200 (CEST)
Received: by elstar.local (Postfix, from userid 501) id EED1933AC14F; Thu, 28 May 2015 17:48:14 +0200 (CEST)
Date: Thu, 28 May 2015 17:48:14 +0200
From: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>
To: Paul Kyzivat <pkyzivat@alum.mit.edu>
Message-ID: <20150528154814.GA272@elstar.local>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
X-Clacks-Overhead: GNU Terry Pratchett
Content-Transfer-Encoding: 8bit
In-Reply-To: <55672EA8.4010709@alum.mit.edu>
User-Agent: Mutt/1.4.2.3i
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/WL_OEcHPFkv3z4re1LkCiauxQjw>
Cc: joel jaeggli <joelja@bogus.com>, 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 Last Call review of draft-ietf-opsawg-vmm-mib-02
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
Reply-To: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>
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: <http://www.ietf.org/mail-archive/web/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: Thu, 28 May 2015 15:53:28 -0000

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.
> >>>
> >>
> >>
> >
> 

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>