From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44023 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OyRQl-0001PG-8U for qemu-devel@nongnu.org; Wed, 22 Sep 2010 11:40:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OyRQf-0006mb-Ci for qemu-devel@nongnu.org; Wed, 22 Sep 2010 11:40:23 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:61043) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OyRQf-0006lx-8W for qemu-devel@nongnu.org; Wed, 22 Sep 2010 11:40:17 -0400 Received: by qyk5 with SMTP id 5so945444qyk.4 for ; Wed, 22 Sep 2010 08:40:16 -0700 (PDT) Message-ID: <4C9A2340.4030207@codemonkey.ws> Date: Wed, 22 Sep 2010 10:39:44 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] RFC: Monitor high-level design References: <20100921154646.76ad2626@doriath> <4C9A0212.6070002@codemonkey.ws> <20100922120347.3a98ebdd@doriath> In-Reply-To: <20100922120347.3a98ebdd@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Avi Kivity , Markus Armbruster , qemu-devel On 09/22/2010 10:03 AM, Luiz Capitulino wrote: > On Wed, 22 Sep 2010 08:18:10 -0500 > Anthony Liguori wrote: > > >> On 09/22/2010 07:32 AM, Markus Armbruster wrote: >> > [...] > > >>> I can't see why we must have a dogmatic "thou shalt not use anything but >>> QMP to implement HMP commands, ever" rule. >>> > I based most of my ideas on what Anthony has described at: > > http://wiki.qemu.org/Features/QMP_0.14 > > But that's more like a start point, of course that we shouldn't be dogmatic, > specially when we have better ideas. > > >> FWIW, print/sum can be easily implemented in terms of QMP commands. >> > Yes, but I think the point is when/if we should do it. > > >> It's not really necessary for every possibly HMP command to be >> implemented in terms of QMP. However, unless you can do everything in >> QMP that you can do in HMP, then people will be inclined to use HMP as a >> management interface. >> > s/use HMP/use QMP > > >> So, the only real rule we should try to adopt is that QMP is at least as >> powerful as HMP. That should be our goal. >> > Agreed. > > >>>> - HMP can't be moved outside of QEMU, if we want that we'd have to >>>> write a new Monitor client (potentially in a different language, >>>> which is actually good) >>>> >>>> >>> I'd rather not worry about that now. >>> >>> >>> >>>> - Not sure if HMP features like command completion will perfectly fit >>>> >>>> >>> Maybe I'm naive, but I figure that as long as the human monitor controls >>> its character device and knows enough about its own commands, it can do >>> completion pretty much the same way it does now. >>> > Yes, that's right. > > >>> Let's have a closer look at the human monitor: >>> >>> +---------------+ reads/writes >>> | human monitor |<---------------> character device >>> +---------------+ ^ >>> | | >>> | calls | >>> | | >>> v | >>> +---------------+ writes | >>> | hmon handlers | -------------------------+ >>> +---------------+ >>> >>> The "human monitor" box actually has identifiable sub-boxes: >>> >>> * Command table: human monitor command description >>> >>> * Several argument types: methods to parse, check, complete, ... >>> >>> * Reader: read a line of input from character device >>> >>> - Readline, uses table for completion >>> >>> * Parser: parse a line of input, uses table to parse arguments >>> >>> * Executor: call the appropriate handler, uses table >>> >>> The machine monitor (short: qmon) is pretty similar, actually. >>> > That's the 'is pretty similar' part I was referring to when I said (by IRC > I guess) that qmon and hmon can share the device handling code. > > >>> Differences: >>> >>> * We read and parse JSON objects, not lines. >>> >>> * We don't bother to set up completion. >>> >>> * The qmon handlers don't write to the character device. They return >>> objects instead, to be written by the qmon executor. >>> >>> Remember that we want qmon handlers to use proper internal interfaces to >>> do the real work. In other words, qmon handlers should be fairly >>> trivial. >>> >>> I can think of three ways to write a human monitor handler: >>> >>> 1. Call a qmon handler, print the object it returns, suitably formatted. >>> >>> 2. Call the internal interfaces, just like qmon handlers do. Print the >>> results. >>> >>> This cut out the qmon handler middle-man. Since the qmon handler is >>> supposed to be trivial, this isn't a big deal. >>> >>> 3. Just do it. I think that's fine for some commands that make sense >>> only human monitor, like print. Also fine for legacy commands >>> scheduled for removal; we got more useful things to do than >>> beautifying those. The ones we want to keep we should convert to >>> 1. or 2. Doing that early may be advisable to get the internal >>> interfaces used, but it's not like "need to convert everything before >>> anything works". >>> >>> You see hmon doesn't use all of qmon, only qmon handlers, and even that >>> only with 1. Can't see a layering issue there. >>> > Yes and note that we have 1 and 3 today already. > > >> Yeah, I think what you describe makes perfect sense. As long as QMP is >> a thin wrapper around an API, and HMP also calls the API, then it's >> pretty clear that QMP is at least as powerful as HMP. >> > Ok, let me get into two details: > > 1. The BufferCharDriverState is only going to be used for human monitor > passthrough, right? > Well, I think the pointer Markus raised is that there are two ways to implement HMP in terms of QMP. One of them is to do: static void do_commit(Monitor *mon, const char *device) { QObject *res; res = qmp_command("commit", "{'device': %s}", device); if (qobject_is_qerror(res)) { monitor_printf(mon, "commit: %s\n", qerror_to_str(qobject_to_qerror(res))); return; } } And the second way is: static void do_commit(Monitor *mon, const char *device) { Error *err = NULL; qpi_commit(device, &err); if (err) { monitor_printf(mon, "commit: %s\n", error_to_str(err); error_free(err); } } Which takes advantage of the notion that all QMP commands are expressed as proper C interfaces internally. Under the covers, qpi_commit() could be a direct API call or it could even be a QMP client RPC interface. As long as qpi_commit(...) maps directly to qmp_command("commit", ...) it's all equivalent. There's definitely an elegance to the second approach. Regards, Anthony Liguori > 2. As Markus described above, QMP and HMP perform similar internal steps, > so what about having them behind a common interface, like this: > > struct monitor_ops { > const char *name; > int (*init)(void *opaque); > void (*exit)(void *opaque); > int (*can_read)(void *opaque); > void (*read)(void *opaque, const uint8_t *buf, int size); > void (*event)(void *opaque, int event); > void (*async_message)(void *opaque, MonitorEvent, QObject *data); > }; > > Then monitor.c code would register itself with the chardev layer, calling > each monitor's monitor_ops. > > Note that we already have a feature request to be able to create new monitors > on demand. I believe such design will help. > > Is this overkill? Does anyone have better ideas on how both modules (QMP/HMP) > should be designed/separated? > > Sketches are welcome. >