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 <[email protected]> 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.
> 


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to