Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
On 10/23/2018 03:42 PM, John Ferlan wrote:
> [...]
>
>>>
>>> Not 100% sure what you meant... The point of the bools is to indicate
>>> when the value was set, doesn't matter if it's zero.
>>
>> Ah sorry, I've misunderstood then. set_##propVal is a boolean not
>> integer holding the value we want to set. So you'd have two variables,
>> int A and bool set_A. Okay, this will work well. However, at this point
>> I wonder if having monitor API that would take attribute name and a
>> value would be simpler. Something like:
>>
>>
>> qemuMonitorSetIOThreadAttr(qemuMonitorPtr mon,
>>unsigned int thread_id,
>>const char *attrName,
>>int attrVal);
>>
>>
>> Then this function would be called like this:
>>
>> static int
>> qemuDomainSetIOThreadParams(virDomainPtr dom,
>> unsigned int iothread_id,
>> virTypedParameterPtr params,
>> int nparams,
>> unsigned int flags)
>> {
>> ...
>>
>> for (i = 0; i < nparams; i++) {
>> qemuMonitorSetIOThreadAttr(mon, thread_id, param[i].str,
>>param[i].val);
>> }
>> ...
>> }
>>
>>
>> Anyway, I'll leave it up to you.
>>
>> Michal
>>
>
> I kept a lot of what Pavel already had for simplicity and leaving the
> setting deeper in the _json code for all values rather than 'n' calls to
> essentially do the same thing. Those patches were posted before this
> discussion. I'm not against rewriting to conform to some other
> mechanism, just let me know.
At this point it is more of a personal preference than anything. From
functional POV these two approaches are the same. So I am okay with you
keeping what you suggested.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
[...]
>>>
>>
>> Not 100% sure what you meant... The point of the bools is to indicate
>> when the value was set, doesn't matter if it's zero.
>
> Ah sorry, I've misunderstood then. set_##propVal is a boolean not
> integer holding the value we want to set. So you'd have two variables,
> int A and bool set_A. Okay, this will work well. However, at this point
> I wonder if having monitor API that would take attribute name and a
> value would be simpler. Something like:
>
>
> qemuMonitorSetIOThreadAttr(qemuMonitorPtr mon,
>unsigned int thread_id,
>const char *attrName,
>int attrVal);
>
>
> Then this function would be called like this:
>
> static int
> qemuDomainSetIOThreadParams(virDomainPtr dom,
> unsigned int iothread_id,
> virTypedParameterPtr params,
> int nparams,
> unsigned int flags)
> {
> ...
>
> for (i = 0; i < nparams; i++) {
> qemuMonitorSetIOThreadAttr(mon, thread_id, param[i].str,
>param[i].val);
> }
> ...
> }
>
>
> Anyway, I'll leave it up to you.
>
> Michal
>
I kept a lot of what Pavel already had for simplicity and leaving the
setting deeper in the _json code for all values rather than 'n' calls to
essentially do the same thing. Those patches were posted before this
discussion. I'm not against rewriting to conform to some other
mechanism, just let me know.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
On 10/22/2018 11:00 PM, John Ferlan wrote:
>
>
> On 10/22/18 5:16 AM, Michal Prívozník wrote:
>> 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 ,
> 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
> ---
> 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(, "/objects/iothread%u",
> +iothreadInfo->iothread_id) < 0)
> +goto cleanup;
> +
> +#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
> +memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
> +prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
> +prop.val.iv = propVal; \
> +if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) <
> 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
>>
>
> Not 100%
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
On 10/22/18 5:16 AM, Michal Prívozník wrote:
> 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 ,
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
---
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(, "/objects/iothread%u",
+iothreadInfo->iothread_id) < 0)
+goto cleanup;
+
+#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
+memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
+prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
+prop.val.iv = propVal; \
+if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) < 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
>
Not 100% sure what you meant... The point of the bools is to indicate
when the value was set, doesn't matter if it's zero.
From: $QEMU.GIT/qapi/misc.json:
# @poll-max-ns: maximum
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
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 ,
>>> 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
>>> ---
>>> 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(, "/objects/iothread%u",
>>> +iothreadInfo->iothread_id) < 0)
>>> +goto cleanup;
>>> +
>>> +#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
>>> +memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
>>> +prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
>>> +prop.val.iv = propVal; \
>>> +if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) < 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
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 ,
>> 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
>> ---
>> 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(, "/objects/iothread%u",
>> +iothreadInfo->iothread_id) < 0)
>> +goto cleanup;
>> +
>> +#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
>> +memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
>> +prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
>> +prop.val.iv = propVal; \
>> +if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) < 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) { \
memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
prop.val.iv = iothreadInfo->propVal; \
if (qemuMonitorJSONSetObjectProperty(mon, path, propName, )
< 0) \
goto cleanup; \
}
VIR_IOTHREAD_SET_PROP("poll-max-ns", poll_max_ns);
VIR_IOTHREAD_SET_PROP("poll-grow", poll_grow);
VIR_IOTHREAD_SET_PROP("poll-shrink", poll_shrink);
That just then gets utilized in patch10 as well.
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
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 ,
> 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
> ---
> 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(, "/objects/iothread%u",
> +iothreadInfo->iothread_id) < 0)
> +goto cleanup;
> +
> +#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
> +memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
> +prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
> +prop.val.iv = propVal; \
> +if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) < 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/11] qemu: Add monitor functions to set IOThread params
Add functions to set the IOThreadInfo param data for the live guest.
Based on code originally posted by Pavel Hrdina ,
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
---
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(, "/objects/iothread%u",
+iothreadInfo->iothread_id) < 0)
+goto cleanup;
+
+#define VIR_IOTHREAD_SET_PROP(propName, propVal) \
+memset(, 0, sizeof(qemuMonitorJSONObjectProperty)); \
+prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
+prop.val.iv = propVal; \
+if (qemuMonitorJSONSetObjectProperty(mon, path, propName, ) < 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)
+
+#undef VIR_IOTHREAD_SET_PROP
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+}
+
+
int
qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
virHashTablePtr info)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index da267b15b0..c3abd0ddf0 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -502,6 +502,10 @@ int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
qemuMonitorIOThreadInfoPtr **iothreads)
ATTRIBUTE_NONNULL(2);
+int qemuMonitorJSONSetIOThread(qemuMonitorPtr mon,
+ qemuMonitorIOThreadInfoPtr iothreadInfo)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
int qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
virHashTablePtr info)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
