[libvirt] [PATCH v2 09/15] qemu_firmware: Introduce qemuFirmwareFetchConfigs

[Laszlo Ersek]

On 03/07/19 13:46, Michal Privoznik wrote:
> On 3/7/19 12:55 PM, Daniel P. Berrangé wrote:
>> On Thu, Mar 07, 2019 at 10:29:19AM +0100, Michal Privoznik wrote:
>>> Implementation for yet another part of firmware description
>>> specification. This one covers selecting which files to parse.
>>>
>>> There are three locations from which description files can be
>>> loaded. In order of preference, from most generic to most
>>> specific these are:
>>>
>>>    /usr/share/qemu/firmware
>>>    /etc/qemu/firmware
>>>    $XDG_CONFIG_HOME/qemu/firmware
>>>
>>> If a file is found in two or more locations then the most specific
>>> one is used. Moreover, if file is empty then it means it is
>>> overriding some generic description and disabling it.
>>>
>>> Again, this is described in more details and with nice examples
>>> in firmware.json specification (qemu commit 3a0adfc9bf).
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>   src/qemu/qemu_firmware.c | 134 +++++++++++++++++++++++++++++++++++++++
>>>   src/qemu/qemu_firmware.h |   3 +
>>>   2 files changed, 137 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
>>> index 8f718ee2a6..a818f60c91 100644
>>> --- a/src/qemu/qemu_firmware.c
>>> +++ b/src/qemu/qemu_firmware.c
>>> @@ -21,9 +21,11 @@
>>>   #include <config.h>
>>>     #include "qemu_firmware.h"
>>> +#include "configmake.h"
>>>   #include "qemu_capabilities.h"
>>>   #include "virarch.h"
>>>   #include "virfile.h"
>>> +#include "virhash.h"
>>>   #include "virjson.h"
>>>   #include "virlog.h"
>>>   #include "virstring.h"
>>> @@ -899,3 +901,135 @@ qemuFirmwareFormat(qemuFirmwarePtr fw)
>>>         return virJSONValueToString(doc, true);
>>>   }
>>> +
>>> +
>>> +static int
>>> +qemuFirmwareBuildFileList(virHashTablePtr files, const char *dir)
>>> +{
>>> +    DIR *dirp;
>>> +    struct dirent *ent = NULL;
>>> +    int rc;
>>> +    int ret = -1;
>>> +
>>> +    if ((rc = virDirOpenIfExists(&dirp, dir)) < 0)
>>> +        return -1;
>>> +
>>> +    if (rc == 0)
>>> +        return 0;
>>> +
>>> +    while ((rc = virDirRead(dirp, &ent, dir)) > 0) {
>>> +        VIR_AUTOFREE(char *) filename = NULL;
>>> +        VIR_AUTOFREE(char *) path = NULL;
>>> +
>>> +        if (ent->d_type != DT_REG && ent->d_type != DT_LNK)
>>> +            continue;
>>> +
>>> +        if (STRPREFIX(ent->d_name, "."))
>>> +            continue;
>>> +
>>> +        if (VIR_STRDUP(filename, ent->d_name) < 0)
>>> +            goto cleanup;
>>> +
>>> +        if (virAsprintf(&path, "%s/%s", dir, filename) < 0)
>>> +            goto cleanup;
>>> +
>>> +        if (virHashUpdateEntry(files, filename, path) < 0)
>>> +            goto cleanup;
>>> +
>>> +        path = NULL;
>>> +    }
>>> +
>>> +    if (rc < 0)
>>> +        goto cleanup;
>>> +
>>> +    ret = 0;
>>> + cleanup:
>>> +    virDirClose(&dirp);
>>> +    return ret;
>>> +}
>>> +
>>> +static int
>>> +qemuFirmwareFilesSorter(const virHashKeyValuePair *a,
>>> +                        const virHashKeyValuePair *b)
>>> +{
>>> +    return strcmp(a->key, b->key);
>>> +}
>>> +
>>> +#define QEMU_FIRMWARE_SYSTEM_LOCATION PREFIX "/share/qemu/firmware"
>>> +#define QEMU_FIRMWARE_ETC_LOCATION SYSCONFDIR "/qemu/firmware"
>>> +
>>> +int
>>> +qemuFirmwareFetchConfigs(char ***firmwares)
>>> +{
>>> +    VIR_AUTOPTR(virHashTable) files = NULL;
>>> +    VIR_AUTOFREE(char *) homeConfig = NULL;
>>> +    VIR_AUTOFREE(char *) xdgConfig = NULL;
>>> +    VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL;
>>> +    virHashKeyValuePairPtr tmp = NULL;
>>> +
>>> +    *firmwares = NULL;
>>> +
>>> +    if (VIR_STRDUP(xdgConfig, virGetEnvBlockSUID("XDG_CONFIG_HOME"))
>>> < 0)
>>> +        return -1;
>>> +
>>> +    if (!xdgConfig) {
>>> +        VIR_AUTOFREE(char *) home = virGetUserDirectory();
>>> +
>>> +        if (!home)
>>> +            return -1;
>>> +
>>> +        if (virAsprintf(&xdgConfig, "%s/.config", home) < 0)
>>> +            return -1;
>>> +    }
>>> +
>>> +    if (virAsprintf(&homeConfig, "%s/qemu/firmware", xdgConfig) < 0)
>>> +        return -1;
>>> +
>>> +    if (!(files = virHashCreate(10, virHashValueFree)))
>>> +        return -1;
>>> +
>>> +    if (qemuFirmwareBuildFileList(files,
>>> QEMU_FIRMWARE_SYSTEM_LOCATION) < 0)
>>> +        return -1;
>>> +
>>> +    if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_ETC_LOCATION)
>>> < 0)
>>> +        return -1;
>>> +
>>> +    if (qemuFirmwareBuildFileList(files, homeConfig) < 0)
>>> +        return -1;
>>
>> I wonder if it really makes sense to read /root/ for this. Normally we
>> would
>> only look at the home config for unprivileged daemon, eg we don't look in
>> /root for finding PKI x509 certs IIRC.
> 
> Fair enough. Root is able to put configs into /etc/qemu/firmware
> anyways. Will fix this locally for now.

Please carry forward my R-b, given up-thread, when you implement the
tweak suggested by Dan.

Thanks
Laszlo