Paul: thank you very much for the review. Jari
On 05 Jun 2015, at 09:54, Paul Kyzivat <pkyzi...@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 <joe...@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 <pkyzi...@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
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art