[Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
@ 2011-09-02 17:34 Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 01/15] error: let error_is_type take a NULL error Anthony Liguori
                   ` (15 more replies)
  0 siblings, 16 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Michael Roth, Luiz Capitulino

This is my attempt to jump-start the QAPI switch over.  All of the hard work
is already done in my qapi branch, we just need to start merging stuff.

To simplify the merge process, I've introduced a new mode to the code generator
that lets us do conversions without using the new QMP server.

Once this series is merged, anything that touchs QMP (to modify a command or
add a new command) must do it through QAPI--no exceptions.

This series also includes Dans change to the 'change' command.  I'm not thrilled
about people using this command but its better to err on the side of caution.
It's now officially deprecated with much more robust replacement commands.

Since v1, I've tried to address all review comments.  I've also tested the
modified commands extensively.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 01/15] error: let error_is_type take a NULL error
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 02/15] qerror: add qerror_report_err() (v2) Anthony Liguori
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 error.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/error.c b/error.c
index b802752..68c0039 100644
--- a/error.c
+++ b/error.c
@@ -97,6 +97,10 @@ bool error_is_type(Error *err, const char *fmt)
     char *ptr;
     char *end;
 
+    if (!err) {
+        return false;
+    }
+
     ptr = strstr(fmt, "'class': '");
     assert(ptr != NULL);
     ptr += strlen("'class': '");
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 02/15] qerror: add qerror_report_err() (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 01/15] error: let error_is_type take a NULL error Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 03/15] qapi: add code generation support for middle mode Anthony Liguori
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

This provides a bridge between Error (new error mechanism) and QError (old error
mechanism).  Errors can be propagated whereas QError cannot.

The minor evilness avoids layering violations.  Since QError should go away RSN,
it seems like a reasonable hack.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Fix propagation of error to the monitor (Luiz)
---
 qerror.c |   33 +++++++++++++++++++++++++++++++++
 qerror.h |    2 ++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 3d64b80..c2823ac 100644
--- a/qerror.c
+++ b/qerror.c
@@ -478,6 +478,39 @@ void qerror_report_internal(const char *file, int linenr, const char *func,
     }
 }
 
+/* Evil... */
+struct Error
+{
+    QDict *obj;
+    const char *fmt;
+    char *msg;
+};
+
+void qerror_report_err(Error *err)
+{
+    QError *qerr;
+    int i;
+
+    qerr = qerror_new();
+    loc_save(&qerr->loc);
+    QINCREF(err->obj);
+    qerr->error = err->obj;
+
+    for (i = 0; qerror_table[i].error_fmt; i++) {
+        if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
+            qerr->entry = &qerror_table[i];
+            break;
+        }
+    }
+
+    if (monitor_cur_is_qmp()) {
+        monitor_set_error(cur_mon, qerr);
+    } else {
+        qerror_print(qerr);
+        QDECREF(qerr);
+    }
+}
+
 /**
  * qobject_to_qerror(): Convert a QObject into a QError
  */
diff --git a/qerror.h b/qerror.h
index 8058456..4fe24aa 100644
--- a/qerror.h
+++ b/qerror.h
@@ -15,6 +15,7 @@
 #include "qdict.h"
 #include "qstring.h"
 #include "qemu-error.h"
+#include "error.h"
 #include <stdarg.h>
 
 typedef struct QErrorStringTable {
@@ -39,6 +40,7 @@ QString *qerror_human(const QError *qerror);
 void qerror_print(QError *qerror);
 void qerror_report_internal(const char *file, int linenr, const char *func,
                             const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report_err(Error *err);
 QString *qerror_format(const char *fmt, QDict *error);
 #define qerror_report(fmt, ...) \
     qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 03/15] qapi: add code generation support for middle mode
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 01/15] error: let error_is_type take a NULL error Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 02/15] qerror: add qerror_report_err() (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2) Anthony Liguori
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

To get the ball rolling merging QAPI, this patch introduces a "middle mode" to
the code generator.  In middle mode, the code generator generates marshalling
functions that are compatible with the current QMP server.  We absolutely need
to replace the current QMP server in order to support proper asynchronous
commands but using a middle mode provides a middle-ground that lets us start
converting commands in tree.

Note that all of the commands have been converted already in my glib branch.
Middle mode only exists until we finish merging them from my branch into the
main tree.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi/qapi-types-core.h   |    3 ++
 scripts/qapi-commands.py |   79 +++++++++++++++++++++++++++++++++++++--------
 scripts/qapi-types.py    |    3 ++
 scripts/qapi.py          |    4 ++-
 4 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/qapi/qapi-types-core.h b/qapi/qapi-types-core.h
index a79bc2b..27e6be0 100644
--- a/qapi/qapi-types-core.h
+++ b/qapi/qapi-types-core.h
@@ -17,4 +17,7 @@
 #include "qemu-common.h"
 #include "error.h"
 
+/* FIXME this is temporary until we remove middle mode */
+#include "monitor.h"
+
 #endif
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index bf61740..f5d402c 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -167,9 +167,10 @@ qmp_input_visitor_cleanup(mi);
     pop_indent()
     return ret.rstrip()
 
-def gen_marshal_output(name, args, ret_type):
+def gen_marshal_output(name, args, ret_type, middle_mode):
     if not ret_type:
         return ""
+
     ret = mcgen('''
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp)
 {
@@ -188,16 +189,34 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-            c_ret_type=c_type(ret_type), c_name=c_var(name), ret_type=ret_type)
+                 c_ret_type=c_type(ret_type), c_name=c_var(name),
+                 ret_type=ret_type)
 
     return ret
 
-def gen_marshal_input(name, args, ret_type):
+def gen_marshal_input_decl(name, args, ret_type, middle_mode):
+    if middle_mode:
+        return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict, QObject **ret)' % c_var(name)
+    else:
+        return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_var(name)
+
+
+
+def gen_marshal_input(name, args, ret_type, middle_mode):
+    hdr = gen_marshal_input_decl(name, args, ret_type, middle_mode)
+
     ret = mcgen('''
-static void qmp_marshal_input_%(c_name)s(QDict *args, QObject **ret, Error **errp)
+%(header)s
 {
 ''',
-                c_name=c_var(name))
+                header=hdr)
+
+    if middle_mode:
+        ret += mcgen('''
+    Error *local_err = NULL;
+    Error **errp = &local_err;
+    QDict *args = (QDict *)qdict;
+''')
 
     if ret_type:
         if c_type(ret_type).endswith("*"):
@@ -220,6 +239,10 @@ static void qmp_marshal_input_%(c_name)s(QDict *args, QObject **ret, Error **err
                      visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
                      visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
                      visitor_input_block=gen_visitor_input_block(args, "QOBJECT(args)"))
+    else:
+        ret += mcgen('''
+    (void)args;
+''')
 
     ret += mcgen('''
     if (error_is_set(errp)) {
@@ -234,10 +257,29 @@ out:
 ''')
     ret += mcgen('''
 %(visitor_input_block_cleanup)s
+''',
+                 visitor_input_block_cleanup=gen_visitor_input_block(args, None,
+                                                                     dealloc=True))
+
+    if middle_mode:
+        ret += mcgen('''
+                                                                    
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+    return 0;
+''')
+    else:
+        ret += mcgen('''
     return;
+''')
+
+    ret += mcgen('''
 }
-''',
-                 visitor_input_block_cleanup=gen_visitor_input_block(args, None, dealloc=True))
+''')
+
     return ret
 
 def gen_registry(commands):
@@ -284,7 +326,7 @@ def gen_command_decl_prologue(header, guard, prefix=""):
 #include "error.h"
 
 ''',
-                 header=basename(h_file), guard=guardname(h_file), prefix=prefix)
+                 header=basename(header), guard=guardname(header), prefix=prefix)
     return ret
 
 def gen_command_def_prologue(prefix="", proxy=False):
@@ -317,11 +359,11 @@ def gen_command_def_prologue(prefix="", proxy=False):
                 prefix=prefix)
     if not proxy:
         ret += '#include "%sqmp-commands.h"' % prefix
-    return ret + "\n"
+    return ret + "\n\n"
 
 
 try:
-    opts, args = getopt.gnu_getopt(sys.argv[1:], "p:o:", ["prefix=", "output-dir=", "type="])
+    opts, args = getopt.gnu_getopt(sys.argv[1:], "p:o:m", ["prefix=", "output-dir=", "type=", "middle"])
 except getopt.GetoptError, err:
     print str(err)
     sys.exit(1)
@@ -331,6 +373,7 @@ prefix = ""
 dispatch_type = "sync"
 c_file = 'qmp-marshal.c'
 h_file = 'qmp-commands.h'
+middle_mode = False
 
 for o, a in opts:
     if o in ("-p", "--prefix"):
@@ -339,6 +382,8 @@ for o, a in opts:
         output_dir = a + "/"
     elif o in ("-t", "--type"):
         dispatch_type = a
+    elif o in ("-m", "--middle"):
+        middle_mode = True
 
 c_file = output_dir + prefix + c_file
 h_file = output_dir + prefix + h_file
@@ -370,14 +415,20 @@ if dispatch_type == "sync":
         ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
         fdecl.write(ret)
         if ret_type:
-            ret = gen_marshal_output(cmd['command'], arglist, ret_type) + "\n"
+            ret = gen_marshal_output(cmd['command'], arglist, ret_type, middle_mode) + "\n"
             fdef.write(ret)
-        ret = gen_marshal_input(cmd['command'], arglist, ret_type) + "\n"
+
+        if middle_mode:
+            fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], arglist, ret_type, middle_mode))
+
+        ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + "\n"
         fdef.write(ret)
 
     fdecl.write("\n#endif\n");
-    ret = gen_registry(commands)
-    fdef.write(ret)
+
+    if not middle_mode:
+        ret = gen_registry(commands)
+        fdef.write(ret)
 
     fdef.flush()
     fdef.close()
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index cece325..fc0f7af 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -268,3 +268,6 @@ fdecl.write('''
 
 fdecl.flush()
 fdecl.close()
+
+fdef.flush()
+fdef.close()
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 56af232..5299976 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -200,4 +200,6 @@ def basename(filename):
     return filename.split("/")[-1]
 
 def guardname(filename):
-    return filename.replace("/", "_").replace("-", "_").split(".")[0].upper()
+    if filename.startswith('./'):
+        filename = filename[2:]
+    return filename.replace("/", "_").replace("-", "_").split(".")[0].upper() + '_H'
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (2 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 03/15] qapi: add code generation support for middle mode Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 20:39   ` Luiz Capitulino
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 05/15] qapi: convert query-name Anthony Liguori
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Use the new middle mode within the existing QMP server.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - fixed some of the generated header dependencies
---
 Makefile         |   12 ++++++++++++
 Makefile.objs    |    2 ++
 Makefile.target  |    6 +++---
 monitor.c        |   11 ++++++++---
 qapi-schema.json |    3 +++
 5 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 qapi-schema.json

diff --git a/Makefile b/Makefile
index e0cf51a..8c010d7 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
 ifeq ($(TRACE_BACKEND),dtrace)
 GENERATED_HEADERS += trace-dtrace.h
 endif
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
 
 ifneq ($(wildcard config-host.mak),)
 # Put the all: rule here so that config-host.mak can contain dependencies.
@@ -184,9 +185,20 @@ $(qapi-dir)/qga-qapi-types.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scr
 $(qapi-dir)/qga-qapi-visit.c: $(qapi-dir)/qga-qapi-visit.h
 $(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
+$(qapi-dir)/qga-qmp-commands.h: $(qapi-dir)/qga-qmp-marshal.c
 $(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
 
+qapi-types.c: qapi-types.h
+qapi-types.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py -o "." < $<, "  GEN   $@")
+qapi-visit.c: qapi-visit.h
+qapi-visit.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "."  < $<, "  GEN   $@")
+qmp-commands.h: qmp-marshal.c
+qmp-marshal.c: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -m -o "." < $<, "  GEN   $@")
+
 test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
 test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
 
diff --git a/Makefile.objs b/Makefile.objs
index 26b885b..4821484 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -402,6 +402,8 @@ qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-
 qapi-nested-y += qmp-registry.o qmp-dispatch.o
 qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
 
+common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
+
 ######################################################################
 # guest agent
 
diff --git a/Makefile.target b/Makefile.target
index 0787758..84f7e49 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -371,7 +371,7 @@ obj-alpha-y += vga.o cirrus_vga.o
 
 main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
-monitor.o: hmp-commands.h qmp-commands.h
+monitor.o: hmp-commands.h qmp-commands-old.h
 
 $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
 
@@ -403,13 +403,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
 hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
 
-qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
+qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
 
 clean:
 	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
 	rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
-	rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
+	rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
 ifdef CONFIG_TRACE_SYSTEMTAP
 	rm -f *.stp
 endif
diff --git a/monitor.c b/monitor.c
index df0f622..628e156 100644
--- a/monitor.c
+++ b/monitor.c
@@ -121,6 +121,7 @@ typedef struct mon_cmd_t {
         int  (*cmd_async)(Monitor *mon, const QDict *params,
                           MonitorCompletion *cb, void *opaque);
     } mhandler;
+    bool qapi;
     int flags;
 } mon_cmd_t;
 
