Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)

[Anthony Liguori <anthony@codemonkey.ws>]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

On 05/13/2011 02:35 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>> That's fine.  But what better way to ensure a consistent and stable UI
>> than having it centralized in one place.
>
> Consistent, stable, and bit-rotten.  Unless you come up with a way to
> catch bit-rot immediately.  That means on "make", not on "make docs".
>
> Extra points if the error message points right to the offending source
> line.

Er, hit send too fast.  Here's the next patch.

[-- Attachment #2: 0001-qdev-add-centralized-documentation-for-qdev-v2.patch --]
[-- Type: text/x-patch, Size: 9450 bytes --]

>From f5c30c922ce1cf755ef78887f4015c131d8d6841 Mon Sep 17 00:00:00 2001
From: Anthony Liguori <aliguori@us.ibm.com>
Date: Thu, 12 May 2011 10:56:29 -0500
Subject: [PATCH 1/1] qdev: add centralized documentation for qdev (v2)

This adds a -qdev-verify option that will confirm that the documentation matches
the code.  It returns a non-zero exit status if any errors are detected.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile                    |    2 +
 Makefile.doc                |    5 ++
 Makefile.objs               |    2 +-
 qdev-doc.h                  |   23 +++++++++++
 qdev-doc.json               |   16 ++++++++
 qemu-options.hx             |   10 +++++
 scripts/qdev-doc-to-c.py    |   30 ++++++++++++++
 scripts/qdev-doc-to-html.py |   40 +++++++++++++++++++
 vl.c                        |   89 +++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 216 insertions(+), 1 deletions(-)
 create mode 100644 Makefile.doc
 create mode 100644 qdev-doc.h
 create mode 100644 qdev-doc.json
 create mode 100644 scripts/qdev-doc-to-c.py
 create mode 100644 scripts/qdev-doc-to-html.py

diff --git a/Makefile b/Makefile
index 2b0438c..fddb261 100644
--- a/Makefile
+++ b/Makefile
@@ -341,5 +341,7 @@ tarbin:
 	$(mandir)/man1/qemu-img.1 \
 	$(mandir)/man8/qemu-nbd.8
 
+include $(SRC_PATH)/Makefile.doc
+
 # Include automatically generated dependency files
 -include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d)
diff --git a/Makefile.doc b/Makefile.doc
new file mode 100644
index 0000000..c1cf74a
--- /dev/null
+++ b/Makefile.doc
@@ -0,0 +1,5 @@
+qdev-doc.html: $(SRC_PATH)/qdev-doc.json $(SRC_PATH)/scripts/qdev-doc-to-html.py
+	python $(SRC_PATH)/scripts/qdev-doc-to-html.py < $< > $@
+
+qdev-doc.c: $(SRC_PATH)/qdev-doc.json $(SRC_PATH)/scripts/qdev-doc-to-c.py
+	python $(SRC_PATH)/scripts/qdev-doc-to-c.py < $< > $@
diff --git a/Makefile.objs b/Makefile.objs
index 4478c61..9547ab1 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -97,7 +97,7 @@ common-obj-y += bt-hci-csr.o
 common-obj-y += buffered_file.o migration.o migration-tcp.o
 common-obj-y += qemu-char.o savevm.o #aio.o
 common-obj-y += msmouse.o ps2.o
-common-obj-y += qdev.o qdev-properties.o
+common-obj-y += qdev.o qdev-properties.o qdev-doc.o
 common-obj-y += block-migration.o iohandler.o
 common-obj-y += pflib.o
 common-obj-y += bitmap.o bitops.o
diff --git a/qdev-doc.h b/qdev-doc.h
new file mode 100644
index 0000000..401e03e
--- /dev/null
+++ b/qdev-doc.h
@@ -0,0 +1,23 @@
+#ifndef QDEV_DOC_H
+#define QDEV_DOC_H
+
+#include "qemu-common.h"
+
+typedef struct PropertyDocumentation
+{
+    const char *name;
+    const char *type;
+    const char *docs;
+} PropertyDocumentation;
+
+typedef struct DeviceStateDocumentation
+{
+    const char *name;
+    PropertyDocumentation *properties;
+} DeviceStateDocumentation;
+
+extern DeviceStateDocumentation device_docs[];
+
+bool qdev_verify_docs(void);
+
+#endif
diff --git a/qdev-doc.json b/qdev-doc.json
new file mode 100644
index 0000000..a798bff
--- /dev/null
+++ b/qdev-doc.json
@@ -0,0 +1,16 @@
+# -*- Mode: Python -*-
+
+[ { "device": "isa-serial",
+    "parent": "ISADevice",
+    "properties": {
+            "index": { "type": "uint32",
+                       "doc": "A value from 0-3 that describes which IO regions to expose the device on.  This sets appropriate values for iobase and irq." },
+            "iobase": { "type": "hex32",
+                        "doc": "The base IO address to expose the device on." },
+            "irq": { "type": "uint32",
+                     "doc": "The IRQ to use for the device." },
+            "chardev": { "type": "chr",
+                         "doc": "The name of a character device to specify." }
+            }
+  }
+  ]
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f121ad..a6abab4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2378,6 +2378,16 @@ Specify a trace file to log output traces to.
 ETEXI
 #endif
 
