On Wed, Jul 17, 2024 at 21:21:35 +0300, Nikolai Barybin via Devel wrote:
> Signed-off-by: Nikolai Barybin <nikolai.barybin@xxxxxxxxxxxxx>
> ---
> src/qemu/qemu_monitor.c | 30 ++++++++++++++++
> src/qemu/qemu_monitor.h | 13 +++++++
> src/qemu/qemu_monitor_json.c | 66 ++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 13 +++++++
> 4 files changed, 122 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index b1c0c6a064..53f5ecf223 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2763,6 +2763,36 @@ qemuMonitorDeleteSnapshot(qemuMonitor *mon, const char *name)
> }
>
>
> +int
> +qemuMonitorSnapshotSave(qemuMonitor *mon,
> + const char *jobname,
> + const char *snapshotName,
> + const char *vmstateDev,
> + char **wrdevs)
> +{
> + VIR_DEBUG("jobname=%s, snapshotName=%s, vmstateDev=%s, wrdevs=%p",
> + jobname, snapshotName, vmstateDev, wrdevs);
Logging pointer to wrdevs is not useful as you can't get to the data
from logs. In this case we can simply drop it.
Also the variable values should be surrounded by quotes ('%s') to
visualy separate the value.
> +
> + QEMU_CHECK_MONITOR(mon);
> +
> + return qemuMonitorJSONSnapshotSave(mon, jobname, snapshotName, vmstateDev, wrdevs);
> +}
[...]
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 8a20ce57e6..312a4aee04 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8955,3 +8955,69 @@ int qemuMonitorJSONDisplayReload(qemuMonitor *mon,
>
> return 0;
> }
> +
> +int
> +qemuMonitorJSONSnapshotSave(qemuMonitor *mon,
> + const char *jobname,
> + const char *snapshotName,
> + const char *vmstateDev,
> + char **wrdevs)
const char **
> +{
> + g_autoptr(virJSONValue) cmd = NULL;
> + g_autoptr(virJSONValue) reply = NULL;
> + g_autoptr(virJSONValue) wrdev_list = NULL;
> + size_t i = 0;
A counter variable is not needed to iterate a NULL-terminated string
list once.
> +
> + if (wrdevs) {
> + wrdev_list = virJSONValueNewArray();
> +
> + for (i = 0; wrdevs[i]; i++)
> + if (virJSONValueArrayAppendString(wrdev_list, wrdevs[i]) < 0)
> + return -1;
> + }
> +
> + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-save",
> + "s:job-id", jobname,
> + "s:tag", snapshotName,
> + "s:vmstate", vmstateDev,
> + "A:devices", &wrdev_list,
> + NULL)))
> + return -1;
> +
> + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> + return -1;
> +
> + return qemuMonitorJSONCheckError(cmd, reply);
> +}
Looks good, but it's missing tests in test/qemumonitorjsontest.c. This
test case is important because it validates the commands libvirt uses
against the QMP schema obtained from qemu. Validation allows us to catch
deprecations and changes early.