@@ -3159,7 +3160,7 @@ static const mon_cmd_t info_cmds[] = {
 };
 
 static const mon_cmd_t qmp_cmds[] = {
-#include "qmp-commands.h"
+#include "qmp-commands-old.h"
     { /* NULL */ },
 };
 
@@ -5029,10 +5030,14 @@ static void qmp_call_query_cmd(Monitor *mon, const mon_cmd_t *cmd)
         if (monitor_has_error(mon)) {
             monitor_protocol_emitter(mon, NULL);
         }
-    } else {
-        cmd->mhandler.info_new(mon, &ret_data);
+    } else if (cmd->qapi) {
+        QDict *args = qdict_new();
+
+        cmd->mhandler.cmd_new(mon, args, &ret_data);
         monitor_protocol_emitter(mon, ret_data);
         qobject_decref(ret_data);
+
+        QDECREF(args);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
new file mode 100644
index 0000000..7fcefdb
--- /dev/null
+++ b/qapi-schema.json
@@ -0,0 +1,3 @@
+# -*- Mode: Python -*-
+#
+# QAPI Schema
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2) Anthony Liguori
@ 2011-09-02 20:39   ` Luiz Capitulino
  0 siblings, 0 replies; 27+ messages in thread
From: Luiz Capitulino @ 2011-09-02 20:39 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, Michael Roth

On Fri,  2 Sep 2011 12:34:47 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Use the new middle mode within the existing QMP server.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
>  - fixed some of the generated header dependencies
> ---
>  Makefile         |   12 ++++++++++++
>  Makefile.objs    |    2 ++
>  Makefile.target  |    6 +++---
>  monitor.c        |   11 ++++++++---
>  qapi-schema.json |    3 +++
>  5 files changed, 28 insertions(+), 6 deletions(-)
>  create mode 100644 qapi-schema.json
> 
> diff --git a/Makefile b/Makefile
> index e0cf51a..8c010d7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
>  ifeq ($(TRACE_BACKEND),dtrace)
>  GENERATED_HEADERS += trace-dtrace.h
>  endif
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
>  
>  ifneq ($(wildcard config-host.mak),)
>  # Put the all: rule here so that config-host.mak can contain dependencies.
> @@ -184,9 +185,20 @@ $(qapi-dir)/qga-qapi-types.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scr
>  $(qapi-dir)/qga-qapi-visit.c: $(qapi-dir)/qga-qapi-visit.h
>  $(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
>  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
> +$(qapi-dir)/qga-qmp-commands.h: $(qapi-dir)/qga-qmp-marshal.c
>  $(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
>  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
>  
> +qapi-types.c: qapi-types.h
> +qapi-types.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py -o "." < $<, "  GEN   $@")
> +qapi-visit.c: qapi-visit.h
> +qapi-visit.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "."  < $<, "  GEN   $@")
> +qmp-commands.h: qmp-marshal.c
> +qmp-marshal.c: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -m -o "." < $<, "  GEN   $@")
> +
>  test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
>  test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
>  
> diff --git a/Makefile.objs b/Makefile.objs
> index 26b885b..4821484 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -402,6 +402,8 @@ qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-
>  qapi-nested-y += qmp-registry.o qmp-dispatch.o
>  qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>  
> +common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
> +
>  ######################################################################
>  # guest agent
>  
> diff --git a/Makefile.target b/Makefile.target
> index 0787758..84f7e49 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -371,7 +371,7 @@ obj-alpha-y += vga.o cirrus_vga.o
>  
>  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>  
> -monitor.o: hmp-commands.h qmp-commands.h
> +monitor.o: hmp-commands.h qmp-commands-old.h
>  
>  $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
>  
> @@ -403,13 +403,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
>  hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
>  	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
>  
> -qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
> +qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
>  	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
>  
>  clean:
>  	rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
>  	rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
> -	rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
> +	rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
>  ifdef CONFIG_TRACE_SYSTEMTAP
>  	rm -f *.stp
>  endif
> diff --git a/monitor.c b/monitor.c
> index df0f622..628e156 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -121,6 +121,7 @@ typedef struct mon_cmd_t {
>          int  (*cmd_async)(Monitor *mon, const QDict *params,
>                            MonitorCompletion *cb, void *opaque);
>      } mhandler;
> +    bool qapi;
>      int flags;
>  } mon_cmd_t;
>  
> @@ -3159,7 +3160,7 @@ static const mon_cmd_t info_cmds[] = {
>  };
>  
>  static const mon_cmd_t qmp_cmds[] = {
> -#include "qmp-commands.h"
> +#include "qmp-commands-old.h"
>      { /* NULL */ },
>  };
>  
> @@ -5029,10 +5030,14 @@ static void qmp_call_query_cmd(Monitor *mon, const mon_cmd_t *cmd)
>          if (monitor_has_error(mon)) {
>              monitor_protocol_emitter(mon, NULL);
>          }
> -    } else {
> -        cmd->mhandler.info_new(mon, &ret_data);

The else clause can't be dropped, otherwise non-qapi converted query
commands won't work.

I can fix it myself in case this is going through my tree (and in case
there isn't any other change request).

> +    } else if (cmd->qapi) {
> +        QDict *args = qdict_new();
> +
> +        cmd->mhandler.cmd_new(mon, args, &ret_data);
>          monitor_protocol_emitter(mon, ret_data);
>          qobject_decref(ret_data);
> +
> +        QDECREF(args);
>      }
>  }
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> new file mode 100644
> index 0000000..7fcefdb
> --- /dev/null
> +++ b/qapi-schema.json
> @@ -0,0 +1,3 @@
> +# -*- Mode: Python -*-
> +#
> +# QAPI Schema

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 05/15] qapi: convert query-name
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (3 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 06/15] block: add unsafe_probe Anthony Liguori
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

A simple example conversion 'info name'.  This also adds the new files for
QMP and HMP.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs    |    1 +
 hmp.c            |   26 ++++++++++++++++++++++++++
 hmp.h            |   22 ++++++++++++++++++++++
 monitor.c        |   27 +++++----------------------
 qapi-schema.json |   23 +++++++++++++++++++++++
 qmp.c            |   28 ++++++++++++++++++++++++++++
 6 files changed, 105 insertions(+), 22 deletions(-)
 create mode 100644 hmp.c
 create mode 100644 hmp.h
 create mode 100644 qmp.c

diff --git a/Makefile.objs b/Makefile.objs
index 4821484..97473fd 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -403,6 +403,7 @@ qapi-nested-y += qmp-registry.o qmp-dispatch.o
 qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
 
 common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
+common-obj-y += qmp.o hmp.o
 
 ######################################################################
 # guest agent
diff --git a/hmp.c b/hmp.c
new file mode 100644
index 0000000..47e1ff7
--- /dev/null
+++ b/hmp.c
@@ -0,0 +1,26 @@
+/*
+ * Human Monitor Interface
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hmp.h"
+#include "qmp-commands.h"
+
+void hmp_info_name(Monitor *mon)
+{
+    NameInfo *info;
+
+    info = qmp_query_name(NULL);
+    if (info->has_name) {
+        monitor_printf(mon, "%s\n", info->name);
+    }
+    qapi_free_NameInfo(info);
+}
diff --git a/hmp.h b/hmp.h
new file mode 100644
index 0000000..5fe73f1
--- /dev/null
+++ b/hmp.h
@@ -0,0 +1,22 @@
+/*
+ * Human Monitor Interface
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef HMP_H
+#define HMP_H
+
+#include "qemu-common.h"
+#include "qapi-types.h"
+
+void hmp_info_name(Monitor *mon);
+
+#endif
diff --git a/monitor.c b/monitor.c
index 628e156..a83d731 100644
--- a/monitor.c
+++ b/monitor.c
@@ -63,6 +63,8 @@
 #endif
 #include "trace/control.h"
 #include "ui/qemu-spice.h"
+#include "qmp-commands.h"
+#include "hmp.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
@@ -759,24 +761,6 @@ static void do_info_version(Monitor *mon, QObject **ret_data)
         'micro': %d }, 'package': %s }", major, minor, micro, QEMU_PKGVERSION);
 }
 
-static void do_info_name_print(Monitor *mon, const QObject *data)
-{
-    QDict *qdict;
-
-    qdict = qobject_to_qdict(data);
-    if (qdict_size(qdict) == 0) {
-        return;
-    }
-
-    monitor_printf(mon, "%s\n", qdict_get_str(qdict, "name"));
-}
-
-static void do_info_name(Monitor *mon, QObject **ret_data)
-{
-    *ret_data = qemu_name ? qobject_from_jsonf("{'name': %s }", qemu_name) :
-                            qobject_from_jsonf("{}");
-}
-
 static QObject *get_cmd_dict(const char *name)
 {
     const char *p;
@@ -3071,8 +3055,7 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM name",
-        .user_print = do_info_name_print,
-        .mhandler.info_new = do_info_name,
+        .mhandler.info = hmp_info_name,
     },
     {
         .name       = "uuid",
@@ -3268,8 +3251,8 @@ static const mon_cmd_t qmp_query_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM name",
-        .user_print = do_info_name_print,
-        .mhandler.info_new = do_info_name,
+        .mhandler.cmd_new = qmp_marshal_input_query_name,
+        .qapi       = true,
     },
     {
         .name       = "uuid",
diff --git a/qapi-schema.json b/qapi-schema.json
index 7fcefdb..654409b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1,3 +1,26 @@
 # -*- Mode: Python -*-
 #
 # QAPI Schema
+
+##
+# @NameInfo:
+#
+# Guest name information.
+#
+# @name: #optional The name of the guest
+#
+# Since 0.14.0
+##
+{ 'type': 'NameInfo', 'data': {'*name': 'str'} }
+
+##
+# @query-name:
+#
+# Return the name information of a guest.
+#
+# Returns: @NameInfo of the guest
+#
+# Since 0.14.0
+##
+{ 'command': 'query-name', 'returns': 'NameInfo' }
+
diff --git a/qmp.c b/qmp.c
new file mode 100644
index 0000000..8aa9c66
--- /dev/null
+++ b/qmp.c
@@ -0,0 +1,28 @@
+/*
+ * QEMU Management Protocol
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "sysemu.h"
+#include "qmp-commands.h"
+
+NameInfo *qmp_query_name(Error **errp)
+{
+    NameInfo *info = g_malloc0(sizeof(*info));
+
+    if (qemu_name) {
+        info->has_name = true;
+        info->name = g_strdup(qemu_name);
+    }
+
+    return info;
+}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 06/15] block: add unsafe_probe
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (4 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 05/15] qapi: convert query-name Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 07/15] monitor: expose readline state Anthony Liguori
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Mark formats that cannot be probed with 100% reliability (just raw).  This is
necessary to implement change-blockdev.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/raw.c |    2 ++
 block_int.h |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block/raw.c b/block/raw.c
index 555db4f..5c74bd6 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -145,6 +145,8 @@ static BlockDriver bdrv_raw = {
     .bdrv_create        = raw_create,
     .create_options     = raw_create_options,
     .bdrv_has_zero_init = raw_has_zero_init,
+
+    .unsafe_probe       = true,
 };
 
 static void bdrv_raw_init(void)
diff --git a/block_int.h b/block_int.h
index 8a72b80..f3d9a79 100644
--- a/block_int.h
+++ b/block_int.h
@@ -146,6 +146,8 @@ struct BlockDriver {
      */
     int (*bdrv_has_zero_init)(BlockDriverState *bs);
 
+    bool unsafe_probe;
+
     QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 07/15] monitor: expose readline state
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (5 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 06/15] block: add unsafe_probe Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 08/15] qerror: add additional parameter to QERR_DEVICE_ENCRYPTED Anthony Liguori
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

HMP is now implemented in terms of QMP.  The monitor has a bunch of logic to
deal with HMP right now like readline support.  Export it from the monitor so
we can consume it in hmp.c.

In short time, hmp.c will take over all of the readline bits.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c |   11 ++++++++---
 monitor.h |    5 +++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index a83d731..fa93239 100644
--- a/monitor.c
+++ b/monitor.c
@@ -223,7 +223,7 @@ int monitor_cur_is_qmp(void)
     return cur_mon && monitor_ctrl_mode(cur_mon);
 }
 
-static void monitor_read_command(Monitor *mon, int show_prompt)
+void monitor_read_command(Monitor *mon, int show_prompt)
 {
     if (!mon->rs)
         return;
@@ -233,8 +233,8 @@ static void monitor_read_command(Monitor *mon, int show_prompt)
         readline_show_prompt(mon->rs);
 }
 
-static int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
-                                 void *opaque)
+int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
+                          void *opaque)
 {
     if (monitor_ctrl_mode(mon)) {
         qerror_report(QERR_MISSING_PARAMETER, "password");
@@ -5303,6 +5303,11 @@ static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
     monitor_read_command(mon, 1);
 }
 
+ReadLineState *monitor_get_rs(Monitor *mon)
+{
+    return mon->rs;
+}
+
 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
                                 BlockDriverCompletionFunc *completion_cb,
                                 void *opaque)
diff --git a/monitor.h b/monitor.h
index 4f2d328..6b2ef77 100644
--- a/monitor.h
+++ b/monitor.h
@@ -6,6 +6,7 @@
 #include "qerror.h"
 #include "qdict.h"
 #include "block.h"
+#include "readline.h"
 
 extern Monitor *cur_mon;
 extern Monitor *default_mon;
@@ -61,5 +62,9 @@ void monitor_flush(Monitor *mon);
 typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
 
 void monitor_set_error(Monitor *mon, QError *qerror);
+void monitor_read_command(Monitor *mon, int show_prompt);
+ReadLineState *monitor_get_rs(Monitor *mon);
+int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
+                          void *opaque);
 
 #endif /* !MONITOR_H */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 08/15] qerror: add additional parameter to QERR_DEVICE_ENCRYPTED
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (6 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 07/15] monitor: expose readline state Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 09/15] qapi: convert eject (qmp and hmp) to QAPI Anthony Liguori
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Changing an encrypted block device requires a flow of:

 1) change device
 2) receive error from change
 3) block_passwd

DeviceEncrypted receives the device name but with a block backend with multiple
backing files, any one of the backing files could be encrypted.  From a UI
perspective, you have no way of knowing which file is encrypted without parsing
the qcow2 files :-/

Add the actual encrypted filename to the error message so that a UI can display
that information to the user.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c |    3 ++-
 qerror.h  |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index fa93239..c6da388 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5321,7 +5321,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
     }
 
     if (monitor_ctrl_mode(mon)) {
-        qerror_report(QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs));
+        qerror_report(QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
+                      bdrv_get_encrypted_filename(bs));
         return -1;
     }
 
diff --git a/qerror.h b/qerror.h
index 4fe24aa..3bbff5c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -64,7 +64,7 @@ QError *qobject_to_qerror(const QObject *obj);
     "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
 
 #define QERR_DEVICE_ENCRYPTED \
-    "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
+    "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s} }"
 
 #define QERR_DEVICE_INIT_FAILED \
     "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 09/15] qapi: convert eject (qmp and hmp) to QAPI
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (7 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 08/15] qerror: add additional parameter to QERR_DEVICE_ENCRYPTED Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 10/15] qapi: convert block_passwd and add set-blockdev-password Anthony Liguori
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

N.B. This temporarily changes the error semantics of eject.  Errors are not
stable in QMP so this is not technically a breakage.  The last patch restores
the previous error semantics.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 blockdev.c       |   22 +++++++++++-----------
 blockdev.h       |    1 -
 hmp-commands.hx  |    3 +--
 hmp.c            |   14 ++++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   25 +++++++++++++++++++++++++
 qmp-commands.hx  |    3 +--
 7 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 2602591..8e68546 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -16,6 +16,7 @@
 #include "sysemu.h"
 #include "hw/qdev.h"
 #include "block_int.h"
+#include "qmp-commands.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
@@ -635,32 +636,31 @@ out:
     return ret;
 }
 
-static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
+static int eject_device(BlockDriverState *bs, int force, Error **errp)
 {
     if (!bdrv_is_removable(bs)) {
-        qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
+        error_set(errp, QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
         return -1;
     }
     if (!force && bdrv_is_locked(bs)) {
-        qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+        error_set(errp, QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
         return -1;
     }
     bdrv_close(bs);
     return 0;
 }
 
-int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
     BlockDriverState *bs;
-    int force = qdict_get_try_bool(qdict, "force", 0);
-    const char *filename = qdict_get_str(qdict, "device");
 
-    bs = bdrv_find(filename);
+    bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, filename);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
-    return eject_device(mon, bs, force);
+
+    eject_device(bs, force, errp);
 }
 
 int do_block_set_passwd(Monitor *mon, const QDict *qdict,
@@ -706,7 +706,7 @@ int do_change_block(Monitor *mon, const char *device,
             return -1;
         }
     }
-    if (eject_device(mon, bs, 0) < 0) {
+    if (eject_device(bs, 0, NULL) < 0) {
         return -1;
     }
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
diff --git a/blockdev.h b/blockdev.h
index 3587786..badbf01 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -58,7 +58,6 @@ DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi);
 DriveInfo *add_init_drive(const char *opts);
 
 void do_commit(Monitor *mon, const QDict *qdict);
-int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..f91591f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -76,8 +76,7 @@ ETEXI
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
         .help       = "eject a removable medium (use -f to force it)",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_eject,
+        .mhandler.cmd = hmp_eject,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 47e1ff7..36eb5b9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -24,3 +24,17 @@ void hmp_info_name(Monitor *mon)
     }
     qapi_free_NameInfo(info);
 }
+
+void hmp_eject(Monitor *mon, const QDict *qdict)
+{
+    int force = qdict_get_try_bool(qdict, "force", 0);
+    const char *device = qdict_get_str(qdict, "device");
+    Error *err = NULL;
+
+    qmp_eject(device, true, force, &err);
+    if (err) {
+        monitor_printf(mon, "eject: %s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
+
diff --git a/hmp.h b/hmp.h
index 5fe73f1..6a552c1 100644
--- a/hmp.h
+++ b/hmp.h
@@ -18,5 +18,6 @@
 #include "qapi-types.h"
 
 void hmp_info_name(Monitor *mon);
+void hmp_eject(Monitor *mon, const QDict *args);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 654409b..934ea81 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -24,3 +24,28 @@
 ##
 { 'command': 'query-name', 'returns': 'NameInfo' }
 
+##
+# @eject:
+#
+# Ejects a device from a removable drive.
+#
+# @device:  The name of the device
+#
+# @force:   @optional If true, eject regardless of whether the drive is locked.
+#           If not specified, the default value is false.
+#
+# Returns:  Nothing on success
+#           If @device is not a valid block device, DeviceNotFound
+#           If @device is not removable and @force is false, DeviceNotRemovable
+#           If @force is false and @device is locked, DeviceLocked
+#
+# Notes:    If the @force flag is used, the backing file will be closed
+#           regardless of whether the device is removable.  This may result in
+#           a badly broken guest.
+#
+#           Ejecting a device with no media results in success
+#
+# Since: 0.14.0
+##
+{ 'command': 'eject', 'data': {'device': 'str', '*force': 'bool'} }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 27cc66e..1c913c1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -89,8 +89,7 @@ EQMP
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
         .help       = "eject a removable medium (use -f to force it)",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_eject,
+        .mhandler.cmd_new = qmp_marshal_input_eject,
     },
 
 SQMP
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 10/15] qapi: convert block_passwd and add set-blockdev-password
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (8 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 09/15] qapi: convert eject (qmp and hmp) to QAPI Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 11/15] qapi: add change-vnc-password (v2) Anthony Liguori
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

block_passwd is unfortunately named so while converting block_passwd to QAPI,
introduce a more properly named alias.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 blockdev.c       |   29 +++++++++++++++--------------
 hmp-commands.hx  |    2 +-
 hmp.c            |   12 ++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   11 +++++++++--
 6 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8e68546..07eafce 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -663,28 +663,29 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
     eject_device(bs, force, errp);
 }
 
-int do_block_set_passwd(Monitor *mon, const QDict *qdict,
-                        QObject **ret_data)
+void qmp_set_blockdev_password(const char *device, const char *password,
+                               Error **err)
 {
     BlockDriverState *bs;
-    int err;
+    int ret;
 
-    bs = bdrv_find(qdict_get_str(qdict, "device"));
+    bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, qdict_get_str(qdict, "device"));
-        return -1;
+        error_set(err, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
 
-    err = bdrv_set_key(bs, qdict_get_str(qdict, "password"));
-    if (err == -EINVAL) {
-        qerror_report(QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
-        return -1;
-    } else if (err < 0) {
-        qerror_report(QERR_INVALID_PASSWORD);
-        return -1;
+    ret = bdrv_set_key(bs, password);
+    if (ret == -EINVAL) {
+        error_set(err, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
+    } else if (ret < 0) {
+        error_set(err, QERR_INVALID_PASSWORD);
     }
+}
 
-    return 0;
+void qmp_block_passwd(const char *device, const char *password, Error **err)
+{
+    qmp_set_blockdev_password(device, password, err);
 }
 
 int do_change_block(Monitor *mon, const char *device,
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f91591f..4dc9f9a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1205,7 +1205,7 @@ ETEXI
         .params     = "block_passwd device password",
         .help       = "set the password of encrypted block devices",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_set_passwd,
+        .mhandler.cmd = hmp_block_passwd,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 36eb5b9..a8ae36b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -38,3 +38,15 @@ void hmp_eject(Monitor *mon, const QDict *qdict)
     }
 }
 
+void hmp_block_passwd(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *password = qdict_get_str(qdict, "password");
+    Error *err = NULL;
+
+    qmp_set_blockdev_password(device, password, &err);
+    if (err) {
+        monitor_printf(mon, "block_passwd: %s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 6a552c1..8f72ef2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -19,5 +19,6 @@
 
 void hmp_info_name(Monitor *mon);
 void hmp_eject(Monitor *mon, const QDict *args);
+void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 934ea81..f159d81 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -49,3 +49,50 @@
 ##
 { 'command': 'eject', 'data': {'device': 'str', '*force': 'bool'} }
 
+##
+# @block_passwd:
+#
+# This command sets the password of a block device that has not been open
+# with a password and requires one.
+#
+# The two cases where this can happen are a block device is created through
+# QEMU's initial command line or a block device is changed through the legacy
+# @change interface.
+#
+# In the event that the block device is created through the initial command
+# line, the VM will start in the stopped state regardless of whether '-S' is
+# used.  The intention is for a management tool to query the block devices to
+# determine which ones are encrypted, set the passwords with this command, and
+# then start the guest with the @cont command.
+#
+# @device:   the name of the device to set the password on
+#
+# @password: the password to use for the device
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @device is not encrypted, DeviceNotEncrypted
+#          If @password is not valid for this device, InvalidPassword
+#
+# Notes:  Not all block formats support encryption and some that do are not
+#         able to validate that a password is correct.  Disk corruption may
+#         occur if an invalid password is specified.
+#
+# Since: 0.14.0
+##
+{ 'command': 'block_passwd',
+  'data': {'device': 'str', 'password': 'str'} }
+
+##
+# @set-blockdev-password:
+#
+# Alias for @block_passwd.
+#
+# @device:   see @block_passwd
+# @password: see @block_passwd
+#
+# Since: 1.0
+##
+{ 'command': 'set-blockdev-password',
+  'data': {'device': 'str', 'password': 'str'} }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1c913c1..1de5af5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -837,8 +837,15 @@ EQMP
         .args_type  = "device:B,password:s",
         .params     = "block_passwd device password",
         .help       = "set the password of encrypted block devices",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_set_passwd,
+        .mhandler.cmd_new = qmp_marshal_input_block_passwd,
+    },
+
+    {
+        .name       = "set-blockdev-password",
+        .args_type  = "device:B,password:s",
+        .params     = "block_passwd device password",
+        .help       = "set the password of encrypted block devices",
+        .mhandler.cmd_new = qmp_marshal_input_set_blockdev_password,
     },
 
 SQMP
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 11/15] qapi: add change-vnc-password (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (9 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 10/15] qapi: convert block_passwd and add set-blockdev-password Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2) Anthony Liguori
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

This is a new QMP only command that only changes the VNC password.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - fix schema documentation
---
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |    8 ++++++++
 qmp.c            |   11 ++++++++++-
 3 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index f159d81..350cf1c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -96,3 +96,16 @@
 { 'command': 'set-blockdev-password',
   'data': {'device': 'str', 'password': 'str'} }
 
+##
+# @change-vnc-password:
+#
+# Change the VNC server password.
+#
+# @password:  the new password to use with VNC authentication
+#
+# Since: 1.0
+#
+# Notes:  An empty password in this command will set the password to the empty
+#         string.  Existing clients are unaffected by executing this command.
+##
+{ 'command': 'change-vnc-password', 'data': {'password': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1de5af5..7df3938 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -868,6 +868,14 @@ Example:
 EQMP
 
     {
+        .name       = "change-vnc-password",
+        .args_type  = "password:s",
+        .params     = "password",
+        .help       = "set vnc password",
+        .mhandler.cmd_new = qmp_marshal_input_change_vnc_password,
+    },
+
+    {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
         .params     = "protocol password action-if-connected",
diff --git a/qmp.c b/qmp.c
index 8aa9c66..f817a88 100644
--- a/qmp.c
+++ b/qmp.c
@@ -12,9 +12,11 @@
  */
 
 #include "qemu-common.h"
-#include "sysemu.h"
 #include "qmp-commands.h"
 
+#include "sysemu.h"
+#include "console.h"
+
 NameInfo *qmp_query_name(Error **errp)
 {
     NameInfo *info = g_malloc0(sizeof(*info));
@@ -26,3 +28,10 @@ NameInfo *qmp_query_name(Error **errp)
 
     return info;
 }
+
+void qmp_change_vnc_password(const char *password, Error **err)
+{
+    if (vnc_display_password(NULL, password) < 0) {
+        error_set(err, QERR_SET_PASSWD_FAILED);
+    }
+}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (10 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 11/15] qapi: add change-vnc-password (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 20:50   ` Luiz Capitulino
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2) Anthony Liguori
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

New QMP only command to change the VNC server's listening address.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Enhanced docs (Luiz)
---
 qapi-schema.json |   15 +++++++++++++++
 qmp-commands.hx  |    8 ++++++++
 qmp.c            |    7 +++++++
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 350cf1c..0c6c9b8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -109,3 +109,18 @@
 #         string.  Existing clients are unaffected by executing this command.
 ##
 { 'command': 'change-vnc-password', 'data': {'password': 'str'} }
+
+##
+# @change-vnc-listen:
+#
+# Change the host that the VNC server listens on.
+#
+# @target:  the new server specification to listen on
+#
+# Since: 1.0
+#
+# Notes:  At this moment in time, the behavior of existing client connections
+#         when this command is executed is undefined.  The authentication
+#         settings may change after executing this command.
+##
+{ 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7df3938..5cab212 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -876,6 +876,14 @@ EQMP
     },
 
     {
+        .name       = "change-vnc-listen",
+        .args_type  = "target:s",
+        .params     = "target",
+        .help       = "set vnc listening address",
+        .mhandler.cmd_new = qmp_marshal_input_change_vnc_listen,
+    },
+
+    {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
         .params     = "protocol password action-if-connected",
diff --git a/qmp.c b/qmp.c
index f817a88..73d6172 100644
--- a/qmp.c
+++ b/qmp.c
@@ -35,3 +35,10 @@ void qmp_change_vnc_password(const char *password, Error **err)
         error_set(err, QERR_SET_PASSWD_FAILED);
     }
 }
