Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Paolo Bonzini pbonz...@redhat.com writes: Il 16/01/2014 03:50, Michael Roth ha scritto: If we go to that effort, it may make sense to try to re-license to GPLv2+ while we're at it, but either way I think this should be done as a separate patchset, and shouldn't hold up Wenchao's series. I can send that out, since it's my screw-up. Removing Red Hat, Blue Swirl and Stefan Weil, who have agreed on a blanket relicensing of GPLv2-only code to GPLv2+ leaves only half a dozen people: Anthony Liguori: anth...@codemonkey.ws (IBM?) Michael Roth: mdr...@linux.vnet.ibm.com (IBM?) Peter Maydell: peter.mayd...@linaro.org Richard Henderson: r...@twiddle.net Tomoki Sekiyama: tomoki.sekiy...@hds.com I sent a patch. Now we hunt for the required ACKs.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Il 16/01/2014 03:50, Michael Roth ha scritto: If we go to that effort, it may make sense to try to re-license to GPLv2+ while we're at it, but either way I think this should be done as a separate patchset, and shouldn't hold up Wenchao's series. I can send that out, since it's my screw-up. Removing Red Hat, Blue Swirl and Stefan Weil, who have agreed on a blanket relicensing of GPLv2-only code to GPLv2+ leaves only half a dozen people: Anthony Liguori: anth...@codemonkey.ws (IBM?) Michael Roth: mdr...@linux.vnet.ibm.com (IBM?) Peter Maydell: peter.mayd...@linaro.org Richard Henderson: r...@twiddle.net Tomoki Sekiyama: tomoki.sekiy...@hds.com Paolo
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Quoting Markus Armbruster (2013-12-16 03:13:08) [Licensing problem, cc: Anthony] Kevin Wolf kw...@redhat.com writes: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. Specifically: FileCommit scripts/qapi-commands.pyc17d9908 scripts/qapi-visit.py fb3182ce scripts/qapi-types.py 06d64c62 scripts/qapi.py 0f923be2 All four from Michael Roth via Luiz. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. IANAL, and I wouldn't dare to judge which of the two conflicting license claims takes precedence. Possibly neither, and then the files might technically not be distributable. IAAlsoNAL, but GPLv2 is explicit, whereas the COPYING.LIB simply references a document with no information relevant to GPLv2, so I think a strong case can be made that the intended license was GPLv2 and the clarification is effectively a no-op. Anyway, this mess needs to be addressed. Michael, what was your *intended* license? GPLv2 was my intention at least (I meant to reference COPYING). But I think we need Anthony's ack to be certain, since he was the original author, and I added the screwed up license header after-the-fact under the assumption that the code was to be GPLv2. Here's the original: http://repo.or.cz/w/qemu/aliguori.git/blob_plain/glib:/scripts/qapi-types.py If it wasn't GPLv2+, then why? This was committed prior to the push to switch to GPLv2+, but I'm fine with relicensing my contributions as GPLv2+ should we opt to do so, but I think that's a separate issue. Do we need formal ACKs from all contributors to fix the licensing comment in these four files? If we were actually re-licensing then yes (at least, that's what we've done in the past). To clarify the existing license maybe not, but we should probably err on the side of caution. Current list seems to be: mdroth@loki:~/w/qemu.git$ git log --format=%an: %ae scripts/qapi* | sort | uniq Amos Kong: ak...@redhat.com Anthony Liguori: aligu...@us.ibm.com Anthony Liguori: anth...@codemonkey.ws Avi Kivity: a...@redhat.com Blue Swirl: blauwir...@gmail.com Cole Robinson: crobi...@redhat.com Federico Simoncelli: fsimo...@redhat.com Igor Mammedov: imamm...@redhat.com Kevin Wolf: kw...@redhat.com Laszlo Ersek: ler...@redhat.com Luiz Capitulino: lcapitul...@redhat.com Markus Armbruster: arm...@redhat.com Michael Roth: mdr...@linux.vnet.ibm.com Paolo Bonzini: pbonz...@redhat.com Peter Maydell: peter.mayd...@linaro.org Richard Henderson: r...@twiddle.net Stefan Weil: s...@weilnetz.de Tomoki Sekiyama: tomoki.sekiy...@hds.com If we go to that effort, it may make sense to try to re-license to GPLv2+ while we're at it, but either way I think this should be done as a separate patchset, and shouldn't hold up Wenchao's series. I can send that out, since it's my screw-up.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
于 2014/1/13 18:08, Markus Armbruster 写道: Ping^2! Markus Armbruster arm...@redhat.com writes: Ping? Markus Armbruster arm...@redhat.com writes: [Licensing problem, cc: Anthony] Kevin Wolf kw...@redhat.com writes: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. Specifically: FileCommit scripts/qapi-commands.pyc17d9908 scripts/qapi-visit.py fb3182ce scripts/qapi-types.py 06d64c62 scripts/qapi.py 0f923be2 All four from Michael Roth via Luiz. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. IANAL, and I wouldn't dare to judge which of the two conflicting license claims takes precedence. Possibly neither, and then the files might technically not be distributable. Anyway, this mess needs to be addressed. Michael, what was your *intended* license? If it wasn't GPLv2+, then why? Do we need formal ACKs from all contributors to fix the licensing comment in these four files? I used GPLv2+ in my new files of v2, but not sure about other files. Michael, do you think other scripts should be changed either?
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Ping^2! Markus Armbruster arm...@redhat.com writes: Ping? Markus Armbruster arm...@redhat.com writes: [Licensing problem, cc: Anthony] Kevin Wolf kw...@redhat.com writes: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. Specifically: FileCommit scripts/qapi-commands.pyc17d9908 scripts/qapi-visit.py fb3182ce scripts/qapi-types.py 06d64c62 scripts/qapi.py 0f923be2 All four from Michael Roth via Luiz. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. IANAL, and I wouldn't dare to judge which of the two conflicting license claims takes precedence. Possibly neither, and then the files might technically not be distributable. Anyway, this mess needs to be addressed. Michael, what was your *intended* license? If it wasn't GPLv2+, then why? Do we need formal ACKs from all contributors to fix the licensing comment in these four files?
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Ping? Markus Armbruster arm...@redhat.com writes: [Licensing problem, cc: Anthony] Kevin Wolf kw...@redhat.com writes: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. Specifically: FileCommit scripts/qapi-commands.pyc17d9908 scripts/qapi-visit.py fb3182ce scripts/qapi-types.py 06d64c62 scripts/qapi.py 0f923be2 All four from Michael Roth via Luiz. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. IANAL, and I wouldn't dare to judge which of the two conflicting license claims takes precedence. Possibly neither, and then the files might technically not be distributable. Anyway, this mess needs to be addressed. Michael, what was your *intended* license? If it wasn't GPLv2+, then why? Do we need formal ACKs from all contributors to fix the licensing comment in these four files?
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Wenchao Xia xiaw...@linux.vnet.ibm.com writes: 于 2013/12/13 21:43, Kevin Wolf 写道: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. Kevin ah..I am bad in license problem, will use the doc as LGPL from other file. Please use GPLv2+ unless you have a specific reason for another license.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
[Licensing problem, cc: Anthony] Kevin Wolf kw...@redhat.com writes: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. Specifically: FileCommit scripts/qapi-commands.pyc17d9908 scripts/qapi-visit.py fb3182ce scripts/qapi-types.py 06d64c62 scripts/qapi.py 0f923be2 All four from Michael Roth via Luiz. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. IANAL, and I wouldn't dare to judge which of the two conflicting license claims takes precedence. Possibly neither, and then the files might technically not be distributable. Anyway, this mess needs to be addressed. Michael, what was your *intended* license? If it wasn't GPLv2+, then why? Do we need formal ACKs from all contributors to fix the licensing comment in these four files?
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
于 2013/12/13 21:43, Kevin Wolf 写道: Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. Kevin ah..I am bad in license problem, will use the doc as LGPL from other file.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
于 2013/12/13 21:31, Eric Blake 写道: On 11/12/2013 06:44 PM, Wenchao Xia wrote: Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } But what IS valid? You need to document this in docs/qapi-code-gen.txt at a bare minimum. This patch series is hard to review, and still has the RFC subject line. At this point, I think it's worth rebasing and resending what you have; even if it needs more review, it will at least get it to a state that is easier to review. Sure, I will wait some time and respin. +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Am 13.12.2013 um 14:31 hat Eric Blake geschrieben: On 11/12/2013 06:44 PM, Wenchao Xia wrote: +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. In fact, it's even worse: +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. These two lines contradict each other, COPYING.LIB contains the LGPL 2.1. The same bad license header is in the other QAPI generator scripts, so it's only copypaste here. This doesn't make things easier, because if things are copied, the license of the source must be respected. And it seems rather dubious to me what this license actually is. If it's GPLv2-only, we can't just change it in the new copy. Kevin pgpPXoh_N4hV5.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
On 11/12/2013 06:44 PM, Wenchao Xia wrote: Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } But what IS valid? You need to document this in docs/qapi-code-gen.txt at a bare minimum. This patch series is hard to review, and still has the RFC subject line. At this point, I think it's worth rebasing and resending what you have; even if it needs more review, it will at least get it to a state that is easier to review. +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. Can you please use GPLv2+ (that is, add the or later clause)? We already have GPLv2-only code, but I don't want to increase the size of that unfortunate license choice. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
于 2013/12/2 14:48, Wenchao Xia 写道: + +if (!qapi_event_functions.emit) { Better to return an error here instead of silently failing. The purpose is allowing emit=NULL and skip event code in that case. But the code will do nothing and the caller won't know that. Now the caller also won't know that useless code will be executed, when qemu-img link with stub of monitor_event functions. :) Actually, I wonder if the code should even abort() in such a case, as emit=NULL would be a programming today. I am not sure why the code should always do something. The code may actually take CPU resource to do nothing meanful, such as build up a qdict and release it later, when emit is not a valid function. So I did this as an improvement: check emit function ahead to escape useless work. Luiz, do you agree with me?
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
+ +if (!qapi_event_functions.emit) { Better to return an error here instead of silently failing. The purpose is allowing emit=NULL and skip event code in that case. But the code will do nothing and the caller won't know that. Now the caller also won't know that useless code will be executed, when qemu-img link with stub of monitor_event functions. :) Actually, I wonder if the code should even abort() in such a case, as emit=NULL would be a programming today. I am not sure why the code should always do something. The code may actually take CPU resource to do nothing meanful, such as build up a qdict and release it later, when emit is not a valid function. So I did this as an improvement: check emit function ahead to escape useless work.
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
On Thu, 28 Nov 2013 15:16:08 +0800 Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: 于 2013/11/28 8:48, Luiz Capitulino 写道: On Wed, 13 Nov 2013 09:44:52 +0800 Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } I think your general approach is reasonable, but there are a number of details to fix. The first one is documentation. This patch's changelog is quite bad, you don't say anything about what the does. You just mention a corner case which doesn't happen to work. Please, add full changelog explaining what this patch is about and please add examples of how an event entry would look like in the schema and maybe you could also add the important parts of a generated event function. Also, please add a event section to docs/writing-qmp-commands.txt (in a different patch). OK. Secondly, for changes like this it's a very good idea to provide conversion examples (as patches, not as changelog doc) at least one or two so that we can see how it will look like. Will add some in the intro part. By the way I think test cases in patch 3 shows a bit. I didn't look at the test to be honest (it didn't apply and I concentrated on the second patch). Having a test is awesome, but you still have to provide at least one conversion so that we can see how it will look like. More below. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile |9 +- Makefile.objs |2 +- qapi/Makefile.objs|1 + scripts/qapi-event.py | 355 + 4 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 scripts/qapi-event.py diff --git a/Makefile b/Makefile index b15003f..a3e465f 100644 --- a/Makefile +++ b/Makefile @@ -38,8 +38,8 @@ endif endif GENERATED_HEADERS = config-host.h qemu-options.def -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c GENERATED_HEADERS += trace/generated-events.h GENERATED_SOURCES += trace/generated-events.c @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y) # Build libraries libqemustub.a: $(stub-obj-y) -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o ## @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o . -b $, GEN $@) +qapi-event.c qapi-event.h :\ +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o . -b $, GEN $@) qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o . $, GEN $@) diff --git a/Makefile.objs b/Makefile.objs index 2b6c1fe..33f5950 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o block-obj-y += block/ -block-obj-y += qapi-types.o qapi-visit.o +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o block-obj-y += qemu-io-cmds.o block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs index 1f9c973..d14b769 100644 --- a/qapi/Makefile.objs +++ b/qapi/Makefile.objs @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o util-obj-y += string-input-visitor.o string-output-visitor.o util-obj-y += opts-visitor.o +util-obj-y += qmp-event.o diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py new file mode 100644 index 000..4c6a0fe I didn't review this hunk, but this series doesn't build for me: CCqapi/string-output-visitor.o CCqapi/opts-visitor.o make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'. Stop. ~/work/src/upstream/qmp-unstable/build (tmp|AM)/ A draft file I forgot to remove in Makefile, will fix. --- /dev/null +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +#
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
On Wed, 13 Nov 2013 09:44:52 +0800 Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } I think your general approach is reasonable, but there are a number of details to fix. The first one is documentation. This patch's changelog is quite bad, you don't say anything about what the does. You just mention a corner case which doesn't happen to work. Please, add full changelog explaining what this patch is about and please add examples of how an event entry would look like in the schema and maybe you could also add the important parts of a generated event function. Also, please add a event section to docs/writing-qmp-commands.txt (in a different patch). Secondly, for changes like this it's a very good idea to provide conversion examples (as patches, not as changelog doc) at least one or two so that we can see how it will look like. More below. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile |9 +- Makefile.objs |2 +- qapi/Makefile.objs|1 + scripts/qapi-event.py | 355 + 4 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 scripts/qapi-event.py diff --git a/Makefile b/Makefile index b15003f..a3e465f 100644 --- a/Makefile +++ b/Makefile @@ -38,8 +38,8 @@ endif endif GENERATED_HEADERS = config-host.h qemu-options.def -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c GENERATED_HEADERS += trace/generated-events.h GENERATED_SOURCES += trace/generated-events.c @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y) # Build libraries libqemustub.a: $(stub-obj-y) -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o ## @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o . -b $, GEN $@) +qapi-event.c qapi-event.h :\ +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o . -b $, GEN $@) qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o . $, GEN $@) diff --git a/Makefile.objs b/Makefile.objs index 2b6c1fe..33f5950 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o block-obj-y += block/ -block-obj-y += qapi-types.o qapi-visit.o +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o block-obj-y += qemu-io-cmds.o block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs index 1f9c973..d14b769 100644 --- a/qapi/Makefile.objs +++ b/qapi/Makefile.objs @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o util-obj-y += string-input-visitor.o string-output-visitor.o util-obj-y += opts-visitor.o +util-obj-y += qmp-event.o diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py new file mode 100644 index 000..4c6a0fe I didn't review this hunk, but this series doesn't build for me: CCqapi/string-output-visitor.o CCqapi/opts-visitor.o make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'. Stop. ~/work/src/upstream/qmp-unstable/build (tmp|AM)/ --- /dev/null +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. + +from ordereddict import OrderedDict +from qapi import * +import sys +import os +import getopt +import errno + +def _generate_event_api_name_with_params(event_name, params): +api_name = void qapi_event_gen_%s( % c_fun(event_name); I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME(). +l = len(api_name) + +for argname, argentry, optional, structured in parse_args(params): +if structured: +
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
于 2013/11/28 8:48, Luiz Capitulino 写道: On Wed, 13 Nov 2013 09:44:52 +0800 Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } I think your general approach is reasonable, but there are a number of details to fix. The first one is documentation. This patch's changelog is quite bad, you don't say anything about what the does. You just mention a corner case which doesn't happen to work. Please, add full changelog explaining what this patch is about and please add examples of how an event entry would look like in the schema and maybe you could also add the important parts of a generated event function. Also, please add a event section to docs/writing-qmp-commands.txt (in a different patch). OK. Secondly, for changes like this it's a very good idea to provide conversion examples (as patches, not as changelog doc) at least one or two so that we can see how it will look like. Will add some in the intro part. By the way I think test cases in patch 3 shows a bit. More below. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile |9 +- Makefile.objs |2 +- qapi/Makefile.objs|1 + scripts/qapi-event.py | 355 + 4 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 scripts/qapi-event.py diff --git a/Makefile b/Makefile index b15003f..a3e465f 100644 --- a/Makefile +++ b/Makefile @@ -38,8 +38,8 @@ endif endif GENERATED_HEADERS = config-host.h qemu-options.def -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c GENERATED_HEADERS += trace/generated-events.h GENERATED_SOURCES += trace/generated-events.c @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y) # Build libraries libqemustub.a: $(stub-obj-y) -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o ## @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o . -b $, GEN $@) +qapi-event.c qapi-event.h :\ +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o . -b $, GEN $@) qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o . $, GEN $@) diff --git a/Makefile.objs b/Makefile.objs index 2b6c1fe..33f5950 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o block-obj-y += block/ -block-obj-y += qapi-types.o qapi-visit.o +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o block-obj-y += qemu-io-cmds.o block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs index 1f9c973..d14b769 100644 --- a/qapi/Makefile.objs +++ b/qapi/Makefile.objs @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o util-obj-y += string-input-visitor.o string-output-visitor.o util-obj-y += opts-visitor.o +util-obj-y += qmp-event.o diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py new file mode 100644 index 000..4c6a0fe I didn't review this hunk, but this series doesn't build for me: CCqapi/string-output-visitor.o CCqapi/opts-visitor.o make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'. Stop. ~/work/src/upstream/qmp-unstable/build (tmp|AM)/ A draft file I forgot to remove in Makefile, will fix. --- /dev/null +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. + +from ordereddict import OrderedDict +from qapi import * +import sys +import os +import getopt +import errno + +def _generate_event_api_name_with_params(event_name, params): +api_name = void qapi_event_gen_%s( % c_fun(event_name); I'd prefer a name like qmp_notify_NAME() or
[Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Nested structure is not supported now, so following define is not valid: { 'event': 'EVENT_C', 'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' } Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile |9 +- Makefile.objs |2 +- qapi/Makefile.objs|1 + scripts/qapi-event.py | 355 + 4 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 scripts/qapi-event.py diff --git a/Makefile b/Makefile index b15003f..a3e465f 100644 --- a/Makefile +++ b/Makefile @@ -38,8 +38,8 @@ endif endif GENERATED_HEADERS = config-host.h qemu-options.def -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c GENERATED_HEADERS += trace/generated-events.h GENERATED_SOURCES += trace/generated-events.c @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y) # Build libraries libqemustub.a: $(stub-obj-y) -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o ## @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o . -b $, GEN $@) +qapi-event.c qapi-event.h :\ +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o . -b $, GEN $@) qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o . $, GEN $@) diff --git a/Makefile.objs b/Makefile.objs index 2b6c1fe..33f5950 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o block-obj-y += block/ -block-obj-y += qapi-types.o qapi-visit.o +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o block-obj-y += qemu-io-cmds.o block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs index 1f9c973..d14b769 100644 --- a/qapi/Makefile.objs +++ b/qapi/Makefile.objs @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o util-obj-y += string-input-visitor.o string-output-visitor.o util-obj-y += opts-visitor.o +util-obj-y += qmp-event.o diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py new file mode 100644 index 000..4c6a0fe --- /dev/null +++ b/scripts/qapi-event.py @@ -0,0 +1,355 @@ +# +# QAPI event generator +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# Wenchao Xia xiaw...@linux.vnet.ibm.com +# +# This work is licensed under the terms of the GNU GPLv2. +# See the COPYING.LIB file in the top-level directory. + +from ordereddict import OrderedDict +from qapi import * +import sys +import os +import getopt +import errno + +def _generate_event_api_name_with_params(event_name, params): +api_name = void qapi_event_gen_%s( % c_fun(event_name); +l = len(api_name) + +for argname, argentry, optional, structured in parse_args(params): +if structured: +sys.stderr.write(Nested structure define in event is not + supported now, event '%s', argname '%s'\n % + (event_name, argname)) +sys.exit(1) +continue + +if optional: +api_name += bool has_%s,\n % c_var(argname) +api_name += .ljust(l) + +if argentry == str: +api_name += const +api_name += %s %s,\n % (c_type(argentry), c_var(argname)) +api_name += .ljust(l) + +api_name += Error **errp) +return api_name; + +def _generate_event_implement_with_params(api_name, event_name, params): +ret = mcgen( + +%(api_name)s +{ +QmpOutputVisitor *qov; +Visitor *v; +Error *err = NULL; +QObject *obj; +QDict *qmp = NULL; + +if (!qapi_event_functions.emit) { +return; +} + +qmp = qdict_new(); +qdict_put(qmp, event, qstring_from_str(%(event_name)s)); +timestamp_put(qmp); + +qov = qmp_output_visitor_new(); +g_assert(qov); + +v = qmp_output_get_visitor(qov); +g_assert(v); + +/* Fake visit, as if all member are under a structure */ +visit_start_struct(v, NULL, , %(event_name)s, 0, err); +if (error_is_set(err)) { +goto clean; +} + +, +