Use Base64 to serialize the binary blobs in JSON.
So far at most 512 bytes will be transfered, which result
in a 684 bytes payload.
Since this command is intented for qtesting, it is acceptable.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
qapi-schema.json | 41 +++++++++++++++++++++++++++++++++
hw/sd/sdbus-qmp.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
stubs/qmp_sdbus.c | 12 ++++++++++
hw/sd/Makefile.objs | 2 +-
stubs/Makefile.objs | 1 +
5 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 hw/sd/sdbus-qmp.c
create mode 100644 stubs/qmp_sdbus.c
diff --git a/qapi-schema.json b/qapi-schema.json
index 18457954a8..be26e8cd34 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3200,3 +3200,44 @@
# Since: 2.11
##
{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
+
+##
+# @SDBusCommandResponse:
+#
+# SD Bus command response.
+#
+# @base64: the command response encoded as a Base64 string, if any (optional)
+#
+# Since: 2.11
+##
+{ 'struct': 'SDBusCommandResponse', 'data': {'*base64': 'str'} }
+
+##
+# @x-debug-sdbus-command:
+#
+# Execute a command on a SD Bus return the response (if any).
+#
+# @qom-path: the SD Bus path
+# @command: the SD protocol command to execute in the bus
+# @arg: a 64-bit command argument (optional)
+# @crc: the command/argument CRC (optional)
+#
+# Returns: the response of the command encoded as a Base64 string
+#
+# Since: 2.11
+#
+# -> { "execute": "x-debug-sdbus-command",
+# "arguments": { "qom-path": "/machine/unattached/device[32]/sd.0",
+# "command": 0x01
+# }
+# }
+# <- { "return": {'base64': 'A='} }
+#
+##
+{ 'command': 'x-debug-sdbus-command',
+ 'data': { 'qom-path': 'str',
+ 'command': 'uint8',
+ '*arg': 'uint64',
+ '*crc': 'uint16' },
+ 'returns': 'SDBusCommandResponse'
+}
diff --git a/hw/sd/sdbus-qmp.c b/hw/sd/sdbus-qmp.c
new file mode 100644
index 0000000000..8c4b6f2aee
--- /dev/null
+++ b/hw/sd/sdbus-qmp.c
@@ -0,0 +1,65 @@
+/*
+ * SD card bus QMP debugging interface (for QTesting).
+ *
+ * Copyright (c) 2017 ?
+ *
+ * Author:
+ * Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+#include "hw/sd/sd.h"
+#include "qmp-commands.h"
+
+SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
+ uint8_t command,
+ bool has_arg, uint64_t arg,
+ bool has_crc, uint16_t crc,
+ Error **errp)
+{
+ uint8_t response[16 + 1];
+ SDBusCommandResponse *res;
+ bool ambiguous = false;
+ Object *obj;
+ SDBus *sdbus;
+ int sz;
+
+ obj = object_resolve_path(qom_path, &ambiguous);
+ if (obj == NULL) {
+ if (ambiguous) {
+ error_setg(errp, "Path '%s' is ambiguous", qom_path);
+ } else {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ "Device '%s' not found", qom_path);
+ }
+ return NULL;
+ }
+ sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
+ if (sdbus == NULL) {
+ error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "Device '%s' not a sd-bus", qom_path);
+ return NULL;
+ }
+
+ res = g_new0(SDBusCommandResponse, 1);
+ sz = sdbus_do_command(sdbus,
+ &(SDRequest){ command, arg, has_crc ? crc : -1 },
+ response);
+ if (sz > 0) {
+ res->has_base64 = true;
+ res->base64 = g_base64_encode(response, sz);
+ }
+
+ return res;
+}
diff --git a/stubs/qmp_sdbus.c b/stubs/qmp_sdbus.c
new file mode 100644
index 0000000000..d9bd75ec71
--- /dev/null
+++ b/stubs/qmp_sdbus.c
@@ -0,0 +1,12 @@
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+#include "hw/sd/sd.h"
+
+SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
+ uint8_t command,
+ bool has_arg, uint64_t arg,
+ bool has_crc, uint16_t crc,
+ Error **errp)
+{
+ return NULL;
+}
diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
index c2b7664264..3a70477bba 100644
--- a/hw/sd/Makefile.objs
+++ b/hw/sd/Makefile.objs
@@ -1,6 +1,6 @@
common-obj-$(CONFIG_PL181) += pl181.o
common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
-common-obj-$(CONFIG_SD) += sd.o core.o
+common-obj-$(CONFIG_SD) += sd.o core.o sdbus-qmp.o
common-obj-$(CONFIG_SDHCI) += sdhci.o
obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8cfe34328a..a46cb3b992 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -35,6 +35,7 @@ stub-obj-y += vm-stop.o
stub-obj-y += vmstate.o
stub-obj-$(CONFIG_WIN32) += fd-register.o
stub-obj-y += qmp_pc_dimm.o
+stub-obj-y += qmp_sdbus.o
stub-obj-y += target-monitor-defs.o
stub-obj-y += target-get-monitor-def.o
stub-obj-y += pc_madt_cpu_entry.o
--
2.15.1
On Wed, Jan 03, 2018 at 06:49:22PM -0300, Philippe Mathieu-Daudé wrote: > Use Base64 to serialize the binary blobs in JSON. > So far at most 512 bytes will be transfered, which result > in a 684 bytes payload. > Since this command is intented for qtesting, it is acceptable. Not a requirement, but have you considered adding a qtest command instead? > sdbus-command <qom-path> <args...> < OK <result...> The (small) advantage is that it keeps test-only commands in the qtest protocol and out of the QMP schema. The downside of qtest is that qapi parsing is not available, so it involves manual marshalling code. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 01/03/2018 03:49 PM, Philippe Mathieu-Daudé wrote:
> Use Base64 to serialize the binary blobs in JSON.
> So far at most 512 bytes will be transfered, which result
s/transfered/transferred/
> in a 684 bytes payload.
> Since this command is intented for qtesting, it is acceptable.
s/intented/intended/
Might be worth mentioning the actual command name,
x-debug-sdbus-command, in the commit message to make future git log
trawling easier.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> +##
> +# @SDBusCommandResponse:
> +#
> +# SD Bus command response.
> +#
> +# @base64: the command response encoded as a Base64 string, if any (optional)
s/ (optional)//, now that the documentation engine automatically takes
care of that.
Even if there is no response, isn't the empty string "" more accurate
than omitting the string altogether? In other words, I'm not sure why
you made the 'base64' member optional.
> +#
> +# Since: 2.11
2.12
> +##
> +{ 'struct': 'SDBusCommandResponse', 'data': {'*base64': 'str'} }
> +
> +##
> +# @x-debug-sdbus-command:
> +#
> +# Execute a command on a SD Bus return the response (if any).
> +#
Maybe mention that this command is only intended for use during unit
testing (that information is already implicit from the x-debug prefix,
but stating it explicitly doesn't hurt).
> +# @qom-path: the SD Bus path
> +# @command: the SD protocol command to execute in the bus
> +# @arg: a 64-bit command argument (optional)
> +# @crc: the command/argument CRC (optional)
> +#
> +# Returns: the response of the command encoded as a Base64 string
> +#
> +# Since: 2.11
2.12
> +#
> +# -> { "execute": "x-debug-sdbus-command",
> +# "arguments": { "qom-path": "/machine/unattached/device[32]/sd.0",
> +# "command": 0x01
That's invalid JSON (which does not understand hex numbers). You need
"command": 1
> +# }
> +# }
> +# <- { "return": {'base64': 'A='} }
> +#
> +##
> +{ 'command': 'x-debug-sdbus-command',
> + 'data': { 'qom-path': 'str',
> + 'command': 'uint8',
> + '*arg': 'uint64',
> + '*crc': 'uint16' },
> + 'returns': 'SDBusCommandResponse'
> +}
> diff --git a/hw/sd/sdbus-qmp.c b/hw/sd/sdbus-qmp.c
> new file mode 100644
> index 0000000000..8c4b6f2aee
> --- /dev/null
> +++ b/hw/sd/sdbus-qmp.c
> @@ -0,0 +1,65 @@
> +/*
> + * SD card bus QMP debugging interface (for QTesting).
> + *
> + * Copyright (c) 2017 ?
The question mark in a copyright line is not right. I don't know what
you meant to use, but unless you were doing the work on behalf of an
employer, your own name is probably correct. You could include 2018 now
if you wanted.
> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
> + uint8_t command,
> + bool has_arg, uint64_t arg,
> + bool has_crc, uint16_t crc,
> + Error **errp)
> +{
> + uint8_t response[16 + 1];
> + SDBusCommandResponse *res;
> + bool ambiguous = false;
> + Object *obj;
> + SDBus *sdbus;
> + int sz;
> +
> + obj = object_resolve_path(qom_path, &ambiguous);
> + if (obj == NULL) {
I don't know if the style 'if (!obj) {' is any more prevalent; but it
doesn't really matter.
> + if (ambiguous) {
> + error_setg(errp, "Path '%s' is ambiguous", qom_path);
> + } else {
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "Device '%s' not found", qom_path);
> + }
> + return NULL;
> + }
> + sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
Is the cast still necessary, or does object_dynamic_cast() return void*
so that you can omit the cast?
> + if (sdbus == NULL) {
> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> + "Device '%s' not a sd-bus", qom_path);
> + return NULL;
> + }
> +
> + res = g_new0(SDBusCommandResponse, 1);
> + sz = sdbus_do_command(sdbus,
> + &(SDRequest){ command, arg, has_crc ? crc : -1 },
It's probably safer to use specific initializer assignments, as in:
&(SDRequest){ .cmd = command, .arg = arg, ...
> +++ b/stubs/qmp_sdbus.c
> @@ -0,0 +1,12 @@
> +#include "qemu/osdep.h"
> +#include "qmp-commands.h"
> +#include "hw/sd/sd.h"
> +
> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
> + uint8_t command,
> + bool has_arg, uint64_t arg,
> + bool has_crc, uint16_t crc,
> + Error **errp)
> +{
> + return NULL;
In addition to returning NULL, the stub should set errp.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Hi Eric,
On 01/05/2018 12:29 PM, Eric Blake wrote:
> On 01/03/2018 03:49 PM, Philippe Mathieu-Daudé wrote:
>> Use Base64 to serialize the binary blobs in JSON.
>> So far at most 512 bytes will be transfered, which result
>
> s/transfered/transferred/
>
>> in a 684 bytes payload.
>> Since this command is intented for qtesting, it is acceptable.
>
> s/intented/intended/
>
> Might be worth mentioning the actual command name,
> x-debug-sdbus-command, in the commit message to make future git log
> trawling easier.
Ok.
What about using 'x-qtest-sdbus-command'?
>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>
>> +##
>> +# @SDBusCommandResponse:
>> +#
>> +# SD Bus command response.
>> +#
>> +# @base64: the command response encoded as a Base64 string, if any (optional)
>
> s/ (optional)//, now that the documentation engine automatically takes
> care of that.
>
> Even if there is no response, isn't the empty string "" more accurate
> than omitting the string altogether? In other words, I'm not sure why
> you made the 'base64' member optional.
Indeed you right.
>
>> +#
>> +# Since: 2.11
>
> 2.12
Oops :)
>
>> +##
>> +{ 'struct': 'SDBusCommandResponse', 'data': {'*base64': 'str'} }
>> +
>> +##
>> +# @x-debug-sdbus-command:
>> +#
>> +# Execute a command on a SD Bus return the response (if any).
>> +#
>
> Maybe mention that this command is only intended for use during unit
> testing (that information is already implicit from the x-debug prefix,
> but stating it explicitly doesn't hurt).
>
>> +# @qom-path: the SD Bus path
>> +# @command: the SD protocol command to execute in the bus
>> +# @arg: a 64-bit command argument (optional)
>> +# @crc: the command/argument CRC (optional)
>> +#
>> +# Returns: the response of the command encoded as a Base64 string
>> +#
>> +# Since: 2.11
>
> 2.12
>
>> +#
>> +# -> { "execute": "x-debug-sdbus-command",
>> +# "arguments": { "qom-path": "/machine/unattached/device[32]/sd.0",
>> +# "command": 0x01
>
> That's invalid JSON (which does not understand hex numbers). You need
> "command": 1
Yes, you right, I wonder how I ended using hex here (I don't have any in
my .qmp_history ...)
>
>> +# }
>> +# }
>> +# <- { "return": {'base64': 'A='} }
>> +#
>> +##
>> +{ 'command': 'x-debug-sdbus-command',
>> + 'data': { 'qom-path': 'str',
>> + 'command': 'uint8',
>> + '*arg': 'uint64',
>> + '*crc': 'uint16' },
>> + 'returns': 'SDBusCommandResponse'
>> +}
>> diff --git a/hw/sd/sdbus-qmp.c b/hw/sd/sdbus-qmp.c
>> new file mode 100644
>> index 0000000000..8c4b6f2aee
>> --- /dev/null
>> +++ b/hw/sd/sdbus-qmp.c
>> @@ -0,0 +1,65 @@
>> +/*
>> + * SD card bus QMP debugging interface (for QTesting).
>> + *
>> + * Copyright (c) 2017 ?
>
> The question mark in a copyright line is not right. I don't know what
> you meant to use, but unless you were doing the work on behalf of an
> employer, your own name is probably correct. You could include 2018 now
> if you wanted.
:)
>
>
>> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
>> + uint8_t command,
>> + bool has_arg, uint64_t arg,
>> + bool has_crc, uint16_t crc,
>> + Error **errp)
>> +{
>> + uint8_t response[16 + 1];
>> + SDBusCommandResponse *res;
>> + bool ambiguous = false;
>> + Object *obj;
>> + SDBus *sdbus;
>> + int sz;
>> +
>> + obj = object_resolve_path(qom_path, &ambiguous);
>> + if (obj == NULL) {
>
> I don't know if the style 'if (!obj) {' is any more prevalent; but it
> doesn't really matter.
old habits are hard to change :)
>
>> + if (ambiguous) {
>> + error_setg(errp, "Path '%s' is ambiguous", qom_path);
>> + } else {
>> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> + "Device '%s' not found", qom_path);
>> + }
>> + return NULL;
>> + }
>> + sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
>
> Is the cast still necessary, or does object_dynamic_cast() return void*
> so that you can omit the cast?
Good to know!
>
>> + if (sdbus == NULL) {
>> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> + "Device '%s' not a sd-bus", qom_path);
>> + return NULL;
>> + }
>> +
>> + res = g_new0(SDBusCommandResponse, 1);
>> + sz = sdbus_do_command(sdbus,
>> + &(SDRequest){ command, arg, has_crc ? crc : -1 },
>
> It's probably safer to use specific initializer assignments, as in:
>
> &(SDRequest){ .cmd = command, .arg = arg, ...
Ok.
>
>
>> +++ b/stubs/qmp_sdbus.c
>> @@ -0,0 +1,12 @@
>> +#include "qemu/osdep.h"
>> +#include "qmp-commands.h"
>> +#include "hw/sd/sd.h"
>> +
>> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
>> + uint8_t command,
>> + bool has_arg, uint64_t arg,
>> + bool has_crc, uint16_t crc,
>> + Error **errp)
>> +{
>> + return NULL;
>
> In addition to returning NULL, the stub should set errp.
Oh, I didn't know.
Thanks for your detailed review!
Phil.
On 01/05/2018 10:06 AM, Philippe Mathieu-Daudé wrote: > Hi Eric, > > On 01/05/2018 12:29 PM, Eric Blake wrote: >> On 01/03/2018 03:49 PM, Philippe Mathieu-Daudé wrote: >>> Use Base64 to serialize the binary blobs in JSON. >>> So far at most 512 bytes will be transfered, which result >> >> s/transfered/transferred/ >> >>> in a 684 bytes payload. >>> Since this command is intented for qtesting, it is acceptable. >> >> s/intented/intended/ >> >> Might be worth mentioning the actual command name, >> x-debug-sdbus-command, in the commit message to make future git log >> trawling easier. > > Ok. > > What about using 'x-qtest-sdbus-command'? x-debug matches existing practice, x-qtest does not. I'm fine with the name you had chosen, and was just asking that it be mentioned in the commit log. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Hi Eric,
On Fri, Jan 5, 2018 at 12:29 PM, Eric Blake <eblake@redhat.com> wrote:
> On 01/03/2018 03:49 PM, Philippe Mathieu-Daudé wrote:
[...]
>> + if (ambiguous) {
>> + error_setg(errp, "Path '%s' is ambiguous", qom_path);
>> + } else {
>> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> + "Device '%s' not found", qom_path);
>> + }
>> + return NULL;
>> + }
>> + sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
>
> Is the cast still necessary, or does object_dynamic_cast() return void*
> so that you can omit the cast?
Apparently the cast is necessary, since the object_dynamic_cast()
returns an Object pointer,
and the SD_BUS() macro uses object_dynamic_cast_assert() which
generates a runtime assert.
without casting:
hw/sd/sdbus-qmp.c:37:11: error: assignment from incompatible pointer
type [-Werror=incompatible-pointer-types]
sdbus = object_dynamic_cast(obj, TYPE_SD_BUS);
^
>
>> + if (sdbus == NULL) {
>> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> + "Device '%s' not a sd-bus", qom_path);
>> + return NULL;
>> + }
Hi Markus,
[Asking again from the correct series thread]
On 1/3/18 10:49 PM, Philippe Mathieu-Daudé wrote:
> Use Base64 to serialize the binary blobs in JSON.
> So far at most 512 bytes will be transfered, which result
> in a 684 bytes payload.
> Since this command is intented for qtesting, it is acceptable.
Any comment regarding QMP for this patch?
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> qapi-schema.json | 41 +++++++++++++++++++++++++++++++++
> hw/sd/sdbus-qmp.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> stubs/qmp_sdbus.c | 12 ++++++++++
> hw/sd/Makefile.objs | 2 +-
> stubs/Makefile.objs | 1 +
> 5 files changed, 120 insertions(+), 1 deletion(-)
> create mode 100644 hw/sd/sdbus-qmp.c
> create mode 100644 stubs/qmp_sdbus.c
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 18457954a8..be26e8cd34 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3200,3 +3200,44 @@
> # Since: 2.11
> ##
> { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
> +
> +##
> +# @SDBusCommandResponse:
> +#
> +# SD Bus command response.
> +#
> +# @base64: the command response encoded as a Base64 string, if any (optional)
> +#
> +# Since: 2.11
> +##
> +{ 'struct': 'SDBusCommandResponse', 'data': {'*base64': 'str'} }
> +
> +##
> +# @x-debug-sdbus-command:
> +#
> +# Execute a command on a SD Bus return the response (if any).
> +#
> +# @qom-path: the SD Bus path
> +# @command: the SD protocol command to execute in the bus
> +# @arg: a 64-bit command argument (optional)
> +# @crc: the command/argument CRC (optional)
> +#
> +# Returns: the response of the command encoded as a Base64 string
> +#
> +# Since: 2.11
> +#
> +# -> { "execute": "x-debug-sdbus-command",
> +# "arguments": { "qom-path": "/machine/unattached/device[32]/sd.0",
> +# "command": 0x01
> +# }
> +# }
> +# <- { "return": {'base64': 'A='} }
> +#
> +##
> +{ 'command': 'x-debug-sdbus-command',
> + 'data': { 'qom-path': 'str',
> + 'command': 'uint8',
> + '*arg': 'uint64',
> + '*crc': 'uint16' },
> + 'returns': 'SDBusCommandResponse'
> +}
> diff --git a/hw/sd/sdbus-qmp.c b/hw/sd/sdbus-qmp.c
> new file mode 100644
> index 0000000000..8c4b6f2aee
> --- /dev/null
> +++ b/hw/sd/sdbus-qmp.c
> @@ -0,0 +1,65 @@
> +/*
> + * SD card bus QMP debugging interface (for QTesting).
> + *
> + * Copyright (c) 2017 ?
> + *
> + * Author:
> + * Philippe Mathieu-Daudé <f4bug@amsat.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "hw/sd/sd.h"
> +#include "qmp-commands.h"
> +
> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
> + uint8_t command,
> + bool has_arg, uint64_t arg,
> + bool has_crc, uint16_t crc,
> + Error **errp)
> +{
> + uint8_t response[16 + 1];
> + SDBusCommandResponse *res;
> + bool ambiguous = false;
> + Object *obj;
> + SDBus *sdbus;
> + int sz;
> +
> + obj = object_resolve_path(qom_path, &ambiguous);
> + if (obj == NULL) {
> + if (ambiguous) {
> + error_setg(errp, "Path '%s' is ambiguous", qom_path);
> + } else {
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "Device '%s' not found", qom_path);
> + }
> + return NULL;
> + }
> + sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
> + if (sdbus == NULL) {
> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> + "Device '%s' not a sd-bus", qom_path);
> + return NULL;
> + }
> +
> + res = g_new0(SDBusCommandResponse, 1);
> + sz = sdbus_do_command(sdbus,
> + &(SDRequest){ command, arg, has_crc ? crc : -1 },
> + response);
> + if (sz > 0) {
> + res->has_base64 = true;
> + res->base64 = g_base64_encode(response, sz);
> + }
> +
> + return res;
> +}
> diff --git a/stubs/qmp_sdbus.c b/stubs/qmp_sdbus.c
> new file mode 100644
> index 0000000000..d9bd75ec71
> --- /dev/null
> +++ b/stubs/qmp_sdbus.c
> @@ -0,0 +1,12 @@
> +#include "qemu/osdep.h"
> +#include "qmp-commands.h"
> +#include "hw/sd/sd.h"
> +
> +SDBusCommandResponse *qmp_x_debug_sdbus_command(const char *qom_path,
> + uint8_t command,
> + bool has_arg, uint64_t arg,
> + bool has_crc, uint16_t crc,
> + Error **errp)
> +{
> + return NULL;
> +}
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index c2b7664264..3a70477bba 100644
> --- a/hw/sd/Makefile.objs
> +++ b/hw/sd/Makefile.objs
> @@ -1,6 +1,6 @@
> common-obj-$(CONFIG_PL181) += pl181.o
> common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
> -common-obj-$(CONFIG_SD) += sd.o core.o
> +common-obj-$(CONFIG_SD) += sd.o core.o sdbus-qmp.o
> common-obj-$(CONFIG_SDHCI) += sdhci.o
>
> obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 8cfe34328a..a46cb3b992 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -35,6 +35,7 @@ stub-obj-y += vm-stop.o
> stub-obj-y += vmstate.o
> stub-obj-$(CONFIG_WIN32) += fd-register.o
> stub-obj-y += qmp_pc_dimm.o
> +stub-obj-y += qmp_sdbus.o
> stub-obj-y += target-monitor-defs.o
> stub-obj-y += target-get-monitor-def.o
> stub-obj-y += pc_madt_cpu_entry.o
>
On 08/03/2019 17.11, Philippe Mathieu-Daudé wrote: > Hi Markus, > > [Asking again from the correct series thread] > > On 1/3/18 10:49 PM, Philippe Mathieu-Daudé wrote: >> Use Base64 to serialize the binary blobs in JSON. >> So far at most 512 bytes will be transfered, which result >> in a 684 bytes payload. >> Since this command is intented for qtesting, it is acceptable. > > Any comment regarding QMP for this patch? Is this useful for anybody else than qtest? If not, I think this should rather go into the qtest protocol instead, since QMP is our "public" protocol. >> diff --git a/hw/sd/sdbus-qmp.c b/hw/sd/sdbus-qmp.c >> new file mode 100644 >> index 0000000000..8c4b6f2aee >> --- /dev/null >> +++ b/hw/sd/sdbus-qmp.c >> @@ -0,0 +1,65 @@ >> +/* >> + * SD card bus QMP debugging interface (for QTesting). >> + * >> + * Copyright (c) 2017 ? Weird line. Add you name here ? Thomas
On Mon, Mar 11, 2019 at 12:49:50PM +0100, Thomas Huth wrote: > On 08/03/2019 17.11, Philippe Mathieu-Daudé wrote: > > Hi Markus, > > > > [Asking again from the correct series thread] > > > > On 1/3/18 10:49 PM, Philippe Mathieu-Daudé wrote: > >> Use Base64 to serialize the binary blobs in JSON. > >> So far at most 512 bytes will be transfered, which result > >> in a 684 bytes payload. > >> Since this command is intented for qtesting, it is acceptable. > > > > Any comment regarding QMP for this patch? > > Is this useful for anybody else than qtest? If not, I think this should > rather go into the qtest protocol instead, since QMP is our "public" > protocol. Extending qtest requires writing parsers by hand. Do we really want to go that route and start extending the qtest protocol more often? I also plan to add new debugging-only QMP commands for testing CPU code, and I'm not looking forward to writing my own parser inside qtest_process_command(). -- Eduardo
On Mon, 11 Mar 2019 at 13:43, Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Mon, Mar 11, 2019 at 12:49:50PM +0100, Thomas Huth wrote: > > On 08/03/2019 17.11, Philippe Mathieu-Daudé wrote: > > > Hi Markus, > > > > > > [Asking again from the correct series thread] > > > > > > On 1/3/18 10:49 PM, Philippe Mathieu-Daudé wrote: > > >> Use Base64 to serialize the binary blobs in JSON. > > >> So far at most 512 bytes will be transfered, which result > > >> in a 684 bytes payload. > > >> Since this command is intented for qtesting, it is acceptable. > > > > > > Any comment regarding QMP for this patch? > > > > Is this useful for anybody else than qtest? If not, I think this should > > rather go into the qtest protocol instead, since QMP is our "public" > > protocol. > > Extending qtest requires writing parsers by hand. Do we really > want to go that route and start extending the qtest protocol more > often? Perhaps we could have qtest-only QMP commands that only get recognized if qtest_enabled() ? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 11 Mar 2019 at 13:43, Eduardo Habkost <ehabkost@redhat.com> wrote: >> >> On Mon, Mar 11, 2019 at 12:49:50PM +0100, Thomas Huth wrote: >> > On 08/03/2019 17.11, Philippe Mathieu-Daudé wrote: >> > > Hi Markus, >> > > >> > > [Asking again from the correct series thread] >> > > >> > > On 1/3/18 10:49 PM, Philippe Mathieu-Daudé wrote: >> > >> Use Base64 to serialize the binary blobs in JSON. >> > >> So far at most 512 bytes will be transfered, which result >> > >> in a 684 bytes payload. >> > >> Since this command is intented for qtesting, it is acceptable. >> > > >> > > Any comment regarding QMP for this patch? >> > >> > Is this useful for anybody else than qtest? If not, I think this should >> > rather go into the qtest protocol instead, since QMP is our "public" >> > protocol. >> >> Extending qtest requires writing parsers by hand. Do we really >> want to go that route and start extending the qtest protocol more >> often? > > Perhaps we could have qtest-only QMP commands that only get > recognized if qtest_enabled() ? QMP commands are defined at compile time. We just got rid of the hack to "unrecognize" selected commands dynamically at run-time (commit 0b69f6f72ce), which we had because our compile-time facilities were lacking. I'd hate to bring this hack back. What's easy is to have QMP commands that fail unless qtest_enabled() :)
© 2016 - 2025 Red Hat, Inc.