Re: [PATCH RFC v2 02/12] qemu: monitor: Add support for ThrottleGroup operations

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

 



On Thu, Apr 11, 2024 at 19:01:50 -0700, wucf@xxxxxxxxxxxxx wrote:
> From: Chun Feng Wu <wucf@xxxxxxxxxxxxx>
> 
> * ThrottleGroup is updated through "qemuMonitorJSONUpdateThrottleGroup"
> * ThrottleGroup is retrieved through "qemuMonitorJSONGetThrottleGroup"
> * ThrottleGroup is deleted by reusing "qemuMonitorDelObject"
> * ThrottleGroup is added by reusing "qemuMonitorAddObject"
> * "qemuMonitorMakeThrottleGroupLimits" will be used by building qemu cmd as well
> 
> Signed-off-by: Chun Feng Wu <wucf@xxxxxxxxxxxxx>
> ---
>  src/qemu/qemu_monitor.c      |  34 ++++++++
>  src/qemu/qemu_monitor.h      |  14 ++++
>  src/qemu/qemu_monitor_json.c | 151 +++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  14 ++++
>  4 files changed, 213 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 34e2ccab97..2cd122ea9e 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2996,6 +2996,40 @@ qemuMonitorGetBlockIoThrottle(qemuMonitor *mon,
>  }
>  
>  
> +int
> +qemuMonitorThrottleGroupLimits(virJSONValue *limits,
> +                               const virDomainThrottleGroupDef *group)
> +{
> +    return qemuMonitorMakeThrottleGroupLimits(limits, group);
> +}
> +
> +
> +int
> +qemuMonitorUpdateThrottleGroup(qemuMonitor *mon,
> +                               const char *qomid,
> +                               virDomainBlockIoTuneInfo *info)
> +{
> +    VIR_DEBUG("qomid=%s, info=%p", NULLSTR(qomid), info);

See below.

> +
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    return qemuMonitorJSONUpdateThrottleGroup(mon, qomid, info);
> +}
> +
> +
> +int
> +qemuMonitorGetThrottleGroup(qemuMonitor *mon,
> +                            const char *groupname,
> +                            virDomainBlockIoTuneInfo *reply)
> +{
> +    VIR_DEBUG("throttle-group=%s, reply=%p", NULLSTR(groupname), reply);

'qemuMonitorJSONGetThrottleGroup' requires that the group name is
non-NULL thus NULLSTR is not needed.

> +
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    return qemuMonitorJSONGetThrottleGroup(mon, groupname, reply);
> +}
> +
> +
>  int
>  qemuMonitorVMStatusToPausedReason(const char *status)

[...]


>  int qemuMonitorGetVersion(qemuMonitor *mon,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index eb84a3d938..9abcb40df2 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4633,6 +4633,157 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitor *mon,
>      return qemuMonitorJSONBlockIoThrottleInfo(devices, qdevid, reply);
>  }
>  
> +
> +int
> +qemuMonitorMakeThrottleGroupLimits(virJSONValue *limits,
> +                                   const virDomainThrottleGroupDef *group)
> +{
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-total", group->total_bytes_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-read", group->read_bytes_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-write", group->write_bytes_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-total", group->total_iops_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-read", group->read_iops_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-write", group->write_iops_sec)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-total-max", group->total_bytes_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-read-max", group->read_bytes_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "bps-write-max", group->write_bytes_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-total-max", group->total_iops_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-read-max", group->read_iops_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-write-max", group->write_iops_sec_max)<0)
> +        return -1;
> +    if (virJSONValueObjectAppendNumberLong(limits, "iops-size", group->size_iops_sec)<0)
> +        return -1;
> +    /* avoid error from QEMU: "the burst length cannot be 0" for throttlelimits
> +     * when setting max-length
> +     */
> +    if (virJSONValueObjectAdd(&limits, "P:bps-total-max-length", group->total_bytes_sec_max_length, NULL)<0)
> +        return -1;
> +    if (virJSONValueObjectAdd(&limits, "P:bps-read-max-length", group->read_bytes_sec_max_length, NULL)<0)
> +        return -1;
> +    if (virJSONValueObjectAdd(&limits, "P:bps-write-max-length", group->write_bytes_sec_max_length, NULL)<0)
> +        return -1;
> +    if (virJSONValueObjectAdd(&limits, "P:iops-total-max-length", group->total_iops_sec_max_length, NULL)<0)
> +        return -1;
> +    if (virJSONValueObjectAdd(&limits, "P:iops-read-max-length", group->read_iops_sec_max_length, NULL)<0)
> +        return -1;
> +    if (virJSONValueObjectAdd(&limits, "P:iops-write-max-length", group->write_iops_sec_max_length, NULL)<0)