+
+void qmp_change_vnc_listen(const char *target, Error **err)
+{
+    if (vnc_display_open(NULL, target) < 0) {
+        error_set(err, QERR_VNC_SERVER_FAILED, target);
+    }
+}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2) Anthony Liguori
@ 2011-09-02 20:50   ` Luiz Capitulino
  2011-09-12  9:17     ` Daniel P. Berrange
  0 siblings, 1 reply; 27+ messages in thread
From: Luiz Capitulino @ 2011-09-02 20:50 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, Michael Roth

On Fri,  2 Sep 2011 12:34:55 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> New QMP only command to change the VNC server's listening address.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
>  - Enhanced docs (Luiz)
> ---
>  qapi-schema.json |   15 +++++++++++++++
>  qmp-commands.hx  |    8 ++++++++
>  qmp.c            |    7 +++++++
>  3 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 350cf1c..0c6c9b8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -109,3 +109,18 @@
>  #         string.  Existing clients are unaffected by executing this command.
>  ##
>  { 'command': 'change-vnc-password', 'data': {'password': 'str'} }
> +
> +##
> +# @change-vnc-listen:
> +#
> +# Change the host that the VNC server listens on.
> +#
> +# @target:  the new server specification to listen on
> +#
> +# Since: 1.0
> +#
> +# Notes:  At this moment in time, the behavior of existing client connections
> +#         when this command is executed is undefined.  The authentication
> +#         settings may change after executing this command.

It seems to completely disable authentication. At least when using
password auth. I'd be very clear about that.

> +##
> +{ 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 7df3938..5cab212 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -876,6 +876,14 @@ EQMP
>      },
>  
>      {
> +        .name       = "change-vnc-listen",
> +        .args_type  = "target:s",
> +        .params     = "target",
> +        .help       = "set vnc listening address",
> +        .mhandler.cmd_new = qmp_marshal_input_change_vnc_listen,
> +    },
> +
> +    {
>          .name       = "set_password",
>          .args_type  = "protocol:s,password:s,connected:s?",
>          .params     = "protocol password action-if-connected",
> diff --git a/qmp.c b/qmp.c
> index f817a88..73d6172 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -35,3 +35,10 @@ void qmp_change_vnc_password(const char *password, Error **err)
>          error_set(err, QERR_SET_PASSWD_FAILED);
>      }
>  }
> +
> +void qmp_change_vnc_listen(const char *target, Error **err)
> +{
> +    if (vnc_display_open(NULL, target) < 0) {
> +        error_set(err, QERR_VNC_SERVER_FAILED, target);
> +    }
> +}

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2)
  2011-09-02 20:50   ` Luiz Capitulino
@ 2011-09-12  9:17     ` Daniel P. Berrange
  2011-09-12  9:28       ` Daniel P. Berrange
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel P. Berrange @ 2011-09-12  9:17 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: Kevin Wolf, Anthony Liguori, qemu-devel, Michael Roth

On Fri, Sep 02, 2011 at 05:50:05PM -0300, Luiz Capitulino wrote:
> On Fri,  2 Sep 2011 12:34:55 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
> 
> > New QMP only command to change the VNC server's listening address.
> > 
> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > ---
> > v1 -> v2
> >  - Enhanced docs (Luiz)
> > ---
> >  qapi-schema.json |   15 +++++++++++++++
> >  qmp-commands.hx  |    8 ++++++++
> >  qmp.c            |    7 +++++++
> >  3 files changed, 30 insertions(+), 0 deletions(-)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 350cf1c..0c6c9b8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -109,3 +109,18 @@
> >  #         string.  Existing clients are unaffected by executing this command.
> >  ##
> >  { 'command': 'change-vnc-password', 'data': {'password': 'str'} }
> > +
> > +##
> > +# @change-vnc-listen:
> > +#
> > +# Change the host that the VNC server listens on.
> > +#
> > +# @target:  the new server specification to listen on
> > +#
> > +# Since: 1.0
> > +#
> > +# Notes:  At this moment in time, the behavior of existing client connections
> > +#         when this command is executed is undefined.  The authentication
> > +#         settings may change after executing this command.
> 
> It seems to completely disable authentication. At least when using
> password auth. I'd be very clear about that.

That is really bad, since even if we have another command to set the
authentication mode, this creates a designed-in race condition. To be
securely race-free, we need to be able to set the desired auth mode
first, and then change the listen address without it affecting auth.

    change-vnc-auth   tls
    change-vnc-listen 123.2.3.5:5901

If we really want vnc-listen to have possible side-effects on auth,
then we need to be able to put the VNC server in an offline mode
while making a sequence of configuration changes eg, something like

    change-vnc-status offline
    change-vnc-listen 123.2.3.5:5901
    change-vnc-auth   tls
    change-vnc-status online

No incoming client connections would be allowed while it is offline

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2)
  2011-09-12  9:17     ` Daniel P. Berrange
@ 2011-09-12  9:28       ` Daniel P. Berrange
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrange @ 2011-09-12  9:28 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: Kevin Wolf, Anthony Liguori, qemu-devel, Michael Roth

On Mon, Sep 12, 2011 at 10:17:21AM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 02, 2011 at 05:50:05PM -0300, Luiz Capitulino wrote:
> > On Fri,  2 Sep 2011 12:34:55 -0500
> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> > 
> > > New QMP only command to change the VNC server's listening address.
> > > 
> > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > > ---
> > > v1 -> v2
> > >  - Enhanced docs (Luiz)
> > > ---
> > >  qapi-schema.json |   15 +++++++++++++++
> > >  qmp-commands.hx  |    8 ++++++++
> > >  qmp.c            |    7 +++++++
> > >  3 files changed, 30 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 350cf1c..0c6c9b8 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -109,3 +109,18 @@
> > >  #         string.  Existing clients are unaffected by executing this command.
> > >  ##
> > >  { 'command': 'change-vnc-password', 'data': {'password': 'str'} }
> > > +
> > > +##
> > > +# @change-vnc-listen:
> > > +#
> > > +# Change the host that the VNC server listens on.
> > > +#
> > > +# @target:  the new server specification to listen on
> > > +#
> > > +# Since: 1.0
> > > +#
> > > +# Notes:  At this moment in time, the behavior of existing client connections
> > > +#         when this command is executed is undefined.  The authentication
> > > +#         settings may change after executing this command.
> > 
> > It seems to completely disable authentication. At least when using
> > password auth. I'd be very clear about that.
> 
> That is really bad, since even if we have another command to set the
> authentication mode, this creates a designed-in race condition. To be
> securely race-free, we need to be able to set the desired auth mode
> first, and then change the listen address without it affecting auth.
> 
>     change-vnc-auth   tls
>     change-vnc-listen 123.2.3.5:5901

On closer inspection, I see that 'change-vnc-listen' just accepts the
full string with encoded options, that is used for the '-vnc' command
line. I thought that for QMP we going to make sure we didn't use any
encoded strings, and gave each option a dedicated parameter ?

eg instead of:

  { 'command': 'change-vnc-password', 'data': {'target': 'str'} }

Wouldn't we want something like:

  { 'command': 'change-vnc-password', 'data': {
        'listen': bool,    /* Whether to listen, or do a reverse connection */
        'address': 'str',
        'port': 'int',
        'password': 'string',
        'sasl': bool,
        'tls': bool,
        'x509': bool,
        'lossy': bool,
        'no-lock-key-sync': bool,
        ...
     }
   }

At which point we could also make  '-vnc' use qemu-config for its option
parsing ?

Or is your idea that we just do the more straightforward QMP command for
change-vnc-listen now, with the view that everything will be changed for
the future QEMU Object model rewrite ?

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (11 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 21:06   ` Luiz Capitulino
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 14/15] qapi: convert change (v2) Anthony Liguori
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

A new QMP only command to change the blockdev associated with a block device.
The semantics of change right now are just plain scary.  This command introduces
sane semantics.  For more details, see the documentation in the schema file.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Rename command to drive-change (Kevin)
---
 blockdev.c       |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 qapi-schema.json |   30 +++++++++++++++
 qmp-commands.hx  |    8 ++++
 3 files changed, 140 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 07eafce..cd338ed 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -688,12 +688,101 @@ void qmp_block_passwd(const char *device, const char *password, Error **err)
     qmp_set_blockdev_password(device, password, err);
 }
 
