Re: [PATCH 06/11] qemu: Add monitor functions to set IOThread params

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

 




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@xxxxxxxxxx>,
>> 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@xxxxxxxxxx>
>> ---
>>  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) { \
        memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); \
        prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \
        prop.val.iv = iothreadInfo->propVal; \
        if (qemuMonitorJSONSetObjectProperty(mon, path, propName, &prop)
< 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux