Re: [PATCH 5/5] qemu: qemuDomainObjPrivateXMLParseVcpu refactor
14. 7. 2025 v 19:09, Ján Tomko <jtomko(a)redhat.com>:
On a Monday in 2025, Kirill Shchetiniuk via Devel wrote:
> From: Kirill Shchetiniuk <kshcheti(a)redhat.com>
>
> Refactored the qemuDomainObjPrivateXMLParseVcpu function to use the
> appropriate virXMLPropUInt function to parse unsigned integers,
> avoiding unccessery string parsing operations.
>
> Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2d62bcc62b..2b505fbddb 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2811,28 +2811,18 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node,
> virDomainDef *def)
> {
> virDomainVcpuDef *vcpu;
> - g_autofree char *idstr = NULL;
> - g_autofree char *pidstr = NULL;
> unsigned int tmp;
>
> - idstr = virXMLPropString(node, "id");
> -
> - if (idstr &&
> - (virStrToLong_uip(idstr, NULL, 10, &idx) < 0)) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("cannot parse vcpu index '%1$s'"),
idstr);
> + if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED, &idx)
< 0)
> return -1;
Before, the id attribute was not required. After your change it's
mandatory.
A refactor should (ideally) not include any changes in behavior.
If it's safe to do such change, it should at least be mentioned in the
commit message.
I did not look into history whether it's safe to make it mandatory,
but in case the 'id' XML attribute is mandatory, it's pointless to
pass 'idx' as the function argument.
Jano
> - }
> +
> if (!(vcpu = virDomainDefGetVcpu(def, idx))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("invalid vcpu index '%1$u'"), idx);
> return -1;
> }
>
> - if (!(pidstr = virXMLPropString(node, "pid")))
> - return -1;
> -
> - if (virStrToLong_uip(pidstr, NULL, 10, &tmp) < 0)
> + if (virXMLPropUInt(node, "pid", 10, VIR_XML_PROP_REQUIRED, &tmp)
< 0)
> return -1;
>
> QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = tmp;
> --
> 2.49.0
>
<signature.asc>
Hi Jano,
oh yeah, I see now, I have incorrectly interpreted idstr NULL check as mandatory
requirement for id, will fix it within next series, thanks!
Kirill