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