virJSONValueObjectAdd can add multiple fields in one call:

       if (virJSONValueObjectAdd(&limits,
                                 "P:bps-total-max-length", group->total_bytes_sec_max_length,
                                 "P:bps-read-max-length", group->read_bytes_sec_max_length,
                                 "P:bps-write-max-length", group->write_bytes_sec_max_length,
                                 ...
                                 NULL) < 0)


> +        return -1;

Please also convert ALL of virJSONValueObjectAppendNumberLong above to
do the same all in one call to virJSONValueObjectAdd.

> +
> +    return 0;
> +}
> +
> +
> +int
> +qemuMonitorJSONUpdateThrottleGroup(qemuMonitor *mon,
> +                                   const char *qomid,
> +                                   virDomainBlockIoTuneInfo *info)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) result = NULL;
> +    g_autoptr(virJSONValue) limits = virJSONValueNewObject();
> +
> +    if (qemuMonitorMakeThrottleGroupLimits(limits, info)<0)
> +        return -1;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("qom-set",
> +                                           "s:property", "limits",
> +                                           "s:path", qomid,
> +                                           "a:value", &limits,
> +                                           NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &result) < 0)
> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, result) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +#define GET_THROTTLE_GROUP_VALUE(FIELD, STORE) \
> +    if (virJSONValueObjectHasKey(ret, FIELD)) { \
> +        if (virJSONValueObjectGetNumberUlong(ret, FIELD, &reply->STORE) < 0) { \
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \
> +                       _("throttle group field '%1$s' missing in qemu's output"), \
> +                       #STORE); \
> +            return -1; \
> +        } \
> +    }
> +
> +
> +int
> +qemuMonitorJSONGetThrottleGroup(qemuMonitor *mon,
> +                                const char *gname,
> +                                virDomainBlockIoTuneInfo *reply)
> +{
> +    char fullpath[100];

This can be a heap-allocated buffer.

> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) result = NULL;
> +    g_autofree char *groupCopy = NULL;
> +    virJSONValue *ret;
> +
> +
> +    g_snprintf(fullpath, sizeof(fullpath), "%s%s", "/objects/", gname);

and use g_strdup_printf to format it. Avoid static buffers.

> +    if (!(cmd = qemuMonitorJSONMakeCommand("qom-get",
> +                                           "s:path", fullpath,
> +                                           "s:property", "limits",
> +                                           NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &result) < 0)
> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, result) < 0)
> +        return -1;
> +
> +    if (!(ret = qemuMonitorJSONGetReply(cmd, result, VIR_JSON_TYPE_OBJECT))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("throttle group entry was not in expected format"));
> +        return -1;
> +    }


'qemuMonitorJSONGetReply' both calls 'qemuMonitorJSONCheckError' and
reports an error, thus you must not override it.

> +
> +    GET_THROTTLE_GROUP_VALUE("bps-total", total_bytes_sec);
> +    GET_THROTTLE_GROUP_VALUE("bps-read", read_bytes_sec);
> +    GET_THROTTLE_GROUP_VALUE("bps-write", write_bytes_sec);
> +    GET_THROTTLE_GROUP_VALUE("iops-total", total_iops_sec);
> +    GET_THROTTLE_GROUP_VALUE("iops-read", read_iops_sec);
> +    GET_THROTTLE_GROUP_VALUE("iops-write", write_iops_sec);
> +
> +    GET_THROTTLE_GROUP_VALUE("bps-total-max", total_bytes_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("bps-read-max", read_bytes_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("bps-write-max", write_bytes_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("iops-total-max", total_iops_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("iops-read-max", read_iops_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("iops-write-max", write_iops_sec_max);
> +    GET_THROTTLE_GROUP_VALUE("iops-size", size_iops_sec);
> +
> +    GET_THROTTLE_GROUP_VALUE("bps-total-max-length", total_bytes_sec_max_length);
> +    GET_THROTTLE_GROUP_VALUE("bps-read-max-length", read_bytes_sec_max_length);
> +    GET_THROTTLE_GROUP_VALUE("bps-write-max-length", write_bytes_sec_max_length);
> +    GET_THROTTLE_GROUP_VALUE("iops-total-max-length", total_iops_sec_max_length);
> +    GET_THROTTLE_GROUP_VALUE("iops-read-max-length", read_iops_sec_max_length);
> +    GET_THROTTLE_GROUP_VALUE("iops-write-max-length", write_iops_sec_max_length);
> +
> +    groupCopy = g_strdup(gname);

'groupCopy' is really pointless here ...

> +    reply->group_name = g_steal_pointer(&groupCopy);

just g_strdup into here directly.

> +
> +    return 0;
> +}
> +#undef GET_THROTTLE_GROUP_VALUE
> +
> +
>  int qemuMonitorJSONSystemWakeup(qemuMonitor *mon)
>  {
>      g_autoptr(virJSONValue) cmd = NULL;
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[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