[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 01/11] QMP: Introduce specification file

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 01/11] QMP: Introduce specification file
Date: Tue, 30 Jun 2009 13:21:42 -0500
User-agent: Thunderbird (X11/20090320)

Avi Kivity wrote:
On 06/30/2009 05:03 PM, Luiz Capitulino wrote:
Great.  So Luiz, do you understand and agree with the proposed changes
to your series?

  Yes, only the lists and dictionaries types are not very clear
to me.

  Should command handlers return data in those types to the
monitor and thus get printed by it?

  Detailed is welcome, anyway.

I would recommend proceeding as follows:

1. Add QValue, QArray, QDictionary

QValue is a polymorphic object that can be a number, a string, an array, or a dictionary. It needs a bunch of accessors like qvalue_to_qarray() to check if a value is an array, and things like qarray_get() or qdict_put() to manipulate them. For the simpler data types, qvalue_to_int64() and qvalue_from_int64() should suffice.

And add a return value to the monitor functions. I think this is a good first step.

Passing a QArray to each monitor function is one way to go. Another way to go is to use something like libffi to do proper dynamic dispatch. The later approach has the benefit of requiring less code churn and reusing the static type checking.

2. Split the human monitor command implementations into protocol adapters and implementation.

The protocol adapter reads the command, parses it according to qemu-monitor.hx, and constructs a QArray of the parameter list and passes it down to the command implementation. The command implementation returns a QValue, which the protocol adapter formats to the human monitor response format.

One place to start would be to convert something like:

monitor_printf(mon, "file: f=%s,b=%s,c=%d", ...)


value = qlist_create(qstring_create("file"), qdict_create("f", qstring_create(..), "b", qstring_create(..), "c", qstring_create(..), NULL));

But since this is a very regular pattern, you could also do:

value = qmonlist_create("file",
                                     "f", QVALUE_STRING, ...,
                                     "b", QVALUE_STRING, ...,
                                     "c", QVALUE_INT, ...,

You could potentially introduce some serious ninja action by doing something like:

value = qvalue_create("[%s, {'f': %s, 'b': %s', 'c': %d}]", ...);

What's cool about this is that you can mark qvalue_create() as having printf formatting so GCC will help you with type checking.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]