+static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
+                                    int bdrv_flags, BlockDriver *drv,
+                                    const char *password, Error **errp)
+{
+    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
+        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+        return;
+    }
+
+    if (bdrv_key_required(bs)) {
+        if (password) {
+            if (bdrv_set_key(bs, password) < 0) {
+                error_set(errp, QERR_INVALID_PASSWORD);
+            }
+        } else {
+            error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
+                      bdrv_get_encrypted_filename(bs));
+        }
+    } else if (password) {
+        error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
+    }
+}
+
+void qmp_drive_change(const char *device, const char *filename,
+                      bool has_format, const char *format,
+                      bool has_password, const char *password,
+                      Error **errp)
+{
+    BlockDriverState *bs, *bs1;
+    BlockDriver *drv = NULL;
+    int bdrv_flags;
+    Error *err = NULL;
+    bool probed_raw = false;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (has_format) {
+        drv = bdrv_find_whitelisted_format(format);
+        if (!drv) {
+            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+            return;
+        }
+    }
+
+    bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
+
+    if (!has_password) {
+        password = NULL;
+    }
+
+    /* Try to open once with a temporary block device to make sure that
+     * the disk isn't encrypted and we lack a key.  This also helps keep
+     * this operation as a transaction.  That is, if we fail, we try very
+     * hard to make sure that the state is the same as before the operation
+     * was started.
+     */
+    bs1 = bdrv_new("");
+    qmp_bdrv_open_encrypted(bs1, filename, bdrv_flags, drv, password, &err);
+    if (!has_format && bs1->drv->unsafe_probe) {
+        probed_raw = true;
+    }
+    bdrv_delete(bs1);
+
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    if (probed_raw) {
+        /* We will not auto probe raw files. */
+        error_set(errp, QERR_MISSING_PARAMETER, "format");
+        return;
+    }
+
+    eject_device(bs, 0, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
+}
+
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt)
 {
     BlockDriverState *bs;
     BlockDriver *drv = NULL;
     int bdrv_flags;
+    Error *err = NULL;
 
     bs = bdrv_find(device);
     if (!bs) {
@@ -707,16 +796,23 @@ int do_change_block(Monitor *mon, const char *device,
             return -1;
         }
     }
-    if (eject_device(bs, 0, NULL) < 0) {
-        return -1;
-    }
+
     bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
     bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
-    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
-        qerror_report(QERR_OPEN_FILE_FAILED, filename);
+
+    eject_device(bs, 0, &err);
+    if (err) {
+        qerror_report_err(err);
+        return -1;
+    }
+
+    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
+    if (err) {
+        qerror_report_err(err);
         return -1;
     }
-    return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
+
+    return 0;
 }
 
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/qapi-schema.json b/qapi-schema.json
index 0c6c9b8..cbb5bf1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -124,3 +124,33 @@
 #         settings may change after executing this command.
 ##
 { 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
+
+##
+# @drive-change:
+#
+# This is the preferred interface for changing a block device.
+#
+# @device:   The block device name.
+#
+# @filename: The new filename for the block device.  This may contain options
+#            encoded in a format specified by @format.
+#
+# @format:   #optional The format to use open the block device
+#
+# @password: #optional The password to use if the block device is encrypted
+#
+# Returns:  Nothing on success.
+#          If @device is not a valid block device, DeviceNotFound
+#          If @format is not a valid block format, InvalidBlockFormat
+#          If @filename is encrypted and @password isn't supplied,
+#            DeviceEncrypted.  The call should be repeated with @password
+#            supplied.
+#          If @format is not specified and @filename is a format that cannot
+#            be safely probed, MissingParameter.
+#          If @filename cannot be opened, OpenFileFailed
+#
+# Since: 1.0
+##
+{ 'command': 'drive-change',
+  'data': {'device': 'str', 'filename': 'str', '*format': 'str',
+           '*password': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5cab212..623f158 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -121,6 +121,14 @@ EQMP
         .mhandler.cmd_new = do_change,
     },
 
+    {
+        .name       = "drive-change",
+        .args_type  = "device:B,filename:F,format:s?password:s?",
+        .params     = "device filename [format] [password]",
+        .help       = "change a removable medium, optional format",
+        .mhandler.cmd_new = qmp_marshal_input_drive_change,
+    },
+
 SQMP
 change
 ------
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2) Anthony Liguori
@ 2011-09-02 21:06   ` Luiz Capitulino
  2011-09-02 21:10     ` Luiz Capitulino
  0 siblings, 1 reply; 27+ messages in thread
From: Luiz Capitulino @ 2011-09-02 21:06 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, Michael Roth

On Fri,  2 Sep 2011 12:34:56 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> A new QMP only command to change the blockdev associated with a block device.
> The semantics of change right now are just plain scary.  This command introduces
> sane semantics.  For more details, see the documentation in the schema file.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
>  - Rename command to drive-change (Kevin)
> ---
>  blockdev.c       |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  qapi-schema.json |   30 +++++++++++++++
>  qmp-commands.hx  |    8 ++++
>  3 files changed, 140 insertions(+), 6 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 07eafce..cd338ed 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -688,12 +688,101 @@ void qmp_block_passwd(const char *device, const char *password, Error **err)
>      qmp_set_blockdev_password(device, password, err);
>  }
>  
> +static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
> +                                    int bdrv_flags, BlockDriver *drv,
> +                                    const char *password, Error **errp)
> +{
> +    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> +        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
> +        return;
> +    }
> +
> +    if (bdrv_key_required(bs)) {
> +        if (password) {
> +            if (bdrv_set_key(bs, password) < 0) {
> +                error_set(errp, QERR_INVALID_PASSWORD);
> +            }
> +        } else {
> +            error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
> +                      bdrv_get_encrypted_filename(bs));
> +        }
> +    } else if (password) {
> +        error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
> +    }
> +}
> +
> +void qmp_drive_change(const char *device, const char *filename,
> +                      bool has_format, const char *format,
> +                      bool has_password, const char *password,
> +                      Error **errp)
> +{
> +    BlockDriverState *bs, *bs1;
> +    BlockDriver *drv = NULL;
> +    int bdrv_flags;
> +    Error *err = NULL;
> +    bool probed_raw = false;
> +
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    if (has_format) {
> +        drv = bdrv_find_whitelisted_format(format);
> +        if (!drv) {
> +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> +            return;
> +        }
> +    }
> +
> +    bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> +    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> +
> +    if (!has_password) {
> +        password = NULL;
> +    }
> +
> +    /* Try to open once with a temporary block device to make sure that
> +     * the disk isn't encrypted and we lack a key.  This also helps keep
> +     * this operation as a transaction.  That is, if we fail, we try very
> +     * hard to make sure that the state is the same as before the operation
> +     * was started.
> +     */
> +    bs1 = bdrv_new("");
> +    qmp_bdrv_open_encrypted(bs1, filename, bdrv_flags, drv, password, &err);
> +    if (!has_format && bs1->drv->unsafe_probe) {
> +        probed_raw = true;
> +    }
> +    bdrv_delete(bs1);
> +
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    if (probed_raw) {
> +        /* We will not auto probe raw files. */
> +        error_set(errp, QERR_MISSING_PARAMETER, "format");
> +        return;
> +    }

This has to be noted in the documentation.

> +
> +    eject_device(bs, 0, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }

Hm. I'm not wondering if this command isn't actually overloading two
distinct operations: eject and insert. Maybe what we want here is drive-insert?

> +
> +    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
> +}
> +
>  int do_change_block(Monitor *mon, const char *device,
>                      const char *filename, const char *fmt)
>  {
>      BlockDriverState *bs;
>      BlockDriver *drv = NULL;
>      int bdrv_flags;
> +    Error *err = NULL;
>  
>      bs = bdrv_find(device);
>      if (!bs) {
> @@ -707,16 +796,23 @@ int do_change_block(Monitor *mon, const char *device,
>              return -1;
>          }
>      }
> -    if (eject_device(bs, 0, NULL) < 0) {
> -        return -1;
> -    }
> +
>      bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
>      bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> -    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> -        qerror_report(QERR_OPEN_FILE_FAILED, filename);
> +
> +    eject_device(bs, 0, &err);
> +    if (err) {
> +        qerror_report_err(err);
> +        return -1;
> +    }
> +
> +    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
> +    if (err) {
> +        qerror_report_err(err);
>          return -1;
>      }
> -    return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
> +
> +    return 0;
>  }
>  
>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0c6c9b8..cbb5bf1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -124,3 +124,33 @@
>  #         settings may change after executing this command.
>  ##
>  { 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
> +
> +##
> +# @drive-change:
> +#
> +# This is the preferred interface for changing a block device.
> +#
> +# @device:   The block device name.
> +#
> +# @filename: The new filename for the block device.  This may contain options
> +#            encoded in a format specified by @format.
> +#
> +# @format:   #optional The format to use open the block device
> +#
> +# @password: #optional The password to use if the block device is encrypted
> +#
> +# Returns:  Nothing on success.
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @format is not a valid block format, InvalidBlockFormat
> +#          If @filename is encrypted and @password isn't supplied,
> +#            DeviceEncrypted.  The call should be repeated with @password
> +#            supplied.
> +#          If @format is not specified and @filename is a format that cannot
> +#            be safely probed, MissingParameter.
> +#          If @filename cannot be opened, OpenFileFailed
> +#
> +# Since: 1.0
> +##
> +{ 'command': 'drive-change',
> +  'data': {'device': 'str', 'filename': 'str', '*format': 'str',
> +           '*password': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 5cab212..623f158 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -121,6 +121,14 @@ EQMP
>          .mhandler.cmd_new = do_change,
>      },
>  
> +    {
> +        .name       = "drive-change",
> +        .args_type  = "device:B,filename:F,format:s?password:s?",
> +        .params     = "device filename [format] [password]",
> +        .help       = "change a removable medium, optional format",
> +        .mhandler.cmd_new = qmp_marshal_input_drive_change,
> +    },
> +
>  SQMP
>  change
>  ------

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2)
  2011-09-02 21:06   ` Luiz Capitulino
@ 2011-09-02 21:10     ` Luiz Capitulino
  0 siblings, 0 replies; 27+ messages in thread
From: Luiz Capitulino @ 2011-09-02 21:10 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, Michael Roth

On Fri, 2 Sep 2011 18:06:13 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Fri,  2 Sep 2011 12:34:56 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
> 
> > A new QMP only command to change the blockdev associated with a block device.
> > The semantics of change right now are just plain scary.  This command introduces
> > sane semantics.  For more details, see the documentation in the schema file.
> > 
> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > ---
> > v1 -> v2
> >  - Rename command to drive-change (Kevin)
> > ---
> >  blockdev.c       |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  qapi-schema.json |   30 +++++++++++++++
> >  qmp-commands.hx  |    8 ++++
> >  3 files changed, 140 insertions(+), 6 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 07eafce..cd338ed 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -688,12 +688,101 @@ void qmp_block_passwd(const char *device, const char *password, Error **err)
> >      qmp_set_blockdev_password(device, password, err);
> >  }
> >  
> > +static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
> > +                                    int bdrv_flags, BlockDriver *drv,
> > +                                    const char *password, Error **errp)
> > +{
> > +    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> > +        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
> > +        return;
> > +    }
> > +
> > +    if (bdrv_key_required(bs)) {
> > +        if (password) {
> > +            if (bdrv_set_key(bs, password) < 0) {
> > +                error_set(errp, QERR_INVALID_PASSWORD);
> > +            }
> > +        } else {
> > +            error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
> > +                      bdrv_get_encrypted_filename(bs));
> > +        }
> > +    } else if (password) {
> > +        error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
> > +    }
> > +}
> > +
> > +void qmp_drive_change(const char *device, const char *filename,
> > +                      bool has_format, const char *format,
> > +                      bool has_password, const char *password,
> > +                      Error **errp)
> > +{
> > +    BlockDriverState *bs, *bs1;
> > +    BlockDriver *drv = NULL;
> > +    int bdrv_flags;
> > +    Error *err = NULL;
> > +    bool probed_raw = false;
> > +
> > +    bs = bdrv_find(device);
> > +    if (!bs) {
> > +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> > +        return;
> > +    }
> > +
> > +    if (has_format) {
> > +        drv = bdrv_find_whitelisted_format(format);
> > +        if (!drv) {
> > +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> > +            return;
> > +        }
> > +    }
> > +
> > +    bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> > +    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> > +
> > +    if (!has_password) {
> > +        password = NULL;
> > +    }
> > +
> > +    /* Try to open once with a temporary block device to make sure that
> > +     * the disk isn't encrypted and we lack a key.  This also helps keep
> > +     * this operation as a transaction.  That is, if we fail, we try very
> > +     * hard to make sure that the state is the same as before the operation
> > +     * was started.
> > +     */
> > +    bs1 = bdrv_new("");
> > +    qmp_bdrv_open_encrypted(bs1, filename, bdrv_flags, drv, password, &err);
> > +    if (!has_format && bs1->drv->unsafe_probe) {
> > +        probed_raw = true;
> > +    }
> > +    bdrv_delete(bs1);
> > +
> > +    if (err) {
> > +        error_propagate(errp, err);
> > +        return;
> > +    }
> > +
> > +    if (probed_raw) {
> > +        /* We will not auto probe raw files. */
> > +        error_set(errp, QERR_MISSING_PARAMETER, "format");
> > +        return;
> > +    }
> 
> This has to be noted in the documentation.
> 
> > +
> > +    eject_device(bs, 0, &err);
> > +    if (err) {
> > +        error_propagate(errp, err);
> > +        return;
> > +    }
> 
> Hm. I'm not wondering if this command isn't actually overloading two

s/not/now

> distinct operations: eject and insert. Maybe what we want here is drive-insert?
> 
> > +
> > +    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
> > +}
> > +
> >  int do_change_block(Monitor *mon, const char *device,
> >                      const char *filename, const char *fmt)
> >  {
> >      BlockDriverState *bs;
> >      BlockDriver *drv = NULL;
> >      int bdrv_flags;
> > +    Error *err = NULL;
> >  
> >      bs = bdrv_find(device);
> >      if (!bs) {
> > @@ -707,16 +796,23 @@ int do_change_block(Monitor *mon, const char *device,
> >              return -1;
> >          }
> >      }
> > -    if (eject_device(bs, 0, NULL) < 0) {
> > -        return -1;
> > -    }
> > +
> >      bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> >      bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> > -    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> > -        qerror_report(QERR_OPEN_FILE_FAILED, filename);
> > +
> > +    eject_device(bs, 0, &err);
> > +    if (err) {
> > +        qerror_report_err(err);
> > +        return -1;
> > +    }
> > +
> > +    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
> > +    if (err) {
> > +        qerror_report_err(err);
> >          return -1;
> >      }
> > -    return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
> > +
> > +    return 0;
> >  }
> >  
> >  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 0c6c9b8..cbb5bf1 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -124,3 +124,33 @@
> >  #         settings may change after executing this command.
> >  ##
> >  { 'command': 'change-vnc-listen', 'data': {'target': 'str'} }
> > +
> > +##
> > +# @drive-change:
> > +#
> > +# This is the preferred interface for changing a block device.
> > +#
> > +# @device:   The block device name.
> > +#
> > +# @filename: The new filename for the block device.  This may contain options
> > +#            encoded in a format specified by @format.
> > +#
> > +# @format:   #optional The format to use open the block device
> > +#
> > +# @password: #optional The password to use if the block device is encrypted
> > +#
> > +# Returns:  Nothing on success.
> > +#          If @device is not a valid block device, DeviceNotFound
> > +#          If @format is not a valid block format, InvalidBlockFormat
> > +#          If @filename is encrypted and @password isn't supplied,
> > +#            DeviceEncrypted.  The call should be repeated with @password
> > +#            supplied.
> > +#          If @format is not specified and @filename is a format that cannot
> > +#            be safely probed, MissingParameter.
> > +#          If @filename cannot be opened, OpenFileFailed
> > +#
> > +# Since: 1.0
> > +##
> > +{ 'command': 'drive-change',
> > +  'data': {'device': 'str', 'filename': 'str', '*format': 'str',
> > +           '*password': 'str'} }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 5cab212..623f158 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -121,6 +121,14 @@ EQMP
> >          .mhandler.cmd_new = do_change,
> >      },
> >  
> > +    {
> > +        .name       = "drive-change",
> > +        .args_type  = "device:B,filename:F,format:s?password:s?",
> > +        .params     = "device filename [format] [password]",
> > +        .help       = "change a removable medium, optional format",
> > +        .mhandler.cmd_new = qmp_marshal_input_drive_change,
> > +    },
> > +
> >  SQMP
> >  change
> >  ------
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 14/15] qapi: convert change (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (12 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 15/15] vnc: don't demote authentication protocol when disabling login Anthony Liguori
  2011-09-07 21:56 ` [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Alexander Graf
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Convert the change command to QAPI and document all of its gloriousness.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Fix docs (Luiz)

fix hmp change command
---
 blockdev.c       |   27 ++++++++----------
 blockdev.h       |    6 +++-
 hmp-commands.hx  |    3 +-
 hmp.c            |   59 ++++++++++++++++++++++++++++++++++++++++
 hmp.h            |    1 +
 monitor.c        |   78 +++++++----------------------------------------------
 qapi-schema.json |   38 ++++++++++++++++++++++++++
 qmp-commands.hx  |    3 +-
 8 files changed, 127 insertions(+), 88 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index cd338ed..c70cfec 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -776,8 +776,9 @@ void qmp_drive_change(const char *device, const char *filename,
     qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
 }
 
-int do_change_block(Monitor *mon, const char *device,
-                    const char *filename, const char *fmt)
+void deprecated_qmp_change_blockdev(const char *device, const char *filename,
+                                    bool has_format, const char *format,
+                                    Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv = NULL;
@@ -786,14 +787,14 @@ int do_change_block(Monitor *mon, const char *device,
 
     bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, device);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
-    if (fmt) {
-        drv = bdrv_find_whitelisted_format(fmt);
+    if (has_format) {
+        drv = bdrv_find_whitelisted_format(format);
         if (!drv) {
-            qerror_report(QERR_INVALID_BLOCK_FORMAT, fmt);
-            return -1;
+            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+            return;
         }
     }
 
@@ -802,17 +803,13 @@ int do_change_block(Monitor *mon, const char *device,
 
     eject_device(bs, 0, &err);
     if (err) {
-        qerror_report_err(err);
-        return -1;
+        error_propagate(errp, err);
+        return;
     }
 
     qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
-    if (err) {
-        qerror_report_err(err);
-        return -1;
-    }
 
-    return 0;
+    error_propagate(errp, err);
 }
 
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/blockdev.h b/blockdev.h
index badbf01..54b142f 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -12,6 +12,7 @@
 
 #include "block.h"
 #include "qemu-queue.h"
+#include "error.h"
 
 void blockdev_mark_auto_del(BlockDriverState *bs);
 void blockdev_auto_del(BlockDriverState *bs);
@@ -59,8 +60,9 @@ DriveInfo *add_init_drive(const char *opts);
 
 void do_commit(Monitor *mon, const QDict *qdict);
 int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
-int do_change_block(Monitor *mon, const char *device,
-                    const char *filename, const char *fmt);
+void deprecated_qmp_change_blockdev(const char *device, const char *filename,
+                                    bool has_format, const char *format,
+                                    Error **errp);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4dc9f9a..e7d4f0f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -108,8 +108,7 @@ ETEXI
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
         .help       = "change a removable medium, optional format",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_change,
+        .mhandler.cmd = hmp_change,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index a8ae36b..4e97bc6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -50,3 +50,62 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict)
         error_free(err);
     }
 }
+
+static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
+                                   void *opaque)
+{
+    Error *encryption_err = opaque;
+    Error *err = NULL;
+    const char *device;
+
+    device = error_get_field(encryption_err, "device");
+
+    qmp_block_passwd(device, password, &err);
+    if (err) {
+        monitor_printf(mon, "invalid password\n");
+        error_free(err);
+    }
+
+    error_free(encryption_err);
+
+    monitor_read_command(mon, 1);
+}
+
+static void hmp_change_read_arg(Monitor *mon, const char *password,
+                                void *opaque)
+{
+    qmp_change_vnc_password(password, NULL);
+    monitor_read_command(mon, 1);
+}
+
+void hmp_change(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *target = qdict_get_str(qdict, "target");
+    const char *arg = qdict_get_try_str(qdict, "arg");
+    Error *err = NULL;
+
+    if (strcmp(device, "vnc") == 0 &&
+        (strcmp(target, "passwd") == 0 ||
+         strcmp(target, "password") == 0)) {
+        if (arg == NULL) {
+            monitor_read_password(mon, hmp_change_read_arg, NULL);
+            return;
+        }
+    }
+
+    qmp_change(device, target, !!arg, arg, &err);
+    if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
+        monitor_printf(mon, "%s (%s) is encrypted.\n",
+                       error_get_field(err, "device"),
+                       error_get_field(err, "filename"));
+        if (!monitor_get_rs(mon)) {
+            monitor_printf(mon,
+                           "terminal does not support password prompting\n");
+            error_free(err);
+            return;
+        }
+        readline_start(monitor_get_rs(mon), "Password: ", 1,
+                       cb_hmp_change_bdrv_pwd, err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 8f72ef2..9df6ccc 100644
--- a/hmp.h
+++ b/hmp.h
@@ -20,5 +20,6 @@
 void hmp_info_name(Monitor *mon);
 void hmp_eject(Monitor *mon, const QDict *args);
 void hmp_block_passwd(Monitor *mon, const QDict *qdict);
+void hmp_change(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index c6da388..f91fabd 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1007,78 +1007,22 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
-#ifdef CONFIG_VNC
-static int change_vnc_password(const char *password)
-{
-    if (!password || !password[0]) {
-        if (vnc_display_disable_login(NULL)) {
-            qerror_report(QERR_SET_PASSWD_FAILED);
-            return -1;
-        }
-        return 0;
-    }
-
-    if (vnc_display_password(NULL, password) < 0) {
-        qerror_report(QERR_SET_PASSWD_FAILED);
-        return -1;
-    }
-
-    return 0;
-}
-
-static void change_vnc_password_cb(Monitor *mon, const char *password,
-                                   void *opaque)
+void qmp_change(const char *device, const char *target,
+                bool has_arg, const char *arg, Error **err)
 {
-    change_vnc_password(password);
-    monitor_read_command(mon, 1);
-}
-
-static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
-{
-    if (strcmp(target, "passwd") == 0 ||
-        strcmp(target, "password") == 0) {
-        if (arg) {
-            char password[9];
-            strncpy(password, arg, sizeof(password));
-            password[sizeof(password) - 1] = '\0';
-            return change_vnc_password(password);
+    if (strcmp(device, "vnc") == 0) {
+        if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
+            if (!has_arg || !arg[0]) {
+                vnc_display_disable_login(NULL);
+            } else {
+                qmp_change_vnc_password(arg, err);
+            }
         } else {
-            return monitor_read_password(mon, change_vnc_password_cb, NULL);
+            qmp_change_vnc_listen(target, err);
         }
     } else {
-        if (vnc_display_open(NULL, target) < 0) {
-            qerror_report(QERR_VNC_SERVER_FAILED, target);
-            return -1;
-        }
+        deprecated_qmp_change_blockdev(device, target, has_arg, arg, err);
     }
-
-    return 0;
-}
-#else
-static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
-{
-    qerror_report(QERR_FEATURE_DISABLED, "vnc");
-    return -ENODEV;
-}
-#endif
-
-/**
- * do_change(): Change a removable medium, or VNC configuration
- */
-static int do_change(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    const char *device = qdict_get_str(qdict, "device");
-    const char *target = qdict_get_str(qdict, "target");
-    const char *arg = qdict_get_try_str(qdict, "arg");
-    int ret;
-
-    if (strcmp(device, "vnc") == 0) {
-        ret = do_change_vnc(mon, target, arg);
-    } else {
-        ret = do_change_block(mon, device, target, arg);
-    }
-
-    return ret;
 }
 
 static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/qapi-schema.json b/qapi-schema.json
index cbb5bf1..7050b24 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -84,6 +84,44 @@
   'data': {'device': 'str', 'password': 'str'} }
 
 ##
+# @change:
+#
+# This command is multiple commands multiplexed together.  Generally speaking,
+# it should not be used in favor of the single purpose alternatives such as
+# @change-vnc-listen, @change-vnc-password, and @change-blockdev.
+#
+# @device: This is normally the name of a block device but it may also be 'vnc'.
+#          when it's 'vnc', then sub command depends on @target
+#
+# @target: If @device is a block device, then this is the new filename.
+#          If @device is 'vnc', then if the value 'password' selects the vnc
+#          change password command.   Otherwise, this specifies a new server URI
+#          address to listen to for VNC connections.
+#
+# @arg:    If @device is a block device, then this is an optional format to open
+#          the device with.
+#          If @device is 'vnc' and @target is 'password', this is the new VNC
+#          password to set.  If this argument is an empty string, then no future
+#          logins will be allowed.
+#
+# Returns: Nothing on success.
+#          If @device is not a valid block device, DeviceNotFound
+#          If @format is not a valid block format, InvalidBlockFormat
+#          If the new block device is encrypted, DeviceEncrypted.  Note that
+#          if this error is returned, the device has been opened successfully
+#          and an additional call to @block_passwd is required to set the
+#          device's password.  The behavior of reads and writes to the block
+#          device between when these calls are executed is undefined.
+#
+# Notes:  It is strongly recommended that this interface is not used for
+#         changing block devices.
+#
+# Since: 0.14.0
+##
+{ 'command': 'change',
+  'data': {'device': 'str', 'target': 'str', '*arg': 'str'} }
+
+##
 # @set-blockdev-password:
 #
 # Alias for @block_passwd.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 623f158..8ea604a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -117,8 +117,7 @@ EQMP
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
         .help       = "change a removable medium, optional format",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_change,
+        .mhandler.cmd_new = qmp_marshal_input_change,
     },
 
     {
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [Qemu-devel] [PATCH 15/15] vnc: don't demote authentication protocol when disabling login
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (13 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 14/15] qapi: convert change (v2) Anthony Liguori
@ 2011-09-02 17:34 ` Anthony Liguori
  2011-09-07 21:56 ` [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Alexander Graf
  15 siblings, 0 replies; 27+ messages in thread
From: Anthony Liguori @ 2011-09-02 17:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Michael Roth, Luiz Capitulino

Currently when disabling login in VNC, the password is cleared out and the
authentication protocol is forced to AUTH_VNC.  If you're using a stronger
authentication protocol, this has the effect of downgrading your security
protocol.

Fix this by only changing the authentication protocol if the current
authentication protocol is AUTH_NONE.  That ensures we're never downgrading.

Reported-by: Daniel Berrange <berrange@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Make sure to not demote when changing password (Daniel)
---
 monitor.c |   18 ------------------
 qmp.c     |   19 +++++++++++++++++++
 ui/vnc.c  |    8 ++++++--
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/monitor.c b/monitor.c
index f91fabd..f2272ec 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1007,24 +1007,6 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
-void qmp_change(const char *device, const char *target,
-                bool has_arg, const char *arg, Error **err)
-{
-    if (strcmp(device, "vnc") == 0) {
-        if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
-            if (!has_arg || !arg[0]) {
-                vnc_display_disable_login(NULL);
-            } else {
-                qmp_change_vnc_password(arg, err);
-            }
-        } else {
-            qmp_change_vnc_listen(target, err);
-        }
-    } else {
-        deprecated_qmp_change_blockdev(device, target, has_arg, arg, err);
-    }
-}
-
 static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *protocol  = qdict_get_str(qdict, "protocol");
diff --git a/qmp.c b/qmp.c
index 73d6172..5674adc 100644
--- a/qmp.c
+++ b/qmp.c
@@ -16,6 +16,7 @@
 
 #include "sysemu.h"
 #include "console.h"
+#include "blockdev.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -42,3 +43,21 @@ void qmp_change_vnc_listen(const char *target, Error **err)
         error_set(err, QERR_VNC_SERVER_FAILED, target);
     }
 }
+
+void qmp_change(const char *device, const char *target,
+                bool has_arg, const char *arg, Error **err)
+{
+    if (strcmp(device, "vnc") == 0) {
+        if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
+            if (!has_arg || !arg[0]) {
+                vnc_display_disable_login(NULL);
+            } else {
+                qmp_change_vnc_password(arg, err);
+            }
+        } else {
+            qmp_change_vnc_listen(target, err);
+        }
+    } else {
+        deprecated_qmp_change_blockdev(device, target, has_arg, arg, err);
+    }
+}
diff --git a/ui/vnc.c b/ui/vnc.c
index fc3a612..2d3705f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2648,7 +2648,9 @@ int vnc_display_disable_login(DisplayState *ds)
     }
 
     vs->password = NULL;
-    vs->auth = VNC_AUTH_VNC;
+    if (vs->auth == VNC_AUTH_NONE) {
+        vs->auth = VNC_AUTH_VNC;
+    }
 
     return 0;
 }
@@ -2675,7 +2677,9 @@ int vnc_display_password(DisplayState *ds, const char *password)
         vs->password = NULL;
     }
     vs->password = g_strdup(password);
-    vs->auth = VNC_AUTH_VNC;
+    if (vs->auth == VNC_AUTH_NONE) {
+        vs->auth = VNC_AUTH_VNC;
+    }
 out:
     if (ret != 0) {
         qerror_report(QERR_SET_PASSWD_FAILED);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
  2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
                   ` (14 preceding siblings ...)
  2011-09-02 17:34 ` [Qemu-devel] [PATCH 15/15] vnc: don't demote authentication protocol when disabling login Anthony Liguori
@ 2011-09-07 21:56 ` Alexander Graf
  2011-09-07 22:03   ` Anthony Liguori
  15 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2011-09-07 21:56 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, Luiz Capitulino, qemu-devel, Michael Roth

On 02.09.2011, at 19:34, Anthony Liguori wrote:

> This is my attempt to jump-start the QAPI switch over.  All of the hard work
> is already done in my qapi branch, we just need to start merging stuff.
> 
> To simplify the merge process, I've introduced a new mode to the code generator
> that lets us do conversions without using the new QMP server.
> 
> Once this series is merged, anything that touchs QMP (to modify a command or
> add a new command) must do it through QAPI--no exceptions.
> 
> This series also includes Dans change to the 'change' command.  I'm not thrilled
> about people using this command but its better to err on the side of caution.
> It's now officially deprecated with much more robust replacement commands.
> 
> Since v1, I've tried to address all review comments.  I've also tested the
> modified commands extensively.

Just stumbled over this while trying to build. Smells like missing Makefile dependencies (works just fine with -j1), but I figured I'd give you a heads up on this. It's a fresh checkout with nothing done but ./configure; make -j.

agraf@e77:/dev/shm/qemu> make -j
  GEN   i386-softmmu/config-devices.mak
  GEN   arm-softmmu/config-devices.mak
  GEN   x86_64-softmmu/config-devices.mak
  GEN   cris-softmmu/config-devices.mak
  GEN   lm32-softmmu/config-devices.mak
  GEN   m68k-softmmu/config-devices.mak
  GEN   microblaze-softmmu/config-devices.mak
  GEN   microblazeel-softmmu/config-devices.mak
  GEN   mips-softmmu/config-devices.mak
  GEN   mipsel-softmmu/config-devices.mak
  GEN   mips64-softmmu/config-devices.mak
  GEN   mips64el-softmmu/config-devices.mak
  GEN   ppcemb-softmmu/config-devices.mak
  GEN   ppc-softmmu/config-devices.mak
  GEN   ppc64-softmmu/config-devices.mak
  GEN   sh4-softmmu/config-devices.mak
  GEN   sh4eb-softmmu/config-devices.mak
  GEN   sparc-softmmu/config-devices.mak
  GEN   sparc64-softmmu/config-devices.mak
  GEN   i386-linux-user/config-devices.mak
  GEN   s390x-softmmu/config-devices.mak
  GEN   x86_64-linux-user/config-devices.mak
  GEN   alpha-linux-user/config-devices.mak
  GEN   arm-linux-user/config-devices.mak
  GEN   armeb-linux-user/config-devices.mak
  GEN   cris-linux-user/config-devices.mak
  GEN   m68k-linux-user/config-devices.mak
  GEN   microblaze-linux-user/config-devices.mak
  GEN   microblazeel-linux-user/config-devices.mak
  GEN   mips-linux-user/config-devices.mak
  GEN   mipsel-linux-user/config-devices.mak
  GEN   ppc-linux-user/config-devices.mak
  GEN   ppc64-linux-user/config-devices.mak
  GEN   sh4-linux-user/config-devices.mak
  GEN   ppc64abi32-linux-user/config-devices.mak
  GEN   sh4eb-linux-user/config-devices.mak
  GEN   sparc64-linux-user/config-devices.mak
  GEN   sparc32plus-linux-user/config-devices.mak
  GEN   unicore32-linux-user/config-devices.mak
  GEN   sparc-linux-user/config-devices.mak
  GEN   s390x-linux-user/config-devices.mak
  GEN   config-all-devices.mak
  GEN   config-host.h
  GEN   trace.h
  GEN   qemu-options.def
  GEN   qapi-generated/qga-qapi-visit.h
  GEN   trace.c
  CC    trace/default.o
  CC    trace/control.o
  GEN   qemu-img-cmds.h
cc1: error: qapi-generated: No such file or directory
cc1: error: qapi-generated: No such file or directory
make: *** [trace/default.o] Error 1
make: *** Waiting for unfinished jobs....
make: *** [trace/control.o] Error 1

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
  2011-09-07 21:56 ` [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Alexander Graf
@ 2011-09-07 22:03   ` Anthony Liguori
  2011-09-07 22:04     ` Alexander Graf
  0 siblings, 1 reply; 27+ messages in thread
From: Anthony Liguori @ 2011-09-07 22:03 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Kevin Wolf, Michael Roth, Anthony Liguori, qemu-devel, Luiz Capitulino

On 09/07/2011 04:56 PM, Alexander Graf wrote:
>
> On 02.09.2011, at 19:34, Anthony Liguori wrote:
>
>> This is my attempt to jump-start the QAPI switch over.  All of the hard work
>> is already done in my qapi branch, we just need to start merging stuff.
>>
>> To simplify the merge process, I've introduced a new mode to the code generator
>> that lets us do conversions without using the new QMP server.
>>
>> Once this series is merged, anything that touchs QMP (to modify a command or
>> add a new command) must do it through QAPI--no exceptions.
>>
>> This series also includes Dans change to the 'change' command.  I'm not thrilled
>> about people using this command but its better to err on the side of caution.
>> It's now officially deprecated with much more robust replacement commands.
>>
>> Since v1, I've tried to address all review comments.  I've also tested the
>> modified commands extensively.
>
> Just stumbled over this while trying to build. Smells like missing Makefile dependencies (works just fine with -j1), but I figured I'd give you a heads up on this. It's a fresh checkout with nothing done but ./configure; make -j.
>
>
> agraf@e77:/dev/shm/qemu>  make -j
>    GEN   i386-softmmu/config-devices.mak
>    GEN   arm-softmmu/config-devices.mak
>    GEN   x86_64-softmmu/config-devices.mak
>    GEN   cris-softmmu/config-devices.mak
>    GEN   lm32-softmmu/config-devices.mak
>    GEN   m68k-softmmu/config-devices.mak
>    GEN   microblaze-softmmu/config-devices.mak
>    GEN   microblazeel-softmmu/config-devices.mak
>    GEN   mips-softmmu/config-devices.mak
>    GEN   mipsel-softmmu/config-devices.mak
>    GEN   mips64-softmmu/config-devices.mak
>    GEN   mips64el-softmmu/config-devices.mak
>    GEN   ppcemb-softmmu/config-devices.mak
>    GEN   ppc-softmmu/config-devices.mak
>    GEN   ppc64-softmmu/config-devices.mak
>    GEN   sh4-softmmu/config-devices.mak
>    GEN   sh4eb-softmmu/config-devices.mak
>    GEN   sparc-softmmu/config-devices.mak
>    GEN   sparc64-softmmu/config-devices.mak
>    GEN   i386-linux-user/config-devices.mak
>    GEN   s390x-softmmu/config-devices.mak
>    GEN   x86_64-linux-user/config-devices.mak
>    GEN   alpha-linux-user/config-devices.mak
>    GEN   arm-linux-user/config-devices.mak
>    GEN   armeb-linux-user/config-devices.mak
>    GEN   cris-linux-user/config-devices.mak
>    GEN   m68k-linux-user/config-devices.mak
>    GEN   microblaze-linux-user/config-devices.mak
>    GEN   microblazeel-linux-user/config-devices.mak
>    GEN   mips-linux-user/config-devices.mak
>    GEN   mipsel-linux-user/config-devices.mak
>    GEN   ppc-linux-user/config-devices.mak
>    GEN   ppc64-linux-user/config-devices.mak
>    GEN   sh4-linux-user/config-devices.mak
>    GEN   ppc64abi32-linux-user/config-devices.mak
>    GEN   sh4eb-linux-user/config-devices.mak
>    GEN   sparc64-linux-user/config-devices.mak
>    GEN   sparc32plus-linux-user/config-devices.mak
>    GEN   unicore32-linux-user/config-devices.mak
>    GEN   sparc-linux-user/config-devices.mak
>    GEN   s390x-linux-user/config-devices.mak
>    GEN   config-all-devices.mak
>    GEN   config-host.h
>    GEN   trace.h
>    GEN   qemu-options.def
>    GEN   qapi-generated/qga-qapi-visit.h
>    GEN   trace.c
>    CC    trace/default.o
>    CC    trace/control.o
>    GEN   qemu-img-cmds.h
> cc1: error: qapi-generated: No such file or directory
> cc1: error: qapi-generated: No such file or directory

I've seen this before too but this is due to guest-agent, not this patch 
series.

Mike, have you seen this before?

Regards,

Anthony Liguori

> make: *** [trace/default.o] Error 1
> make: *** Waiting for unfinished jobs....
> make: *** [trace/control.o] Error 1
>
>
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
  2011-09-07 22:03   ` Anthony Liguori
@ 2011-09-07 22:04     ` Alexander Graf
  2011-09-07 22:24       ` Anthony Liguori
  0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2011-09-07 22:04 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: Kevin Wolf, Michael Roth, Anthony Liguori, qemu-devel, Luiz Capitulino


On 08.09.2011, at 00:03, Anthony Liguori wrote:

> On 09/07/2011 04:56 PM, Alexander Graf wrote:
>> 
>> On 02.09.2011, at 19:34, Anthony Liguori wrote:
>> 
>>> This is my attempt to jump-start the QAPI switch over.  All of the hard work
>>> is already done in my qapi branch, we just need to start merging stuff.
>>> 
>>> To simplify the merge process, I've introduced a new mode to the code generator
>>> that lets us do conversions without using the new QMP server.
>>> 
>>> Once this series is merged, anything that touchs QMP (to modify a command or
>>> add a new command) must do it through QAPI--no exceptions.
>>> 
>>> This series also includes Dans change to the 'change' command.  I'm not thrilled
>>> about people using this command but its better to err on the side of caution.
>>> It's now officially deprecated with much more robust replacement commands.
>>> 
>>> Since v1, I've tried to address all review comments.  I've also tested the
>>> modified commands extensively.
>> 
>> Just stumbled over this while trying to build. Smells like missing Makefile dependencies (works just fine with -j1), but I figured I'd give you a heads up on this. It's a fresh checkout with nothing done but ./configure; make -j.
>> 
>> 
>> agraf@e77:/dev/shm/qemu>  make -j
>>   GEN   i386-softmmu/config-devices.mak
>>   GEN   arm-softmmu/config-devices.mak
>>   GEN   x86_64-softmmu/config-devices.mak
>>   GEN   cris-softmmu/config-devices.mak
>>   GEN   lm32-softmmu/config-devices.mak
>>   GEN   m68k-softmmu/config-devices.mak
>>   GEN   microblaze-softmmu/config-devices.mak
>>   GEN   microblazeel-softmmu/config-devices.mak
>>   GEN   mips-softmmu/config-devices.mak
>>   GEN   mipsel-softmmu/config-devices.mak
>>   GEN   mips64-softmmu/config-devices.mak
>>   GEN   mips64el-softmmu/config-devices.mak
>>   GEN   ppcemb-softmmu/config-devices.mak
>>   GEN   ppc-softmmu/config-devices.mak
>>   GEN   ppc64-softmmu/config-devices.mak
>>   GEN   sh4-softmmu/config-devices.mak
>>   GEN   sh4eb-softmmu/config-devices.mak
>>   GEN   sparc-softmmu/config-devices.mak
>>   GEN   sparc64-softmmu/config-devices.mak
>>   GEN   i386-linux-user/config-devices.mak
>>   GEN   s390x-softmmu/config-devices.mak
>>   GEN   x86_64-linux-user/config-devices.mak
>>   GEN   alpha-linux-user/config-devices.mak
>>   GEN   arm-linux-user/config-devices.mak
>>   GEN   armeb-linux-user/config-devices.mak
>>   GEN   cris-linux-user/config-devices.mak
>>   GEN   m68k-linux-user/config-devices.mak
>>   GEN   microblaze-linux-user/config-devices.mak
>>   GEN   microblazeel-linux-user/config-devices.mak
>>   GEN   mips-linux-user/config-devices.mak
>>   GEN   mipsel-linux-user/config-devices.mak
>>   GEN   ppc-linux-user/config-devices.mak
>>   GEN   ppc64-linux-user/config-devices.mak
>>   GEN   sh4-linux-user/config-devices.mak
>>   GEN   ppc64abi32-linux-user/config-devices.mak
>>   GEN   sh4eb-linux-user/config-devices.mak
>>   GEN   sparc64-linux-user/config-devices.mak
>>   GEN   sparc32plus-linux-user/config-devices.mak
>>   GEN   unicore32-linux-user/config-devices.mak
>>   GEN   sparc-linux-user/config-devices.mak
>>   GEN   s390x-linux-user/config-devices.mak
>>   GEN   config-all-devices.mak
>>   GEN   config-host.h
>>   GEN   trace.h
>>   GEN   qemu-options.def
>>   GEN   qapi-generated/qga-qapi-visit.h
>>   GEN   trace.c
>>   CC    trace/default.o
>>   CC    trace/control.o
>>   GEN   qemu-img-cmds.h
>> cc1: error: qapi-generated: No such file or directory
>> cc1: error: qapi-generated: No such file or directory
> 
> I've seen this before too but this is due to guest-agent, not this patch series.

Ah, sorry, didn't track down what exactly was going wrong where and figured since it fails on qapi, it'd be related :)

Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
  2011-09-07 22:04     ` Alexander Graf
@ 2011-09-07 22:24       ` Anthony Liguori
  2011-09-07 23:12         ` Michael Roth
  0 siblings, 1 reply; 27+ messages in thread
From: Anthony Liguori @ 2011-09-07 22:24 UTC (permalink / raw)
  To: Alexander Graf; +Cc: Kevin Wolf, Michael Roth, qemu-devel, Luiz Capitulino

On 09/07/2011 05:04 PM, Alexander Graf wrote:
>
> On 08.09.2011, at 00:03, Anthony Liguori wrote:
>
>> On 09/07/2011 04:56 PM, Alexander Graf wrote:
>>>
>>> On 02.09.2011, at 19:34, Anthony Liguori wrote:
>>>
>>>> This is my attempt to jump-start the QAPI switch over.  All of the hard work
>>>> is already done in my qapi branch, we just need to start merging stuff.
>>>>
>>>> To simplify the merge process, I've introduced a new mode to the code generator
>>>> that lets us do conversions without using the new QMP server.
>>>>
>>>> Once this series is merged, anything that touchs QMP (to modify a command or
>>>> add a new command) must do it through QAPI--no exceptions.
>>>>
>>>> This series also includes Dans change to the 'change' command.  I'm not thrilled
>>>> about people using this command but its better to err on the side of caution.
>>>> It's now officially deprecated with much more robust replacement commands.
>>>>
>>>> Since v1, I've tried to address all review comments.  I've also tested the
>>>> modified commands extensively.
>>>
>>> Just stumbled over this while trying to build. Smells like missing Makefile dependencies (works just fine with -j1), but I figured I'd give you a heads up on this. It's a fresh checkout with nothing done but ./configure; make -j.
>>>
>>>
>>> agraf@e77:/dev/shm/qemu>   make -j
>>>    GEN   i386-softmmu/config-devices.mak
>>>    GEN   arm-softmmu/config-devices.mak
>>>    GEN   x86_64-softmmu/config-devices.mak
>>>    GEN   cris-softmmu/config-devices.mak
>>>    GEN   lm32-softmmu/config-devices.mak
>>>    GEN   m68k-softmmu/config-devices.mak
>>>    GEN   microblaze-softmmu/config-devices.mak
>>>    GEN   microblazeel-softmmu/config-devices.mak
>>>    GEN   mips-softmmu/config-devices.mak
>>>    GEN   mipsel-softmmu/config-devices.mak
>>>    GEN   mips64-softmmu/config-devices.mak
>>>    GEN   mips64el-softmmu/config-devices.mak
>>>    GEN   ppcemb-softmmu/config-devices.mak
>>>    GEN   ppc-softmmu/config-devices.mak
>>>    GEN   ppc64-softmmu/config-devices.mak
>>>    GEN   sh4-softmmu/config-devices.mak
>>>    GEN   sh4eb-softmmu/config-devices.mak
>>>    GEN   sparc-softmmu/config-devices.mak
>>>    GEN   sparc64-softmmu/config-devices.mak
>>>    GEN   i386-linux-user/config-devices.mak
>>>    GEN   s390x-softmmu/config-devices.mak
>>>    GEN   x86_64-linux-user/config-devices.mak
>>>    GEN   alpha-linux-user/config-devices.mak
>>>    GEN   arm-linux-user/config-devices.mak
>>>    GEN   armeb-linux-user/config-devices.mak
>>>    GEN   cris-linux-user/config-devices.mak
>>>    GEN   m68k-linux-user/config-devices.mak
>>>    GEN   microblaze-linux-user/config-devices.mak
>>>    GEN   microblazeel-linux-user/config-devices.mak
>>>    GEN   mips-linux-user/config-devices.mak
>>>    GEN   mipsel-linux-user/config-devices.mak
>>>    GEN   ppc-linux-user/config-devices.mak
>>>    GEN   ppc64-linux-user/config-devices.mak
>>>    GEN   sh4-linux-user/config-devices.mak
>>>    GEN   ppc64abi32-linux-user/config-devices.mak
>>>    GEN   sh4eb-linux-user/config-devices.mak
>>>    GEN   sparc64-linux-user/config-devices.mak
>>>    GEN   sparc32plus-linux-user/config-devices.mak
>>>    GEN   unicore32-linux-user/config-devices.mak
>>>    GEN   sparc-linux-user/config-devices.mak
>>>    GEN   s390x-linux-user/config-devices.mak
>>>    GEN   config-all-devices.mak
>>>    GEN   config-host.h
>>>    GEN   trace.h
>>>    GEN   qemu-options.def
>>>    GEN   qapi-generated/qga-qapi-visit.h
>>>    GEN   trace.c
>>>    CC    trace/default.o
>>>    CC    trace/control.o
>>>    GEN   qemu-img-cmds.h
>>> cc1: error: qapi-generated: No such file or directory
>>> cc1: error: qapi-generated: No such file or directory
>>
>> I've seen this before too but this is due to guest-agent, not this patch series.
>
> Ah, sorry, didn't track down what exactly was going wrong where and figured since it fails on qapi, it'd be related :)

I assume the root cause is that qapi-generated is being generated by 
make instead of by configure.  Mike, care to throw together a quick 
patch to change that?  Then there's no guess work tracking down 
dependencies.

Regards,

Anthony Liguori

> Alex
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2)
  2011-09-07 22:24       ` Anthony Liguori
@ 2011-09-07 23:12         ` Michael Roth
  0 siblings, 0 replies; 27+ messages in thread
