PATCH v2: Implement NFS Protocol
Per Issue 90[1], QEMU supports using an NFS disk but Libvirt had no support for attaching a disk of this type. This patch series adds in this capability as well as the tests necessary to make sure it works correctly. The changes in this second version over the previous patch series for this issue primarily address feedback. Documentation was updated and the XML <nfs> element has been renamed as an <identity> element. The code for checking user XML for an NFS protocol has been relocated to the validation method and other pieces of code were refactored per suggestion. Tests have been altered to reflect the new XML element and the fact that NFS does not allow hosts to have a port. References: [1]: https://gitlab.com/libvirt/libvirt/-/issues/90
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/libxl/libxl_conf.c | 1 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 3 +++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_snapshot.c | 3 +++ src/util/virstoragefile.c | 6 ++++++ src/util/virstoragefile.h | 1 + 8 files changed, 18 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 00748e21e8..6a8ae27f54 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -941,6 +941,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index ba0942601f..17b93d0f5c 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1600,6 +1600,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4640e339c0..b224a550f3 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1180,6 +1180,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break; + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportEnumRangeError(virStorageNetProtocol, src->protocol); @@ -2111,6 +2112,7 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: @@ -2502,6 +2504,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: case VIR_STORAGE_NET_PROTOCOL_FTP: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 02f956ce48..be06042cce 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1044,6 +1044,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, _("'ssh' protocol is not yet supported")); return NULL; + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bfb6e23942..d91c32b2c5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9630,6 +9630,8 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Assumed NFS doesn't support TLS (needs Kerberos) */ case VIR_STORAGE_NET_PROTOCOL_SSH: if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 15494c3415..7e89a8839b 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -413,6 +413,7 @@ qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " @@ -501,6 +502,7 @@ qemuSnapshotPrepareDiskExternalActive(virDomainObjPtr vm, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external active snapshots are not supported on " @@ -631,6 +633,7 @@ qemuSnapshotPrepareDiskInternal(virDomainDiskDefPtr disk, case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("internal inactive snapshots are not supported on " diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3db85d8b89..1176630282 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -90,6 +90,7 @@ VIR_ENUM_IMPL(virStorageNetProtocol, "tftp", "ssh", "vxhs", + "nfs", ); VIR_ENUM_IMPL(virStorageNetHostTransport, @@ -2999,6 +3000,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SSH: case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: virReportError(VIR_ERR_INTERNAL_ERROR, _("malformed backing store path for protocol %s"), protocol); @@ -4450,6 +4452,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) case VIR_STORAGE_NET_PROTOCOL_VXHS: return 9999; + case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Port is not supported by NFS, so no default is provided */ + return 0; + case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: return 0; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 2452b967b2..e1fbbafe26 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -135,6 +135,7 @@ typedef enum { VIR_STORAGE_NET_PROTOCOL_TFTP, VIR_STORAGE_NET_PROTOCOL_SSH, VIR_STORAGE_NET_PROTOCOL_VXHS, + VIR_STORAGE_NET_PROTOCOL_NFS, VIR_STORAGE_NET_PROTOCOL_LAST } virStorageNetProtocol; -- 2.29.2
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/util/virstoragefile.c | 8 ++++++++ src/util/virstoragefile.h | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 1176630282..3097e11984 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2293,6 +2293,11 @@ virStorageSourceCopy(const virStorageSource *src, def->ssh_host_key_check_disabled = src->ssh_host_key_check_disabled; def->ssh_user = g_strdup(src->ssh_user); + def->nfs_user = g_strdup(src->nfs_user); + def->nfs_group = g_strdup(src->nfs_group); + def->nfs_uid = src->nfs_uid; + def->nfs_gid = src->nfs_gid; + return g_steal_pointer(&def); } @@ -2533,6 +2538,9 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->ssh_user); + VIR_FREE(def->nfs_user); + VIR_FREE(def->nfs_group); + virStorageSourceInitiatorClear(&def->initiator); /* clear everything except the class header as the object APIs diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index e1fbbafe26..47d901a478 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -385,6 +385,14 @@ struct _virStorageSource { /* these must not be used apart from formatting the output JSON in the qemu driver */ char *ssh_user; bool ssh_host_key_check_disabled; + + /* nfs_user and nfs_group store the strings passed in by the user for NFS params. + * nfs_uid and nfs_gid represent the converted/looked up ID numbers which are used + * during run time, and are not based on the configuration */ + char *nfs_user; + char *nfs_group; + uid_t nfs_uid; + gid_t nfs_gid; }; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref); -- 2.29.2
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/formatdomain.rst | 24 ++++++++++++++++++++++-- docs/schemas/domaincommon.rng | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 1189795974..4402c52461 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2370,6 +2370,14 @@ paravirtualized driver is specified via the ``disk`` element. </source> <target dev='sdb' bus='scsi'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nfs' name='PATH'> + <host name='example.com'/> + <identity user='USER' group='GROUP'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> <disk type='network' device='lun'> <driver name='qemu' type='raw'/> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> @@ -2491,7 +2499,7 @@ paravirtualized driver is specified via the ``disk`` element. ``network`` The ``protocol`` attribute specifies the protocol to access to the requested image. Possible values are "nbd", "iscsi", "rbd", "sheepdog", - "gluster", "vxhs", "http", "https", "ftp", ftps", or "tftp". + "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", or "tftp". For any ``protocol`` other than ``nbd`` an additional attribute ``name`` is mandatory to specify which volume/image will be used. @@ -2601,12 +2609,15 @@ paravirtualized driver is specified via the ``disk`` element. sheepdog one of the sheepdog servers (default is localhost:7000) zero or one 7000 gluster a server running glusterd daemon one or more ( :since:`Since 2.1.0` ), just one prior to that 24007 vxhs a server running Veritas HyperScale daemon only one 9999 + nfs a server running Network File System only one ( :since:`Since 7.0.0` ) must be omitted ======== ======================================================= ============================================================ ================ gluster supports "tcp", "rdma", "unix" as valid values for the transport attribute. nbd supports "tcp" and "unix". Others only support "tcp". If nothing is specified, "tcp" is assumed. If the transport is "unix", the - socket attribute specifies the path to an AF_UNIX socket. + socket attribute specifies the path to an AF_UNIX socket. nfs only + supports the use of a "tcp" transport, and does not support using a + port at all so it must be omitted. ``snapshot`` The ``name`` attribute of ``snapshot`` element can optionally specify an @@ -2683,6 +2694,15 @@ paravirtualized driver is specified via the ``disk`` element. ``timeout`` Specifies the connection timeout for protocols which support it. Note that '0' is considered as if the value is not provided. :since:`Since 6.2.0` + ``identity`` + When using an ``nfs`` protocol, this is used to provide information on the + configuration of the user and group. The element has two attributes, + ``user`` and ``group``. The user can provide these elements as user or + group strings, or as user and group ID numbers directly if the string + is formatted using a "+" at the beginning of the ID number. If either + of these attributes is omitted, then that field is assumed to be the + default value for the current system. If both ``user`` and ``group`` + are intended to be default, then the entire element may be omitted. For a "file" or "volume" disk type which represents a cdrom or floppy (the ``device`` attribute), it is possible to define policy what to do with the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 24b4994670..c3a4dd8ebc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1774,6 +1774,21 @@ </element> </define> + <define name="diskSourceNetworkNFS"> + <element name="identity"> + <optional> + <attribute name="user"> + <ref name="genericName"/> + </attribute> + </optional> + <optional> + <attribute name="group"> + <ref name="genericName"/> + </attribute> + </optional> + </element> + </define> + <define name="diskSourceNetworkProtocolRBD"> <element name="source"> <interleave> @@ -2039,6 +2054,22 @@ </element> </define> + <define name="diskSourceNetworkProtocolNFS"> + <element name="source"> + <interleave> + <attribute name="protocol"> + <choice> + <value>nfs</value> + </choice> + </attribute> + <attribute name="name"/> + <ref name="diskSourceCommon"/> + <ref name="diskSourceNetworkHost"/> + <ref name="diskSourceNetworkNFS"/> + </interleave> + </element> + </define> + <define name="diskSourceNetwork"> <attribute name="type"> <value>network</value> @@ -2053,6 +2084,7 @@ <ref name="diskSourceNetworkProtocolFTPS"/> <ref name="diskSourceNetworkProtocolSimple"/> <ref name="diskSourceNetworkProtocolVxHS"/> + <ref name="diskSourceNetworkProtocolNFS"/> </choice> </define> -- 2.29.2
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 453e06491e..96ee009058 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6888,6 +6888,23 @@ virDomainStorageNetworkParseHosts(xmlNodePtr node, } +static void +virDomainStorageNetworkParseNFS(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virStorageSourcePtr src) +{ + xmlNodePtr nfsIdentityNode = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + ctxt->node = node; + + if ((nfsIdentityNode = virXPathNode("./identity", ctxt))) { + src->nfs_user = virXMLPropString(nfsIdentityNode, "user"); + src->nfs_group = virXMLPropString(nfsIdentityNode, "group"); + } +} + + static int virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, xmlXPathContextPtr ctxt, @@ -8250,6 +8267,9 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, if (virDomainStorageNetworkParseHosts(node, ctxt, &src->hosts, &src->nhosts) < 0) return -1; + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS) + virDomainStorageNetworkParseNFS(node, ctxt, src); + virStorageSourceNetworkAssignDefaultPorts(src); virStorageSourceInitiatorParseXML(ctxt, &src->initiator); @@ -23851,6 +23871,17 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferAddLit(childBuf, "/>\n"); } + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + (src->nfs_user || src->nfs_group)) { + virBufferAddLit(childBuf, "<identity"); + + virBufferEscapeString(childBuf, " user='%s'", src->nfs_user); + virBufferEscapeString(childBuf, " group='%s'", src->nfs_group); + + virBufferAddLit(childBuf, "/>\n"); + } + + virBufferEscapeString(childBuf, "<snapshot name='%s'/>\n", src->snapshot); virBufferEscapeString(childBuf, "<config file='%s'/>\n", src->configFile); -- 2.29.2
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/qemu/qemu_block.c | 67 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.c | 70 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b224a550f3..cef2f7d050 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -574,6 +574,29 @@ qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) } +/** + * qemuBlockStorageSourceBuildJSONNFSServer(virStorageNetHostDefPtr host) + * @host: the virStorageNetHostDefPtr definition to build + * + * Formats @hosts into a json object conforming to the 'NFSServer' type + * in qemu. + * + * Returns a virJSONValuePtr for a single server. + */ +static virJSONValuePtr +qemuBlockStorageSourceBuildJSONNFSServer(virStorageNetHostDefPtr host) +{ + virJSONValuePtr ret = NULL; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:host", host->name, + "s:type", "inet", + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceBuildHostsJSONInetSocketAddress: * @src: disk storage source @@ -674,6 +697,38 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, } +static virJSONValuePtr +qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) server = NULL; + virJSONValuePtr ret = NULL; + + if (!(server = qemuBlockStorageSourceBuildJSONNFSServer(&src->hosts[0]))) + return NULL; + + /* NFS disk specification example: + * { driver:"nfs", + * user: "0", + * group: "0", + * path: "/foo/bar/baz", + * server: {type:"tcp", host:"1.2.3.4"}} + */ + ignore_value(virJSONValueObjectCreate(&ret, + "a:server", &server, + "S:path", src->path, NULL)); + + if (src->nfs_uid != -1 && + virJSONValueObjectAdd(ret, "i:user", src->nfs_uid, NULL) < 0) + return NULL; + + if (src->nfs_gid != -1 && + virJSONValueObjectAdd(ret, "i:group", src->nfs_gid, NULL) < 0) + return NULL; + + return ret; +} + + static virJSONValuePtr qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src, bool onlytarget) @@ -1181,6 +1236,11 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, break; case VIR_STORAGE_NET_PROTOCOL_NFS: + driver = "nfs"; + if (!(fileprops = qemuBlockStorageSourceGetNFSProps(src))) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: virReportEnumRangeError(virStorageNetProtocol, src->protocol); @@ -2500,11 +2560,16 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, return -1; break; + case VIR_STORAGE_NET_PROTOCOL_NFS: + driver = "nfs"; + if (!(location = qemuBlockStorageSourceGetNFSProps(src))) + return -1; + break; + /* unsupported/impossible */ case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_ISCSI: case VIR_STORAGE_NET_PROTOCOL_VXHS: - case VIR_STORAGE_NET_PROTOCOL_NFS: case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: case VIR_STORAGE_NET_PROTOCOL_FTP: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d91c32b2c5..fa427a1087 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4626,6 +4626,37 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS) { + /* NFS protocol may only be used if current QEMU supports blockdev */ + if (!blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' protocol is not supported with this QEMU binary")); + return -1; + } + + /* NFS protocol must have exactly one host */ + if (src->nhosts != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' protocol requires the usage of exactly one host")); + return -1; + } + + /* NFS can only use a TCP protocol */ + if (src->hosts[0].transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' host must use TCP protocol")); + return -1; + } + + /* NFS host cannot have a port */ + if (src->hosts[0].port != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("port cannot be specified in 'nfs' protocol host")); + return -1; + } + } + return 0; } @@ -9590,6 +9621,42 @@ qemuProcessPrepareStorageSourceTLSNBD(virStorageSourcePtr src, } +/* qemuPrepareStorageSourceNFS: + * @src: source for a disk + * + * If src is an NFS source, translate nfs_user and nfs_group + * into a uid and gid field. If these strings are empty (ie "") + * then provide the hypervisor default uid and gid. + */ +static int +qemuDomainPrepareStorageSourceNFS(virStorageSourcePtr src) +{ + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) + return 0; + + if (src->protocol != VIR_STORAGE_NET_PROTOCOL_NFS) + return 0; + + if (src->nfs_user) { + if (virGetUserID(src->nfs_user, &src->nfs_uid) < 0) + return -1; + } else { + /* -1 indicates default UID */ + src->nfs_uid = -1; + } + + if (src->nfs_group) { + if (virGetGroupID(src->nfs_group, &src->nfs_gid) < 0) + return -1; + } else { + /* -1 indicates default GID */ + src->nfs_gid = -1; + } + + return 0; +} + + /* qemuProcessPrepareStorageSourceTLS: * @source: source for a disk * @cfg: driver configuration @@ -10401,6 +10468,9 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, priv) < 0) return -1; + if (qemuDomainPrepareStorageSourceNFS(src) < 0) + return -1; + return 0; } -- 2.29.2
On Wed, Jan 06, 2021 at 15:32:30 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/qemu/qemu_block.c | 67 +++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.c | 70 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b224a550f3..cef2f7d050 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c
[...]
@@ -674,6 +697,38 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, }
+static virJSONValuePtr +qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) server = NULL; + virJSONValuePtr ret = NULL; + + if (!(server = qemuBlockStorageSourceBuildJSONNFSServer(&src->hosts[0]))) + return NULL; + + /* NFS disk specification example: + * { driver:"nfs", + * user: "0", + * group: "0", + * path: "/foo/bar/baz", + * server: {type:"tcp", host:"1.2.3.4"}} + */ + ignore_value(virJSONValueObjectCreate(&ret, + "a:server", &server, + "S:path", src->path, NULL));
Not checking return here means that 'ret' can still be NULL after this call ...
+ + if (src->nfs_uid != -1 && + virJSONValueObjectAdd(ret, "i:user", src->nfs_uid, NULL) < 0)
and virJSONValueObjectAdd does not check if the first argument is non-NULL and dereferences it always, thus this can lead to a crash. I'll add the check of return value before pushing.
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/util/virstoragefile.c | 49 +++++++++++++++++++++++++++++++++++++++ tests/virstoragetest.c | 13 +++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3097e11984..d67f0f2c3f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3652,6 +3652,54 @@ 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); + + 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")); + if (!src->path) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'path' attribute in JSON backing definition for NFS volume")); + return -1; + } + + src->nfs_user = g_strdup_printf("+%d", uidStore); + src->nfs_group = g_strdup_printf("+%d", gidStore); + + 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, @@ -3711,6 +3759,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}, }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index a376154def..86c7cd910c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1624,6 +1624,19 @@ mymain(void) "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n" " <host name='example.com' port='9999'/>\n" "</source>\n"); + TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nfs\"," + "\"user\":2," + "\"group\":9," + "\"path\":\"/foo/bar/baz\"," + "\"server\": { \"host\":\"example.com\"," + "\"type\":\"inet\"" + "}" + "}" + "}", + "<source protocol='nfs' name='/foo/bar/baz'>\n" + " <host name='example.com'/>\n" + " <identity user='+2' group='+9'/>\n" + "</source>\n"); TEST_BACKING_PARSE_FULL("json:{ \"driver\": \"raw\"," "\"offset\": 10752," "\"size\": 4063232," -- 2.29.2
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- .../disk-network-nfs.x86_64-latest.args | 56 +++++++++++++++++++ tests/qemuxml2argvdata/disk-network-nfs.xml | 48 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + ...isk-network-nfs-inactive.x86_64-latest.xml | 54 ++++++++++++++++++ .../disk-network-nfs.x86_64-latest.xml | 54 ++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 214 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-nfs.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-nfs.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-nfs-inactive.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-nfs.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/disk-network-nfs.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-nfs.x86_64-latest.args new file mode 100644 index 0000000000..b0bc83bfc0 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-nfs.x86_64-latest.args @@ -0,0 +1,56 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-cpu qemu64 \ +-m 214 \ +-object memory-backend-ram,id=pc.ram,size=224395264 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"nfs","server":{"host":"example.com","type":"inet"},\ +"path":"/foo/bar/baz","user":6234,"group":12354,\ +"node-name":"libvirt-3-storage","cache":{"direct":true,"no-flush":false},\ +"auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,\ +"cache":{"direct":true,"no-flush":false},"driver":"raw",\ +"file":"libvirt-3-storage"}' \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-3-format,\ +id=virtio-disk0,bootindex=1,write-cache=on,\ +serial=eb90327c-8302-4725-9e1b-4e85ed4dc251 \ +-blockdev '{"driver":"nfs","server":{"host":"example.org","type":"inet"},\ +"path":"/backing/store/nfs","user":1234,"group":5678,\ +"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2",\ +"file":"libvirt-2-storage","backing":null}' \ +-blockdev '{"driver":"gluster","volume":"Volume2","path":"Image",\ +"server":[{"type":"unix","path":"/path/to/sock"}],"debug":4,\ +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\ +"file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ +-device virtio-blk-pci,bus=pci.0,addr=0x2,drive=libvirt-1-format,\ +id=virtio-disk1 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-nfs.xml b/tests/qemuxml2argvdata/disk-network-nfs.xml new file mode 100644 index 0000000000..df7b104a8e --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-nfs.xml @@ -0,0 +1,48 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='nfs' name='/foo/bar/baz'> + <host name='example.com'/> + <identity user='+6234' group='+12354'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume2/Image'> + <host transport='unix' socket='/path/to/sock'/> + </source> + <backingStore type='network' index='1'> + <format type='qcow2'/> + <source protocol='nfs' name='/backing/store/nfs'> + <host name='example.org'/> + <identity user='+1234' group='+5678'/> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d2712e0dce..afc696ec69 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1242,6 +1242,7 @@ mymain(void) DO_TEST_CAPS_VER("disk-network-source-auth", "2.12.0"); DO_TEST_CAPS_LATEST("disk-network-source-auth"); DO_TEST("disk-network-vxhs", QEMU_CAPS_VXHS); + DO_TEST_CAPS_LATEST("disk-network-nfs"); driver.config->vxhsTLS = 1; driver.config->nbdTLSx509secretUUID = g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea"); driver.config->vxhsTLSx509secretUUID = g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea"); diff --git a/tests/qemuxml2xmloutdata/disk-network-nfs-inactive.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-network-nfs-inactive.x86_64-latest.xml new file mode 100644 index 0000000000..87e341411a --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-network-nfs-inactive.x86_64-latest.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='nfs' name='/foo/bar/baz'> + <host name='example.com'/> + <identity user='+6234' group='+12354'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume2/Image'> + <host transport='unix' socket='/path/to/sock'/> + </source> + <backingStore type='network'> + <format type='qcow2'/> + <source protocol='nfs' name='/backing/store/nfs'> + <host name='example.org'/> + <identity user='+1234' group='+5678'/> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/disk-network-nfs.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-network-nfs.x86_64-latest.xml new file mode 100644 index 0000000000..d920d46adf --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-network-nfs.x86_64-latest.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='nfs' name='/foo/bar/baz'> + <host name='example.com'/> + <identity user='+6234' group='+12354'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='gluster' name='Volume2/Image'> + <host transport='unix' socket='/path/to/sock'/> + </source> + <backingStore type='network' index='1'> + <format type='qcow2'/> + <source protocol='nfs' name='/backing/store/nfs'> + <host name='example.org'/> + <identity user='+1234' group='+5678'/> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f8bca9f559..b8651901ce 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -302,6 +302,7 @@ mymain(void) DO_TEST("disk-network-source-auth", NONE); DO_TEST("disk-network-sheepdog", NONE); DO_TEST("disk-network-vxhs", NONE); + DO_TEST_CAPS_LATEST("disk-network-nfs"); DO_TEST("disk-network-tlsx509-nbd", NONE); DO_TEST("disk-network-tlsx509-vxhs", NONE); DO_TEST("disk-nvme", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_QCOW2_LUKS); -- 2.29.2
On Wed, Jan 06, 2021 at 15:32:25 -0600, Ryan Gahagan wrote:
Per Issue 90[1], QEMU supports using an NFS disk but Libvirt had no support for attaching a disk of this type. This patch series adds in this capability as well as the tests necessary to make sure it works correctly.
The changes in this second version over the previous patch series for this issue primarily address feedback. Documentation was updated and the XML <nfs> element has been renamed as an <identity> element. The code for checking user XML for an NFS protocol has been relocated to the validation method and other pieces of code were refactored per suggestion. Tests have been altered to reflect the new XML element and the fact that NFS does not allow hosts to have a port.
References: [1]: https://gitlab.com/libvirt/libvirt/-/issues/90
We usually prefer if you describe what you are adding in the actual commit messages since they are recorded in git history instead of the cover letter which gets forgotten. Anyways. Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com> I'll fix the one problem pointed out in the specific commit and I'll push the patches later today.
participants (2)
-
Peter Krempa -
Ryan Gahagan