Re: [libvirt] [PATCH v3 3/5] qemu_monitor: Introduce qemuMonitorGetObjectProps
On 04/19/2018 06:04 PM, Ján Tomko wrote:
On Thu, Apr 19, 2018 at 04:00:25PM +0200, Michal Privoznik wrote:
> Now that we've gotten rid of misleading names we can introduce
> qemuMonitorGetObjectProps() function which queries -object
> properties. Again, some parts of code can be reused.
>
This should be two commits:
* split out qemuMonitorJSONParsePropsList
* introduce qemuMonitorGetObjectProps
Well, I can split it, but one patch without the other does not make
sense. That's the reason I've merged them. If somebody is going to
backport this, they will have to backport both patches or none.
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_monitor.c | 13 ++++++++++
> src/qemu/qemu_monitor.h | 3 +++
> src/qemu/qemu_monitor_json.c | 62
> ++++++++++++++++++++++++++++++++++----------
> src/qemu/qemu_monitor_json.h | 4 +++
> 4 files changed, 68 insertions(+), 14 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index f642d9a51a..a1e2e40d0f 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3932,6 +3932,19 @@ qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
> }
>
>
> +int
> +qemuMonitorGetObjectProps(qemuMonitorPtr mon,
> + const char *object,
> + char ***props)
> +{
> + VIR_DEBUG("object=%s props=%p", object, props);
> +
> + QEMU_CHECK_MONITOR_JSON(mon);
> +
> + return qemuMonitorJSONGetObjectProps(mon, object, props);
> +}
> +
> +
> char *
> qemuMonitorGetTargetArch(qemuMonitorPtr mon)
> {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index d6b68b44ca..0f648ce27c 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1048,6 +1048,9 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
> int qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
> const char *device,
> char ***props);
> +int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
> + const char *object,
> + char ***props);
> char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
>
> int qemuMonitorNBDServerStart(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 24d37eb41d..bd1729488c 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6053,29 +6053,23 @@ int
> qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
> #undef MAKE_SET_CMD
>
>
> -int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
> - const char *device,
> - char ***props)
> +static int
> +qemuMonitorJSONParsePropsList(qemuMonitorPtr mon,
> + virJSONValuePtr cmd,
> + char ***props)
> {
> - int ret = -1;
> - virJSONValuePtr cmd;
> virJSONValuePtr reply = NULL;
> virJSONValuePtr data;
> char **proplist = NULL;
> ssize_t n = 0;
> size_t i;
> -
> - *props = NULL;
> -
> - if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
> - "s:typename", device,
> - NULL)))
> - return -1;
> + int ret = -1;
>
> if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> goto cleanup;
>
> - if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
> + if (qemuMonitorJSONHasError(reply, "DeviceNotFound") ||
> + qemuMonitorJSONHasError(reply, "CommandNotFound")) {
One of the errors is appropriate for device-list-properties,
the other one for qom-list-properties.
Yes. But it doesn't really matter. If device-list-properties returns
CommandNotFound we will not add any capability. So it's safe.
Each caller should take care of their error and
qemuMonitorJSONParsePropsList
would work with virJSON replies data.
I guess it's just a matter of preference, but okay. I'll rework this patch.
Michal