+DEF("qdev-verify", 0, QEMU_OPTION_qdev_verify,
+    "-qdev-verify\n"
+    "                Verify qdev properties match documentation\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -qdev-verify
+@findex -qdev-verify
+Verify qdev properties match documentation.
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/scripts/qdev-doc-to-c.py b/scripts/qdev-doc-to-c.py
new file mode 100644
index 0000000..eb7ee59
--- /dev/null
+++ b/scripts/qdev-doc-to-c.py
@@ -0,0 +1,30 @@
+#!/usr/bin/python
+
+import sys
+
+data = sys.stdin.read()
+
+docs = eval(data)
+
+sys.stdout.write('''
+#include "qdev-doc.h"
+
+DeviceStateDocumentation device_docs[] = {''')
+
+for item in docs:
+    sys.stdout.write('''
+    {
+      .name = "%(device)s",
+      .properties = (PropertyDocumentation[]){''' % item)
+    for prop in item["properties"]:
+        sys.stdout.write('''
+        { "%s", "%s", "%s" },''' % (prop, item["properties"][prop]['type'], item["properties"][prop]['doc']))
+
+    sys.stdout.write('''
+        { } },
+    },''')
+
+sys.stdout.write('''
+    { }
+};
+''')
diff --git a/scripts/qdev-doc-to-html.py b/scripts/qdev-doc-to-html.py
new file mode 100644
index 0000000..a25fe35
--- /dev/null
+++ b/scripts/qdev-doc-to-html.py
@@ -0,0 +1,40 @@
+#!/usr/bin/python
+
+import sys
+
+data = sys.stdin.read()
+
+docs = eval(data)
+
+sys.stdout.write('''
+<html>
+<head>
+<title>QEMU device documentation</title>
+</head>
+<body>
+''')
+
+for item in docs:
+    sys.stdout.write('''
+<h2>%(device)s :: %(parent)s</h2>
+
+<table border="1">
+<tr>
+<th>Name</th><th>Type</th><th>Comments</th>
+</tr>
+''' % item)
+    for prop in item["properties"]:
+        sys.stdout.write('''
+<tr>
+<td>%s</td><td>%s</td><td>%s</td>
+</tr>
+''' % (prop, item["properties"][prop]['type'], item["properties"][prop]['doc']))
+
+    sys.stdout.write('''
+</table>
+''')
+
+sys.stdout.write('''
+</body>
+</html>
+''')
diff --git a/vl.c b/vl.c
index bffba69..b9f6e63 100644
--- a/vl.c
+++ b/vl.c
@@ -163,6 +163,8 @@ int main(int argc, char **argv)
 
 #include "ui/qemu-spice.h"
 
+#include "qdev-doc.h"
+
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
 
@@ -1298,6 +1300,82 @@ void qemu_system_vmstop_request(int reason)
     qemu_notify_event();
 }
 
+bool qdev_verify_docs(void)
+{
+    DeviceStateDocumentation *doc;
+    DeviceInfo *dev;
+    int errors = 0;
+    int warnings = 0;
+
+    for (dev = device_info_list; dev; dev = dev->next) {
+        PropertyDocumentation *prop_doc;
+        Property *prop;
+
+        for (doc = device_docs; doc->name; doc++) {
+            if (strcmp(doc->name, dev->name) == 0) {
+                break;
+            }
+        }
+
+        if (doc->name == NULL) {
+            fprintf(stderr, "Warning: device `%s' is undocumented\n",
+                    dev->name);
+            warnings++;
+            continue;
+        }
+
+        for (prop = dev->props; prop->name; prop++) {
+            for (prop_doc = doc->properties; prop_doc->name; prop_doc++) {
+                if (strcmp(prop->name, prop_doc->name) == 0) {
+                    break;
+                }
+            }
+
+            if (prop_doc->name == NULL) {
+                fprintf(stderr, "Warning: device `%s' has undocumented property `%s'\n",
+                        dev->name, prop->name);
+                warnings++;
+            }
+        }
+
+        for (prop_doc = doc->properties; prop_doc->name; prop_doc++) {
+            for (prop = dev->props; prop->name; prop++) {
+                if (strcmp(prop->name, prop_doc->name) == 0) {
+                    break;
+                }
+            }
+
+            if (prop->name == NULL) {
+                fprintf(stderr, "Error: device `%s' has documented property `%s' with no definition\n",
+                        dev->name, prop_doc->name);
+                errors++;
+            }
+        }
+    }
+
+    for (doc = device_docs; doc->name; doc++) {
+        for (dev = device_info_list; dev; dev = dev->next) {
+            if (strcmp(doc->name, dev->name) == 0) {
+                break;
+            }
+        }
+
+        if (dev == NULL) {
+            fprintf(stderr, "Error: documented device `%s' has no definition\n",
+                    doc->name);
+            errors++;
+        }
+    }
+
+    fprintf(stderr, "%d warnings, %d errors.\n", warnings, errors);
+
+    if (errors > 0) {
+        return true;
+    }
+
+    return false;
+}
+
 void main_loop_wait(int nonblocking)
 {
     fd_set rfds, wfds, xfds;
@@ -2057,6 +2135,7 @@ int main(int argc, char **argv, char **envp)
 #endif
     int defconfig = 1;
     const char *trace_file = NULL;
+    bool do_qdev_verify = false;
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -2890,6 +2969,9 @@ int main(int argc, char **argv, char **envp)
                     fclose(fp);
                     break;
                 }
+            case QEMU_OPTION_qdev_verify:
+                do_qdev_verify = true;
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3136,6 +3218,13 @@ int main(int argc, char **argv, char **envp)
 
     module_call_init(MODULE_INIT_DEVICE);
 
+    if (do_qdev_verify) {
+        if (qdev_verify_docs()) {
+            exit(1);
+        }
+        exit(0);
+    }
+
     if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func, NULL, 0) != 0)
         exit(0);
 
-- 
1.7.4.1