4:16 a.m.
On 10/19/2018 03:09 PM, John Ferlan wrote:
On 10/19/18 7:06 AM, Michal Privoznik wrote:
> On 10/07/2018 03:00 PM, John Ferlan wrote:
>> Add functions to set the IOThreadInfo param data for the live guest.
>>
>> Based on code originally posted by Pavel Hrdina <phrdina(a)redhat.com>,
>> but extracted into a separate patch. Note that qapi expects to receive
>> integer parameters rather than unsigned long long or unsigned int's.
>> QEMU does save the value in larger signed 64 bit values eventually.
>>
>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>> ---
>> src/qemu/qemu_monitor.c | 19 +++++++++++++++++++
>> src/qemu/qemu_monitor.h | 2 ++
>> src/qemu/qemu_monitor_json.c | 33 +++++++++++++++++++++++++++++++++
>> src/qemu/qemu_monitor_json.h | 4 ++++
>> 4 files changed, 58 insertions(+)
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index 7f7013e115..a65d638ab8 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -4135,6 +4135,25 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
>> }
>>
>>
>> +/**
>> + * qemuMonitorSetIOThread:
>> + * @mon: Pointer to the monitor
>> + * @iothreadInfo: filled IOThread info with data
>> + *
>> + * Alter the specified IOThread's IOThreadInfo values.
>> + */
>> +int
>> +qemuMonitorSetIOThread(qemuMonitorPtr mon,
>> + qemuMonitorIOThreadInfoPtr iothreadInfo)
>> +{
>> + VIR_DEBUG("iothread=%p", iothreadInfo);
>> +
>> + QEMU_CHECK_MONITOR(mon);
>> +
>> + return qemuMonitorJSONSetIOThread(mon, iothreadInfo);
>> +}
>> +
>> +
>> /**
>> * qemuMonitorGetMemoryDeviceInfo:
>> * @mon: pointer to the monitor
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index c2991e2b16..ef71fc6448 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1123,6 +1123,8 @@ struct _qemuMonitorIOThreadInfo {
>> };
>> int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
>> qemuMonitorIOThreadInfoPtr **iothreads);
>> +int qemuMonitorSetIOThread(qemuMonitorPtr mon,
>> + qemuMonitorIOThreadInfoPtr iothreadInfo);
>>
>> typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo;
>> typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr;
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 2e92984b44..bb1d62b844 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -7474,6 +7474,39 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
>> }
>>
>>
>> +int
>> +qemuMonitorJSONSetIOThread(qemuMonitorPtr mon,
>> + qemuMonitorIOThreadInfoPtr iothreadInfo)
>> +{
>> + int ret = -1;
>> + char *path = NULL;
>> + qemuMonitorJSONObjectProperty prop;
>> +
>> + if (virAsprintf(&path, "/objects/iothread%u",
>> + iothreadInfo->iothread_id) < 0)
>> + goto cleanup;
>> +
>> +#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
>> + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); \
>> + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
>> + prop.val.iv = propVal; \
>> + if (qemuMonitorJSONSetObjectProperty(mon, path, propName, &prop) < 0)
\
>> + goto cleanup;
>> +
>> + VIR_IOTHREAD_SET_PROP("poll-max-ns",
iothreadInfo->poll_max_ns)
>> + VIR_IOTHREAD_SET_PROP("poll-grow", iothreadInfo->poll_grow)
>> + VIR_IOTHREAD_SET_PROP("poll-shrink",
iothreadInfo->poll_shrink)
>
> So this is all or nothing approach. All values are set - even though
> user might request through public API to change just one. I don't think
> it is a good design. Either we need a monitor API that changes just one
> value and call it for every typed parameter that user sends to us, or
> public API implementation must copy the old values into this struct
> (even though it would be ugly).
>
> Michal
>
Fair complaint - I tried to reuse as much as possible from the initial
series so that I didn't "waste" time implementing something that in the
long run wasn't desired. Originally there were lots of checks about what
was or wasn't set - I just took the path of least resistance.
It should be simple to add flag for each to determine which was set
before setting them in the object path.
IOW:
#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
if (iothreadInfo->set_##propVal) { \
This will fly iff zero value can't be set on the monitor. I have no idea
whether it can or can not. If it can be set, then we have to go with
something more clever.
Michal