[PATCH 6/7] util: Added a backing store NFS parser

[Peter Krempa]

On Tue, Dec 29, 2020 at 15:21:28 -0600, Ryan Gahagan wrote:
> Signed-off-by: Ryan Gahagan <rgahagan at cs.utexas.edu>
> ---
>  src/util/virstoragefile.c | 51 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index b5a5f267b9..ee8e31ce58 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -3805,6 +3805,56 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
>  }
>  
>  
> +static int
> +virStorageSourceParseBackingJSONNFS(virStorageSourcePtr src,
> +                                    virJSONValuePtr json,
> +                                    const char *jsonstr G_GNUC_UNUSED,
> +                                    int opaque G_GNUC_UNUSED)
> +{
> +    virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
> +    int uidStore = -1;
> +    int gidStore = -1;
> +    int gotUID = virJSONValueObjectGetNumberInt(json, "user", &uidStore);
> +    int gotGID = virJSONValueObjectGetNumberInt(json, "group", &gidStore);
> +    g_auto(virBuffer) userBuf = VIR_BUFFER_INITIALIZER;
> +    g_auto(virBuffer) groupBuf = VIR_BUFFER_INITIALIZER;
> +
> +    if (!server) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("missing 'server' attribute in JSON backing definition for NFS volume"));
> +        return -1;
> +    }
> +
> +    if (gotUID < 0 || gotGID < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("missing 'user' or 'group' attribute in JSON backing definition for NFS volume"));
> +        return -1;
> +    }
> +
> +    src->path = g_strdup(virJSONValueObjectGetString(json, "path"));

'path' is mandatory in 'BlockdevOptionsNfs' thus you must check that
it's present.

> +
> +    src->nfs_uid = (uid_t) uidStore;
> +    src->nfs_gid = (gid_t) gidStore;

This function must not fill in runtime data, just configuration. I
presume you did this to silence tests but you'll need to add a hack into
the test code rather than abusing this to fill runtime data.

Ideally in the future the runtime data will be split off into an opaque
sub-object so it will not be accessible in this code. Don't touch
nfs_uid/nfs_gid in this function at all.

> +
> +    virBufferAsprintf(&userBuf, "+%d", src->nfs_uid);
> +    virBufferAsprintf(&groupBuf, "+%d", src->nfs_gid);
> +    src->nfs_user = g_strdup(virBufferCurrentContent(&userBuf));
> +    src->nfs_group = g_strdup(virBufferCurrentContent(&groupBuf));

This is overkill including a pointless copy of the string.
Replace it by:

      src->nfs_user = g_strdup_printf("+%d", uidStore);
      ...


> +
> +    src->type = VIR_STORAGE_TYPE_NETWORK;
> +    src->protocol = VIR_STORAGE_NET_PROTOCOL_NFS;
> +
> +    src->hosts = g_new0(virStorageNetHostDef, 1);
> +    src->nhosts = 1;
> +
> +    if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts,
> +                                                          server) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
>  static int
>  virStorageSourceParseBackingJSONNVMe(virStorageSourcePtr src,
>                                       virJSONValuePtr json,
> @@ -3864,6 +3914,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = {
>      {"ssh", false, virStorageSourceParseBackingJSONSSH, 0},
>      {"rbd", false, virStorageSourceParseBackingJSONRBD, 0},
>      {"raw", true, virStorageSourceParseBackingJSONRaw, 0},
> +    {"nfs", false, virStorageSourceParseBackingJSONNFS, 0},
>      {"vxhs", false, virStorageSourceParseBackingJSONVxHS, 0},
>      {"nvme", false, virStorageSourceParseBackingJSONNVMe, 0},

The test case which tests this addition should be part of this patch
rather than stashed in the separate patch at the end of the series.