[Qemu-devel] Re: [PATCH 2/3] QMP: Introduce commands documentation

[Jan Kiszka]

Luiz Capitulino wrote:
> On Wed, 19 May 2010 11:15:16 +0200
> Jan Kiszka <address@hidden> wrote:
> 
>> Luiz Capitulino wrote:
>>> From: Jan Kiszka <address@hidden>
>>>
>>> One of the most important missing feature in QMP today is its
>>> supported commands documentation.
>>>
>>> The plan is to make it part of self-description support, however
>>> self-description is a big task we have been postponing for a
>>> long time now and still don't know when it's going to be done.
>>>
>>> In order not to compromise QMP adoption and make users' life easier,
>>> this commit adds a simple text documentation which fully describes
>>> all QMP supported commands.
>>>
>>> This is not ideal for a number of reasons (harder to maintain,
>>> text-only, etc) but does improve the current situation. To avoid at
>>> least divering from the user monitor help and texi snippets, QMP bits
>>> are also maintained inside qemu-monitor.hx, and hxtool is extended to
>>> generate a single text file from them.
>>>
>>> Signed-off-by: Jan Kiszka <address@hidden>
>>> Signed-off-by: Luiz Capitulino <address@hidden>
>>> ---
>>>  Makefile        |    5 +-
>>>  QMP/README      |    5 +-
>>>  configure       |    4 +
>>>  hxtool          |   44 ++-
>>>  qemu-monitor.hx | 1314 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  5 files changed, 1367 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 306a1a4..110698e 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -29,7 +29,7 @@ $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
>>>  LIBS+=-lz $(LIBS_TOOLS)
>>>
>>>  ifdef BUILD_DOCS
>>> -DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8
>>> +DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 
>>> QMP/qmp-commands.txt
>>>  else
>>>  DOCS=
>>>  endif
>>> @@ -259,6 +259,9 @@ qemu-options.texi: $(SRC_PATH)/qemu-options.hx
>>>  qemu-monitor.texi: $(SRC_PATH)/qemu-monitor.hx
>>>         $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
>>> $@")
>>>
>>> +QMP/qmp-commands.txt: $(SRC_PATH)/qemu-monitor.hx
>>> +       $(call quiet-command,sh $(SRC_PATH)/hxtool -q < $< > $@,"  GEN   
>>> $@")
>>> +
>>>  qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
>>>         $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
>>> $@")
>>>
>>> diff --git a/QMP/README b/QMP/README
>>> index 9334c25..35a80c7 100644
>>> --- a/QMP/README
>>> +++ b/QMP/README
>>> @@ -15,8 +15,9 @@ QMP is JSON[1] based and has the following features:
>>>
>>>  For more information, please, refer to the following files:
>>>
>>> -o qmp-spec.txt    QEMU Monitor Protocol current specification
>>> -o qmp-events.txt  List of available asynchronous events
>>> +o qmp-spec.txt      QEMU Monitor Protocol current specification
>>> +o qmp-commands.txt  QMP supported commands
>>> +o qmp-events.txt    List of available asynchronous events
>>>
>>>  There are also two simple Python scripts available:
>>>
>>> diff --git a/configure b/configure
>>> index 36d028f..6738e0b 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2807,3 +2807,7 @@ ln -s $source_path/Makefile.user $d/Makefile
>>>  if test "$static" = "no" -a "$user_pie" = "yes" ; then
>>>    echo "QEMU_CFLAGS+=-fpie" > $d/config.mak
>>>  fi
>>> +
>>> +if test "$docs" = "yes" ; then
>>> +  mkdir -p QMP
>>> +fi
>>> diff --git a/hxtool b/hxtool
>>> index 8f65532..d499dc0 100644
>>> --- a/hxtool
>>> +++ b/hxtool
>>> @@ -7,7 +7,7 @@ hxtoh()
>>>          case $str in
>>>              HXCOMM*)
>>>              ;;
>>> -            STEXI*|ETEXI*) flag=$(($flag^1))
>>> +            STEXI*|ETEXI*|SQMP*|EQMP*) flag=$(($flag^1))
>>>              ;;
>>>              *)
>>>              test $flag -eq 1 && printf "%s\n" "$str"
>>> @@ -38,6 +38,12 @@ hxtotexi()
>>>              fi
>>>              flag=0
>>>              ;;
>>> +            SQMP*|EQMP*)
>>> +            if test $flag -eq 1 ; then
>>> +                echo "line $line: syntax error: expected ETEXI, found 
>>> $str" >&2
>>> +                exit 1
>>> +            fi
>>> +            ;;
>>>              DEFHEADING*)
>>>              echo "$(expr "$str" : "DEFHEADING(\(.*\))")"
>>>              ;;
>>> @@ -49,9 +55,45 @@ hxtotexi()
>>>      done
>>>  }
>>>
>>> +hxtoqmp()
>>> +{
>>> +    IFS=
>>> +    flag=0
>>> +    while read -r str; do
>>> +        case "$str" in
>>> +            HXCOMM*)
>>> +            ;;
>>> +            SQMP*)
>>> +            if test $flag -eq 1 ; then
>>> +                echo "line $line: syntax error: expected EQMP, found $str" 
>>> >&2
>>> +                exit 1
>>> +            fi
>>> +            flag=1
>>> +            ;;
>>> +            EQMP*)
>>> +            if test $flag -ne 1 ; then
>>> +                echo "line $line: syntax error: expected SQMP, found $str" 
>>> >&2
>>> +                exit 1
>>> +            fi
>>> +            flag=0
>>> +            ;;
>>> +            STEXI*|ETEXI*)
>>> +            if test $flag -eq 1 ; then
>>> +                echo "line $line: syntax error: expected EQMP, found $str" 
>>> >&2
>>> +                exit 1
>>> +            fi
>>> +            ;;
>>> +            *)
>>> +            test $flag -eq 1 && echo "$str"
>>> +            ;;
>>> +        esac
>>> +    done
>>> +}
>>> +
>>>  case "$1" in
>>>  "-h") hxtoh ;;
>>>  "-t") hxtotexi ;;
>>> +"-q") hxtoqmp ;;
>>>  *) exit 1 ;;
>>>  esac
>>>
>> Unfortunately, it looks like you mismerged my hxtool changes (the syntax
>> checking bits and the extensions for S/EQMP.
> 
>  Isn't the extensions for S/EQMP in the hunk right above us?

The conversion should work, and there is even the check for proper
SQMP/EQMP balancing in place. I just realized that already my version
was lacking "line=1" and "line=$((line+1))" for the loop above. Hmpf,
will send an add-on fix later.

> 
>  I didn't submit the syntax checking bits on purpose, there's something
> failing there and I didn't want to check it now.

You already did for QMP, just skipped the STEXI/ETEXI balancing checks.
And if that reports an error for TEXI, better have a closer look. It
worked fine with my tree, so something may have regressed.

> 
>  This series seems to work as expected, anyway.

Yes, no problem, we can still fix small fallouts on top of it.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux