[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 05/11] qemu: Introduce nbd-server-stop command



> This will be used after all migration work is done
> to stop NBD server running on destination.  It
> doesn't take any arguments, just issues a command.
> ---
>  src/qemu/qemu_monitor.c      |   19 +++++++++++++++++++
>  src/qemu/qemu_monitor.h      |    1 +
>  src/qemu/qemu_monitor_json.c |   21 +++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |    1 +
>  4 files changed, 42 insertions(+), 0 deletions(-)

> +++ b/src/qemu/qemu_monitor.c
> @@ -3365,3 +3365,22 @@ int qemuMonitorNBDServerAdd(qemuMonitorPtr
> mon,
>  
>      return qemuMonitorJSONNBDServerAdd(mon, deviceID);
>  }
> +
> +int qemuMonitorNBDServerStop(qemuMonitorPtr mon)
> +{
> +    VIR_DEBUG("mon=%p", mon);
> +
> +    if (!mon) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("monitor must not be NULL"));
> +        return -1;
> +    }
> +
> +    if (!mon->json) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("JSON monitor is required"));
> +        return -1;
> +    }

No change to this patch, but I'm getting rather tired of this
copy and paste pattern - every new monitor command has to touch
two .h files.  I wonder if we should instead switch to more of
a callback pattern, where we only have to touch one .h file
(the public entry point, and the signature of a callback function),
then have qemu_monitor.c do:

if (mon->driver.func)
    mon->driver.func(args)
else
    report error about unsupported

then qemu_monitor_json.c would register a table of static
callback functions, rather than having to also declare every
single function almost verbatim like qemu_monitor.h.

But enough of that side-track thought about a potential future
reorganization of the code.

ACK to this patch; fine as-is.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]