From: Michael Roth @ 2011-09-07 23:12 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, Alexander Graf, Luiz Capitulino

On 09/07/2011 05:24 PM, Anthony Liguori wrote:
> On 09/07/2011 05:04 PM, Alexander Graf wrote:
>>
>> On 08.09.2011, at 00:03, Anthony Liguori wrote:
>>
>>> On 09/07/2011 04:56 PM, Alexander Graf wrote:
>>>>
>>>> On 02.09.2011, at 19:34, Anthony Liguori wrote:
>>>>
>>>>> This is my attempt to jump-start the QAPI switch over. All of the
>>>>> hard work
>>>>> is already done in my qapi branch, we just need to start merging
>>>>> stuff.
>>>>>
>>>>> To simplify the merge process, I've introduced a new mode to the
>>>>> code generator
>>>>> that lets us do conversions without using the new QMP server.
>>>>>
>>>>> Once this series is merged, anything that touchs QMP (to modify a
>>>>> command or
>>>>> add a new command) must do it through QAPI--no exceptions.
>>>>>
>>>>> This series also includes Dans change to the 'change' command. I'm
>>>>> not thrilled
>>>>> about people using this command but its better to err on the side
>>>>> of caution.
>>>>> It's now officially deprecated with much more robust replacement
>>>>> commands.
>>>>>
>>>>> Since v1, I've tried to address all review comments. I've also
>>>>> tested the
>>>>> modified commands extensively.
>>>>
>>>> Just stumbled over this while trying to build. Smells like missing
>>>> Makefile dependencies (works just fine with -j1), but I figured I'd
>>>> give you a heads up on this. It's a fresh checkout with nothing done
>>>> but ./configure; make -j.
>>>>
>>>>
>>>> agraf@e77:/dev/shm/qemu> make -j
>>>> GEN i386-softmmu/config-devices.mak
>>>> GEN arm-softmmu/config-devices.mak
>>>> GEN x86_64-softmmu/config-devices.mak
>>>> GEN cris-softmmu/config-devices.mak
>>>> GEN lm32-softmmu/config-devices.mak
>>>> GEN m68k-softmmu/config-devices.mak
>>>> GEN microblaze-softmmu/config-devices.mak
>>>> GEN microblazeel-softmmu/config-devices.mak
>>>> GEN mips-softmmu/config-devices.mak
>>>> GEN mipsel-softmmu/config-devices.mak
>>>> GEN mips64-softmmu/config-devices.mak
>>>> GEN mips64el-softmmu/config-devices.mak
>>>> GEN ppcemb-softmmu/config-devices.mak
>>>> GEN ppc-softmmu/config-devices.mak
>>>> GEN ppc64-softmmu/config-devices.mak
>>>> GEN sh4-softmmu/config-devices.mak
>>>> GEN sh4eb-softmmu/config-devices.mak
>>>> GEN sparc-softmmu/config-devices.mak
>>>> GEN sparc64-softmmu/config-devices.mak
>>>> GEN i386-linux-user/config-devices.mak
>>>> GEN s390x-softmmu/config-devices.mak
>>>> GEN x86_64-linux-user/config-devices.mak
>>>> GEN alpha-linux-user/config-devices.mak
>>>> GEN arm-linux-user/config-devices.mak
>>>> GEN armeb-linux-user/config-devices.mak
>>>> GEN cris-linux-user/config-devices.mak
>>>> GEN m68k-linux-user/config-devices.mak
>>>> GEN microblaze-linux-user/config-devices.mak
>>>> GEN microblazeel-linux-user/config-devices.mak
>>>> GEN mips-linux-user/config-devices.mak
>>>> GEN mipsel-linux-user/config-devices.mak
>>>> GEN ppc-linux-user/config-devices.mak
>>>> GEN ppc64-linux-user/config-devices.mak
>>>> GEN sh4-linux-user/config-devices.mak
>>>> GEN ppc64abi32-linux-user/config-devices.mak
>>>> GEN sh4eb-linux-user/config-devices.mak
>>>> GEN sparc64-linux-user/config-devices.mak
>>>> GEN sparc32plus-linux-user/config-devices.mak
>>>> GEN unicore32-linux-user/config-devices.mak
>>>> GEN sparc-linux-user/config-devices.mak
>>>> GEN s390x-linux-user/config-devices.mak
>>>> GEN config-all-devices.mak
>>>> GEN config-host.h
>>>> GEN trace.h
>>>> GEN qemu-options.def
>>>> GEN qapi-generated/qga-qapi-visit.h
>>>> GEN trace.c
>>>> CC trace/default.o
>>>> CC trace/control.o
>>>> GEN qemu-img-cmds.h
>>>> cc1: error: qapi-generated: No such file or directory
>>>> cc1: error: qapi-generated: No such file or directory
>>>
>>> I've seen this before too but this is due to guest-agent, not this
>>> patch series.
>>
>> Ah, sorry, didn't track down what exactly was going wrong where and
>> figured since it fails on qapi, it'd be related :)
>
> I assume the root cause is that qapi-generated is being generated by
> make instead of by configure. Mike, care to throw together a quick patch

