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

[Michal Privoznik]

On 28.11.2012 00:46, Eric Blake wrote:
>> 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.
> 

Yeah, this patch just follows structure we have. I agree that your
suggestion is nicer, but not really connected to feature I am
introducing. It should be saved for separate patch set.

Michal