[PATCH v4 1/4] conf: domain: Introduce and use virDomainObjGetMessages()

[Martin Kletzander]

On Tue, Jul 13, 2021 at 10:54:14AM +0800, Luke Yue wrote:
>On Mon, 2021-07-12 at 19:32 +0800, Luke Yue wrote:
>> The test driver and qemu driver could share the same code in
>> virDomainGetMessages(), so extract it to a function.
>>
>> Signed-off-by: Luke Yue <lukedyue at gmail.com>
>> ---
>>  src/conf/domain_conf.c   | 53
>> ++++++++++++++++++++++++++++++++++++++++
>>  src/conf/domain_conf.h   |  5 ++++
>>  src/libvirt_private.syms |  1 +
>>  src/qemu/qemu_driver.c   | 34 +-------------------------
>>  4 files changed, 60 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 2d8ae7e860..09aa81f476 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -31134,3 +31134,56 @@ virHostdevIsVFIODevice(const
>> virDomainHostdevDef *hostdev)
>>          hostdev->source.subsys.type ==
>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
>>          hostdev->source.subsys.u.pci.backend ==
>> VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
>>  }
>> +
>> +
>> +/**
>> + * virDomainObjGetMessages:
>> + * @vm: domain object
>> + * @msgs: pointer to a variable to store messages
>> + * @flags: zero or more virDomainMessageType flags
>> + *
>> + * Returns number of messages stored in @msgs, -1 otherwise.
>> + */
>> +int
>> +virDomainObjGetMessages(virDomainObj *vm,
>> +                        char ***msgs,
>> +                        unsigned int flags)
>> +{
>> +    size_t i, n;
>> +    int nmsgs;
>> +    int rv = -1;
>> +
>> +    *msgs = NULL;
>> +    nmsgs = 0;
>> +    n = 0;
>> +

No need to split these any more, I'll join them with the declarations
before pushing.

>> +    if (!flags || (flags & VIR_DOMAIN_MESSAGE_TAINTING)) {
>> +        nmsgs += __builtin_popcount(vm->taint);
>> +        *msgs = g_renew(char *, *msgs, nmsgs+1);
>> +
>> +        for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
>> +            if (vm->taint & (1 << i)) {
>> +                (*msgs)[n++] = g_strdup_printf(
>> +                    _("tainted: %s"),
>> +                    _(virDomainTaintMessageTypeToString(i)));
>> +            }
>> +        }
>> +    }
>> +
>> +    if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
>> +        nmsgs += vm->ndeprecations;
>> +        *msgs = g_renew(char *, *msgs, nmsgs+1);
>> +
>> +        for (i = 0; i < vm->ndeprecations; i++) {
>> +            (*msgs)[n++] = g_strdup_printf(
>> +                _("deprecated configuration: %s"),
>> +                vm->deprecations[i]);
>> +        }
>> +    }
>> +
>> +    (*msgs)[nmsgs] = NULL;
>> +
>> +    rv = nmsgs;
>> +
>> +    return rv;
>> +}
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index c31531c93b..35cdfa3a9f 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -4142,3 +4142,8 @@ virHostdevIsMdevDevice(const
>> virDomainHostdevDef *hostdev)
>>  bool
>>  virHostdevIsVFIODevice(const virDomainHostdevDef *hostdev)
>>      ATTRIBUTE_NONNULL(1);
>> +
>> +int
>> +virDomainObjGetMessages(virDomainObj *vm,
>> +                        char ***msgs,
>> +                        unsigned int flags);
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index cc7533a707..b20bcb2c65 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -564,6 +564,7 @@ virDomainObjDeprecation;
>>  virDomainObjEndAPI;
>>  virDomainObjFormat;
>>  virDomainObjGetDefs;
>> +virDomainObjGetMessages;
>>  virDomainObjGetMetadata;
>>  virDomainObjGetOneDef;
>>  virDomainObjGetOneDefState;
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 235f575901..592e1236e7 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -20319,8 +20319,6 @@ qemuDomainGetMessages(virDomainPtr dom,
>>  {
>>      virDomainObj *vm = NULL;
>>      int rv = -1;
>> -    size_t i, n;
>> -    int nmsgs;
>>  
>>      virCheckFlags(VIR_DOMAIN_MESSAGE_DEPRECATION |
>>                    VIR_DOMAIN_MESSAGE_TAINTING, -1);
>> @@ -20331,37 +20329,7 @@ qemuDomainGetMessages(virDomainPtr dom,
>>      if (virDomainGetMessagesEnsureACL(dom->conn, vm->def) < 0)
>>          goto cleanup;
>>  
>> -    *msgs = NULL;
>> -    nmsgs = 0;
>> -    n = 0;
>> -
>> -    if (!flags || (flags & VIR_DOMAIN_MESSAGE_TAINTING)) {
>> -        nmsgs += __builtin_popcount(vm->taint);
>> -        *msgs = g_renew(char *, *msgs, nmsgs+1);
>> -
>> -        for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
>> -            if (vm->taint & (1 << i)) {
>> -                (*msgs)[n++] = g_strdup_printf(
>> -                    _("tainted: %s"),
>> -                    _(virDomainTaintMessageTypeToString(i)));
>> -            }
>> -        }
>> -    }
>> -
>> -    if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
>> -        nmsgs += vm->ndeprecations;
>> -        *msgs = g_renew(char *, *msgs, nmsgs+1);
>> -
>> -        for (i = 0; i < vm->ndeprecations; i++) {
>> -            (*msgs)[n++] = g_strdup_printf(
>> -                _("deprecated configuration: %s"),
>> -                vm->deprecations[i]);
>> -        }
>> -    }
>> -
>> -    (*msgs)[nmsgs] = NULL;
>> -
>> -    rv = nmsgs;
>> +    rv = virDomainObjGetMessages(vm, msgs, flags);
>>  
>>   cleanup:
>>      virDomainObjEndAPI(&vm);
>
>I just find out that maybe I should extract the function to
>hypervisor/domain_driver.c? If the left parts of the patch set are fine
>then I will send v5 to extract the function to
>hypervisor/domain_driver.
>

I would rather go for domain_conf, we can always move it later if
needed.

>Thanks
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210803/6936ca99/attachment-0001.sig>