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

Paul Kyzivat <pkyzivat@alum.mit.edu> Thu, 14 May 2015 16:24 UTC

Return-Path: <pkyzivat@alum.mit.edu>
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 293C61A884E for <gen-art@ietfa.amsl.com>; Thu, 14 May 2015 09:24:07 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.235
X-Spam-Level:
X-Spam-Status: No, score=-1.235 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, SPF_SOFTFAIL=0.665] autolearn=no
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 kSIbk0PXg5xs for <gen-art@ietfa.amsl.com>; Thu, 14 May 2015 09:24:04 -0700 (PDT)
Received: from resqmta-ch2-10v.sys.comcast.net (resqmta-ch2-10v.sys.comcast.net [IPv6:2001:558:fe21:29:69:252:207:42]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 57C211A8855 for <gen-art@ietf.org>; Thu, 14 May 2015 09:24:01 -0700 (PDT)
Received: from resomta-ch2-06v.sys.comcast.net ([69.252.207.102]) by resqmta-ch2-10v.sys.comcast.net with comcast id TsNe1q0012D5gil01sQ0sA; Thu, 14 May 2015 16:24:00 +0000
Received: from Paul-Kyzivats-MacBook-Pro.local ([50.138.229.151]) by resomta-ch2-06v.sys.comcast.net with comcast id TsPz1q00L3Ge9ey01sPz5d; Thu, 14 May 2015 16:24:00 +0000
Message-ID: <5554CC1E.1050304@alum.mit.edu>
Date: Thu, 14 May 2015 12:23:58 -0400
From: Paul Kyzivat <pkyzivat@alum.mit.edu>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: Hirochika Asai <panda@hongo.wide.ad.jp>
References: <554FA03F.1040707@alum.mit.edu> <99F0B974-FF7F-40B6-952A-B1D094842A67@hongo.wide.ad.jp>
In-Reply-To: <99F0B974-FF7F-40B6-952A-B1D094842A67@hongo.wide.ad.jp>
Content-Type: text/plain; charset="windows-1252"; format="flowed"
Content-Transfer-Encoding: 8bit
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1431620640; bh=RSw2aQ5WfgRL07mKYqVmF51at/5Z0bNSUBm4ca21Bo8=; h=Received:Received:Message-ID:Date:From:MIME-Version:To:Subject: Content-Type; b=tR76gDy1CfMwTFwdqtJ+jqKn5Mi5N3D3WT/bmx4wM0O7m3V+7CxzVRMN7FrD+JCVd /8CU0aZ+miBbqSah8tqVZnd5ud2yROeO4QG33EHdVSd7ats8znFQ2sB/u1n4rIW9U+ CJ5KqD/vv8DncQSPuUvaJ4fhjQrAaMGJYDYH2KoaQXa3CehkwBBxsOBYHVkK9R+y9I AOxZ8Pt19HNMzA3OoujGOnVFQaZHA0Ckqqgdss7zOmlT/PXT4EDKycBW1fVw/nRWnM V2L9xlCzK/8A9N1O8Eit2Aw/VTGISECjvX7f577jLFJXQi5/XYvpCQVj0qdOWJ2fQS KqJneI0OaTVrA==
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/ipiCgHZKfJTFV7_IL4Vir8vZpyQ>
Cc: 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
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, 14 May 2015 16:24:07 -0000

Dear Hirochika Asai,

The changes you describe below will resolve my concerns.

	Thanks,
	Paul

On 5/14/15 4: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.
>