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

[Juergen Schoenwaelder <[e-mail]>]

Thanks for the review. Here is a proposal to resolve the mentioned
notification issue:

- Remove the following sentence:

    An implementation MUST support both, either of, or none
    of per-VM notifications and bulk notifications.

  Rationale: Compliance statements define whether notifications are
  optional or not (and both compliance statements make them optional,
  so this is sufficient, the sentence does not serve a purpose).

- Change vmPerVMNotificationOptionalGroup to vmPerVMNotificationGroup
  and vmBulkNotificationOptionalGroup to vmBulkNotificationGroup. The
  group name is the wrong place to declare something optional; the
  context in which a group name is referenced defines whether
  something is optional, conditionally mandatory, or mandatory.

/js

On Fri, Jun 05, 2015 at 08:54:04AM -0400, Paul Kyzivat 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 <[e-mail]> 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 <[e-mail]> 
> >>>>>>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: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <http://www.jacobs-university.de/>