Yah, QGA's code generators create qapi-generated/, and that code doesn't 
execute until a dependency on GENERATED_HEADERS is met. So it seems 
there was a pre-existing race to begin with, which got exacerbated by 
e4858974ec36afd8a6b3a9e2b0ad8f357f28efc7 which is missing some deps on 
GENERATED_HEADERS, so it got in more ahead of the code generation 
scripts than we'd seen previously.

> to change that? Then there's no guess work tracking down dependencies.
>

Sure, ill send that in a few. Looks like will still get make -j build 
failures from that tracing patch though so I'll look at that as well.

> Regards,
>
> Anthony Liguori
>
>> Alex
>>
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2011-09-12  9:28 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-02 17:34 [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 01/15] error: let error_is_type take a NULL error Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 02/15] qerror: add qerror_report_err() (v2) Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 03/15] qapi: add code generation support for middle mode Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 04/15] qapi: use middle mode in QMP server (v2) Anthony Liguori
2011-09-02 20:39   ` Luiz Capitulino
2011-09-02 17:34 ` [Qemu-devel] [PATCH 05/15] qapi: convert query-name Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 06/15] block: add unsafe_probe Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 07/15] monitor: expose readline state Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 08/15] qerror: add additional parameter to QERR_DEVICE_ENCRYPTED Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 09/15] qapi: convert eject (qmp and hmp) to QAPI Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 10/15] qapi: convert block_passwd and add set-blockdev-password Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 11/15] qapi: add change-vnc-password (v2) Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2) Anthony Liguori
2011-09-02 20:50   ` Luiz Capitulino
2011-09-12  9:17     ` Daniel P. Berrange
2011-09-12  9:28       ` Daniel P. Berrange
2011-09-02 17:34 ` [Qemu-devel] [PATCH 13/15] qapi: introduce drive-change (v2) Anthony Liguori
2011-09-02 21:06   ` Luiz Capitulino
2011-09-02 21:10     ` Luiz Capitulino
2011-09-02 17:34 ` [Qemu-devel] [PATCH 14/15] qapi: convert change (v2) Anthony Liguori
2011-09-02 17:34 ` [Qemu-devel] [PATCH 15/15] vnc: don't demote authentication protocol when disabling login Anthony Liguori
2011-09-07 21:56 ` [Qemu-devel] [PATCH 00/15] Convert commands to QAPI (batch 1) (v2) Alexander Graf
2011-09-07 22:03   ` Anthony Liguori
2011-09-07 22:04     ` Alexander Graf
2011-09-07 22:24       ` Anthony Liguori
2011-09-07 23:12         ` Michael Roth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.