I don't know why I'm included in review of this driver, but why not :) it is good to include people.
Thanks for your patch! It looks exciting and complicated, I like that kind of stuff. On Mon, Apr 23, 2018 at 4:46 PM, Bryant G. Ly <bryan...@linux.vnet.ibm.com> wrote: > This driver is a logical device which provides an > interface between the hypervisor and a management > partition. Please indicate what this management partition is to be used for, i.e. the intended use case. > This driver is to be used for the POWER Virtual > Management Channel Virtual Adapter on the PowerVM > platform. Is that virtualization on the PowerPC platform? > It provides both request/response and > async message support through the /dev/ibmvmc node. Is that a character or block device? I guess block, but it's nice to have some specifics in the commit message. > diff --git a/Documentation/misc-devices/ibmvmc.txt > b/Documentation/misc-devices/ibmvmc.txt The document suffers a bit from internal lingo, so it's be great if you improve it a bit with a style that will be welcoming for newcomers who have no idea what you are talking about. I will point out some items below. > +Description > +=========== > + > +The Virtual Management Channel (VMC) is a logical device which provides an > +interface between the hypervisor and a management partition. What kind of interface? A bitstream? Message passing interface? Marshalling of structs? Some specifics would be great. > This > +management partition is intended to provide an alternative to HMC-based > +system management. Please expand the acronym HMC for the newcomer so we do not need to look it up, or it looks like the document is only for people who already know everything about virtualization specifics. > In the management partition, a Novalink application > +exists Is Novalink some kind of product name? In that case, can it be instead described with some neutral technology name and state Novalink as an examplf of this type of technology? > which enables a system administrator to configure the system’s > +partitioning characteristics Exemplify "partitioning characteristics", with a sentence in this vein: "partitioning characteristics such as FOO, BAR and BAZ". > via a command line interface (CLI) or > +Representational State Transfer Application (REST). I understand what CLI is but what is a representational state transfer application? Is that a fancy name for a GUI application doing the same thing as a CLI program would do? > You can also manage the > +server by using PowerVC There was PowerVM now there is PowerVC. What are those things really? PowerVM I guess is virtual machine on PowerPC? What is PowerVC? Virtual client (guest) in the PowerVM? This kind of lingo makes the document a hard read for me. > or other OpenStack solution. I heard about OpenStack. I have a vague idea about what it is, do you mean "other virtualization concepts"? > Support for > +conventional HMC management of the system may still be provided on a > +system; however, when an HMC is attached to the system, the VMC > +interface is disabled by the hypervisor. The VMC acronym is expanded below, instead expand it at first instance. The same for HMC (hypervisor management channel I guess? it is never really explained.) > +NovaLink runs on a Linux logical partition on a POWER8 or newer processor- > +based server that is virtualized by PowerVM. So this information needs to be in the top of the document. > System configuration, > +maintenance, and control functions which traditionally require an HMC can > +be implemented in the Novalink using a combination of HMC to hypervisor > +interfaces and existing operating system methods. This tool provides What tool? It is unclear what tool we are referring to here. A command line tool? A technology? etc. > a > +subset of the functions implemented by the HMC and enables basic partition > +configuration. The set of HMC to hypervisor messages This makes me think that probably the "h" in HMC stands for "hypervisor" so now I start to puzzle things together. But it's nicer if you just expand the acronym the first time you use it. > supported by the > +Novalink component I still haven't figured out what that is, in practice. > are passed to the hypervisor over a VMC interface, which > +is defined below. Maybe some illustration of how HMC, Novalink and VMC work together would be appropriate? > +Virtual Management Channel (VMC) > +A logical device, called the virtual management channel (VMC), is defined > +for communicating between the Novalink application and the hypervisor. So is "Novalink" some specific application, as in a proprietary piece of software or something? I'm still confused. > +This device, similar to a VSCSI server device, What is that acronym now, I get completely confused. > is presented to a designated > +management partition as a virtual device and is only presented when the > +system is not HMC managed. > +This communication device borrows aspects from both VSCSI and ILLAN devices Now there is ILLAN too. > +and is implemented using the CRQ and the RDMA interfaces. CRQ and RDMA. The RDMA I heard about, I wonder what CRQ is. At this point the document is referring to so much external a priori knowledge that I don't have that I don't understand anything at all. So if this document is meant as an introduction to the technology for newcomers, it's not working at all, unfortunately. If some specific domain knowledge is needed to read and understand the document, you need to state these prerequisites at the beginning of the document. Something like: "if you don't know a lot about virtualization already, then don't even bother trying to read this document". Yours, Linus Walleij
