On Tue, Dec 29, 2020 at 15:21:28 -0600, Ryan Gahagan wrote:
> Signed-off-by: Ryan Gahagan <rgahagan@xxxxxxxxxxxxx>
> ---
> 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.
