[PATCH 1/7] conf: Add NFS disk protocol
Per Issue 90, Libvirt does not support attaching an NFS disk even though QEMU has added support for it. This series of patches seeks to implement this support in Libvirt and begins by adding in flags for an NFS disk. 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 b06a086e18..c58f39ebf1 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 fac93118fd..103dade0e7 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, @@ -3152,6 +3153,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); @@ -4627,6 +4629,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) case VIR_STORAGE_NET_PROTOCOL_VXHS: return 9999; + case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Per https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/htm... */ + return 2049; + 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 87763cf389..c5d5f0233a 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 | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 103dade0e7..b5a5f267b9 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2446,6 +2446,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); } @@ -2686,6 +2691,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 c5d5f0233a..64fc519f87 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -385,6 +385,11 @@ 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; + + char *nfs_user; + char *nfs_group; + uid_t nfs_uid; + gid_t nfs_gid; }; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref); -- 2.29.2
On Tue, Dec 29, 2020 at 15:21:24 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/util/virstoragefile.c | 8 ++++++++ src/util/virstoragefile.h | 5 +++++ 2 files changed, 13 insertions(+)
[...]
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c5d5f0233a..64fc519f87 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -385,6 +385,11 @@ 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; + + char *nfs_user; + char *nfs_group; + uid_t nfs_uid; + gid_t nfs_gid;
Please add comments for nfs_uid and nfs_gid which state that the variables hold the converted/looked up uid/gid which is used at runtinme . This will help any further reader of the code see what they are used for and that they are not actually based on the configuration but are rather runtime (such as when we'll be adding driver-specific private data for virStorageSource).
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/formatdomain.rst | 11 +++++++++- docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512939679b..23a7bca643 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' port='2049'/> + <nfs 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,6 +2609,7 @@ 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` ) 2049 ======== ======================================================= ============================================================ ================ gluster supports "tcp", "rdma", "unix" as valid values for the transport diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 17e25f14f2..4ddbf13ec4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1774,6 +1774,27 @@ </element> </define> + <define name="diskSourceNetworkNFS"> + <element name="nfs"> + <optional> + <attribute name="user"> + <choice> + <ref name="unsignedInt"/> + <ref name="genericName"/> + </choice> + </attribute> + </optional> + <optional> + <attribute name="group"> + <choice> + <ref name="unsignedInt"/> + <ref name="genericName"/> + </choice> + </attribute> + </optional> + </element> + </define> + <define name="diskSourceNetworkProtocolRBD"> <element name="source"> <interleave> @@ -2039,6 +2060,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 +2090,7 @@ <ref name="diskSourceNetworkProtocolFTPS"/> <ref name="diskSourceNetworkProtocolSimple"/> <ref name="diskSourceNetworkProtocolVxHS"/> + <ref name="diskSourceNetworkProtocolNFS"/> </choice> </define> -- 2.29.2
On Tue, Dec 29, 2020 at 03:21:25PM -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/formatdomain.rst | 11 +++++++++- docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512939679b..23a7bca643 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' port='2049'/> + <nfs user='USER' group='GROUP'/>
I'd be inclined to call this <identity> not <nfs> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Jan 04, 2021 at 13:19:25 +0000, Daniel Berrange wrote:
On Tue, Dec 29, 2020 at 03:21:25PM -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/formatdomain.rst | 11 +++++++++- docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512939679b..23a7bca643 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' port='2049'/> + <nfs user='USER' group='GROUP'/>
I'd be inclined to call this <identity> not <nfs>
Yeah, that seems a bit better
On Tue, Dec 29, 2020 at 15:21:25 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/formatdomain.rst | 11 +++++++++- docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512939679b..23a7bca643 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' port='2049'/> + <nfs 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".
The docs is missing any description of the user/group fields. Please include the syntax as well. Additionally we tend to include any caveats of the initial implementation. In this case you should include that NFS-backed disks work only with TCP and port must be omitted.
For any ``protocol`` other than ``nbd`` an additional attribute ``name`` is mandatory to specify which volume/image will be used. @@ -2601,6 +2609,7 @@ 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` ) 2049
You've dropped the code which sets the default port so this isn't accurate.
======== ======================================================= ============================================================ ================
gluster supports "tcp", "rdma", "unix" as valid values for the transport diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 17e25f14f2..4ddbf13ec4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1774,6 +1774,27 @@ </element> </define>
+ <define name="diskSourceNetworkNFS"> + <element name="nfs">
This will need changes after Daniel's suggestion.
+ <optional> + <attribute name="user"> + <choice> + <ref name="unsignedInt"/>
Note that 'unsignedInt' doesn't actually allow the '+' prefix.
+ <ref name="genericName"/>
But both numbers and strings started with + are covered by genericName, so the choice isn't required.
+ </choice> + </attribute> + </optional> + <optional> + <attribute name="group"> + <choice> + <ref name="unsignedInt"/> + <ref name="genericName"/>
Same here.
+ </choice> + </attribute> + </optional> + </element> + </define> + <define name="diskSourceNetworkProtocolRBD"> <element name="source"> <interleave> @@ -2039,6 +2060,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"/>
This allows also 'port' and non TCP transports. It's okay to simplify the schema though and use the generic type. The code will need to reject those when we'll validate whether the disk is possible to represent for qemu.
On Mon, Jan 4, 2021 at 8:24 AM Peter Krempa <pkrempa@redhat.com> wrote:
On Tue, Dec 29, 2020 at 15:21:25 -0600, Ryan Gahagan wrote:
+ <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"/>
This allows also 'port' and non TCP transports. It's okay to simplify the schema though and use the generic type. The code will need to reject those when we'll validate whether the disk is possible to represent for qemu.
Just to be 100% clear, does this mean we can leave the diskSourceNetworkProtocolNFS element definition as is and simply enforce the port omission/tcp transport in the code? Or would you prefer that we re-write the schema to use a custom variety of the diskSourceNetworkHost which only supports tcp and has no port option?
On Mon, Jan 04, 2021 at 15:49:07 -0600, Ryan Gahagan wrote:
On Mon, Jan 4, 2021 at 8:24 AM Peter Krempa <pkrempa@redhat.com> wrote:
On Tue, Dec 29, 2020 at 15:21:25 -0600, Ryan Gahagan wrote:
+ <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"/>
This allows also 'port' and non TCP transports. It's okay to simplify the schema though and use the generic type. The code will need to reject those when we'll validate whether the disk is possible to represent for qemu.
Just to be 100% clear, does this mean we can leave the diskSourceNetworkProtocolNFS element definition as is and simply enforce the port omission/tcp transport in the code? Or would you prefer that we re-write the schema to use a custom variety of the diskSourceNetworkHost which only supports tcp and has no port option?
Yes, our schema doesn't cover all edge cases so we can use it here.
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/conf/domain_conf.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b301ac0a08..565ca680c9 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 nfsnode = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + ctxt->node = node; + + if ((nfsnode = virXPathNode("./nfs", ctxt))) { + src->nfs_user = virXMLPropString(nfsnode, "user"); + src->nfs_group = virXMLPropString(nfsnode, "group"); + } +} + + static int virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, xmlXPathContextPtr ctxt, @@ -8252,6 +8269,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); @@ -23805,6 +23825,19 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferAddLit(childBuf, "/>\n"); } + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + (src->nfs_user || src->nfs_group)) { + virBufferAddLit(childBuf, "<nfs"); + + if (src->nfs_user) + virBufferEscapeString(childBuf, " user='%s'", src->nfs_user); + if (src->nfs_group) + 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
On Tue, Dec 29, 2020 at 15:21:26 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/conf/domain_conf.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b301ac0a08..565ca680c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
This patch can be merged into the previous one if you want.
@@ -23805,6 +23825,19 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferAddLit(childBuf, "/>\n"); }
+ if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + (src->nfs_user || src->nfs_group)) { + virBufferAddLit(childBuf, "<nfs"); + + if (src->nfs_user)
virBufferEscapeString has a special corner-case that it doesn't format anything if the third argument is NULL, so the explicit check is not necessary.
+ virBufferEscapeString(childBuf, " user='%s'", src->nfs_user); + if (src->nfs_group) + virBufferEscapeString(childBuf, " group='%s'", src->nfs_group); + + virBufferAddLit(childBuf, "/>\n"); + }
With the code simplified: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/qemu/qemu_block.c | 79 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.c | 47 +++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b224a550f3..5413a4f0e8 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -574,6 +574,35 @@ 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; + + if (host->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only TCP protocol can be converted to NFSServer")); + return NULL; + } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:host", host->name, + "s:type", "inet", + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceBuildHostsJSONInetSocketAddress: * @src: disk storage source @@ -674,6 +703,44 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, } +static virJSONValuePtr +qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) server = NULL; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NFS protocol accepts only one host")); + return 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 +1248,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 +2572,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..8812df5138 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4626,6 +4626,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + /* NFS protocol may only be used if current QEMU supports blockdev */ + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + !blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' protocol is not supported with this QEMU binary")); + } + return 0; } @@ -9590,6 +9598,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 ((virStorageNetProtocol) 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 +10445,9 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, priv) < 0) return -1; + if (qemuDomainPrepareStorageSourceNFS(src) < 0) + return -1; + return 0; } -- 2.29.2
On qemu-5.2.0-4.fc34.x86_64, libvirt v6.10.0-262-gbed50bcbbb with this patch series, by following script: #!/bin/bash - set -o nounset # Treat unset variables as an error VM=test NFS_EXPORT=/tmp/nfs IMG=disk1 DISK_XML=/tmp/nfs.xml mkdir -p $NFS_EXPORT echo "$NFS_EXPORT *(rw,no_root_squash,insecure)" > /etc/exports systemctl restart nfs-server qemu-img create nfs://localhost"$NFS_EXPORT/$IMG" 100M chmod 777 $NFS_EXPORT -R virsh start $VM --console echo "<disk type='network' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source protocol='nfs' name='$NFS_EXPORT/$IMG'> <host name='localhost'/> <nfs user='+107' group='+107'/> </source> <target dev='vdb' bus='virtio'/> </disk>" > $DISK_XML virsh attach-device $VM $DISK_XML virsh console $VM virsh detach-device $VM $DISK_XML virsh console $VM virsh destroy $VM virsh attach-device $VM $DISK_XML --config virsh start $VM --console virsh destroy $VM virsh detach-device $VM $DISK_XML --config It works for me. All attach/detach tests have passed. On Wed, Dec 30, 2020 at 5:23 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/qemu/qemu_block.c | 79 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.c | 47 +++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b224a550f3..5413a4f0e8 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -574,6 +574,35 @@ 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; + + if (host->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only TCP protocol can be converted to NFSServer")); + return NULL; + } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:host", host->name, + "s:type", "inet", + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceBuildHostsJSONInetSocketAddress: * @src: disk storage source @@ -674,6 +703,44 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, }
+static virJSONValuePtr +qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) server = NULL; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NFS protocol accepts only one host")); + return 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 +1248,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 +2572,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..8812df5138 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4626,6 +4626,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; }
+ /* NFS protocol may only be used if current QEMU supports blockdev */ + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + !blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' protocol is not supported with this QEMU binary")); + } + return 0; }
@@ -9590,6 +9598,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 ((virStorageNetProtocol) 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 +10445,9 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, priv) < 0) return -1;
+ if (qemuDomainPrepareStorageSourceNFS(src) < 0) + return -1; + return 0; }
-- 2.29.2
-- Tested-by: Han Han <hhan@redhat.com>
On Tue, Dec 29, 2020 at 15:21:27 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- src/qemu/qemu_block.c | 79 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.c | 47 +++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b224a550f3..5413a4f0e8 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -574,6 +574,35 @@ 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; + + if (host->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("only TCP protocol can be converted to NFSServer")); + return NULL;
This check belongs into the validation function.
+ } + + ignore_value(virJSONValueObjectCreate(&ret, + "s:host", host->name, + "s:type", "inet", + NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceBuildHostsJSONInetSocketAddress: * @src: disk storage source @@ -674,6 +703,44 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src, }
+static virJSONValuePtr +qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src) +{ + g_autoptr(virJSONValue) server = NULL; + virJSONValuePtr ret = NULL; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NFS protocol accepts only one host")); + return NULL; + }
This check belongs to the validation function.
+ + 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) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d91c32b2c5..8812df5138 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4626,6 +4626,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; }
+ /* NFS protocol may only be used if current QEMU supports blockdev */ + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS && + !blockdev) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'nfs' protocol is not supported with this QEMU binary"));
Here you must also check that the 'port' is not provided and that the protocol is TCP. Otherwise you'd accept configurations which can't be represented in qemu silently.
+ } + return 0; }
@@ -9590,6 +9598,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 ((virStorageNetProtocol) src->protocol != VIR_STORAGE_NET_PROTOCOL_NFS)
The typecast to virStorageNetProtocol is required only when you want to use a 'switch' statement so that the complier checks that all cases are handled. It's not necessary in a single if-condition.
+ 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; + }
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")); + + src->nfs_uid = (uid_t) uidStore; + src->nfs_gid = (gid_t) gidStore; + + 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)); + + 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}, }; -- 2.29.2
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.
On Mon, Jan 4, 2021 at 8:41 AM Peter Krempa <pkrempa@redhat.com> wrote:
On Tue, Dec 29, 2020 at 15:21:28 -0600, Ryan Gahagan wrote:
+ 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.
We removed this code (specifically these assignments to nfs_uid and nfs_gid) and did all the other refactors requested for this method. Interestingly, the virstoragetest which tests this code still passes. However, we're unaware of any "hack" in our test code which actually fixes the missing uid and gid values. We're worried that this might indicate a problem with our test. Where should this storage when parsing backing store data actually be done? For reference, here is the test <https://gitlab.com/bschoney/libvirt/-/blob/issue-90-rt2/tests/virstoragetest.c#L1633> (on our branch after the changes) which is passing. Here <https://gitlab.com/bschoney/libvirt/-/blob/issue-90-rt2/src/util/virstoragefile.c#L3809> is the parse backing store method after the changes. Maybe we're just overreacting but we figured asking to make sure would be better than submitting a new patch only to find a problem.
On Mon, Jan 04, 2021 at 17:14:54 -0600, Ryan Gahagan wrote:
On Mon, Jan 4, 2021 at 8:41 AM Peter Krempa <pkrempa@redhat.com> wrote:
On Tue, Dec 29, 2020 at 15:21:28 -0600, Ryan Gahagan wrote:
+ 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.
We removed this code (specifically these assignments to nfs_uid and nfs_gid) and did all the other refactors requested for this method. Interestingly, the virstoragetest which tests this code still passes. However, we're unaware of any "hack" in our test code which actually fixes the missing uid and gid values. We're worried that this might indicate a problem with our test. Where should this storage when parsing backing store data actually be done?
Well. I thought the code was there to fix some test. If it's not fixing anything just don't assign those and you'll be in compliance with what I wanted.
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tests/qemuxml2argvdata/disk-network-nfs.args | 1 + .../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 + tests/virstoragetest.c | 13 +++++ 8 files changed, 228 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-nfs.args 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.args b/tests/qemuxml2argvdata/disk-network-nfs.args new file mode 100644 index 0000000000..fdc2941925 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-nfs.args @@ -0,0 +1 @@ +qemu_nfs 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..67d2843e01 --- /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' port='2049'/> + <nfs 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'/> + <nfs 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 96a2b95331..af00feab61 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..4728f5ee70 --- /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' port='2049'/> + <nfs 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' port='2049'/> + <nfs 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..9f03766810 --- /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' port='2049'/> + <nfs 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' port='2049'/> + <nfs 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 1968be6782..856a52b65c 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); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2e466ecb99..08e2723235 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1630,6 +1630,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\"," + "\"port\":\"2049\"" + "}" + "}" + "}", + "<source protocol='nfs' name='/foo/bar/baz'>\n" + " <host name='example.com' port='2049'/>\n" + " <nfs user='+2' group='+9'/>\n" + "</source>\n"); TEST_BACKING_PARSE_FULL("json:{ \"driver\": \"raw\"," "\"offset\": 10752," "\"size\": 4063232," -- 2.29.2
On Wed, Dec 30, 2020 at 5:23 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tests/qemuxml2argvdata/disk-network-nfs.args | 1 + .../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 + tests/virstoragetest.c | 13 +++++ 8 files changed, 228 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-nfs.args 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.args b/tests/qemuxml2argvdata/disk-network-nfs.args new file mode 100644 index 0000000000..fdc2941925 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-nfs.args @@ -0,0 +1 @@ +qemu_nfs 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..67d2843e01 --- /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' port='2049'/> + <nfs 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'/> + <nfs user='+1234' group='+5678'/>
I am curious why the uid/gid here is formatted as +NUMBER instead of the NUMBER itself...
+ </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 96a2b95331..af00feab61 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..4728f5ee70 --- /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' port='2049'/> + <nfs 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' port='2049'/> + <nfs 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..9f03766810 --- /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' port='2049'/> + <nfs 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' port='2049'/> + <nfs 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 1968be6782..856a52b65c 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); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2e466ecb99..08e2723235 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1630,6 +1630,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 \"," + "\"port\":\"2049\"" + "}" + "}" + "}", + "<source protocol='nfs' name='/foo/bar/baz'>\n" + " <host name='example.com' port='2049'/>\n" + " <nfs user='+2' group='+9'/>\n" + "</source>\n"); TEST_BACKING_PARSE_FULL("json:{ \"driver\": \"raw\"," "\"offset\": 10752," "\"size\": 4063232," -- 2.29.2
On Mon, Jan 04, 2021 at 15:53:08 +0800, Han Han wrote:
On Wed, Dec 30, 2020 at 5:23 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> ---
[...]
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'/> + <nfs user='+1234' group='+5678'/>
I am curious why the uid/gid here is formatted as +NUMBER instead of the NUMBER itself...
The '+' prefix means that the user is explicitly an UID (numeric) and must not be translated to an uid from a username (you can have numeric user name which maps to a different UID). In the tests we want to bypass the translation to preserve stable output.
On Tue, Dec 29, 2020 at 15:21:29 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tests/qemuxml2argvdata/disk-network-nfs.args | 1 + .../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 + tests/virstoragetest.c | 13 +++++ 8 files changed, 228 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-nfs.args 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.args b/tests/qemuxml2argvdata/disk-network-nfs.args new file mode 100644 index 0000000000..fdc2941925 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-nfs.args @@ -0,0 +1 @@ +qemu_nfs
This file is not used by the tests, drop it. [...]
diff --git a/tests/qemuxml2argvdata/disk-network-nfs.xml b/tests/qemuxml2argvdata/disk-network-nfs.xml new file mode 100644 index 0000000000..67d2843e01 --- /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' port='2049'/>
'port' can't be controlled so you shouldn't add it into the XML.
+ <nfs 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'/> + <nfs 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/virstoragetest.c b/tests/virstoragetest.c index 2e466ecb99..08e2723235 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1630,6 +1630,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\"," + "\"port\":\"2049\""
'port' contradicts the QMP schema, so this snippet is wrong. Also the transporrt type is missing. As noted in the patch adding the backing store parser code, this also belongs to that patch.
+ "}" + "}" + "}", + "<source protocol='nfs' name='/foo/bar/baz'>\n" + " <host name='example.com' port='2049'/>\n" + " <nfs user='+2' group='+9'/>\n" + "</source>\n"); TEST_BACKING_PARSE_FULL("json:{ \"driver\": \"raw\"," "\"offset\": 10752," "\"size\": 4063232," -- 2.29.2
On Wed, Dec 30, 2020 at 5:24 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Per Issue 90, Libvirt does not support attaching an NFS disk even though QEMU has added support for it. This series of patches seeks to implement this support in Libvirt and begins by adding in flags for an NFS disk.
It is hard to know what the issue 90 actual means. Please add a ref link for it. For example: Per issue 90[1], .... .... Reference: [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 b06a086e18..c58f39ebf1 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 fac93118fd..103dade0e7 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, @@ -3152,6 +3153,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); @@ -4627,6 +4629,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) case VIR_STORAGE_NET_PROTOCOL_VXHS: return 9999;
+ case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Per https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/htm... */ + return 2049; + 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 87763cf389..c5d5f0233a 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
Generally, for the patch series, it is better to be sent with a cover letter to describe the content and the background of the patch series. Since this is your second version of the nfs disk, please mention the patch version in the prefix of titles, like: [PATCH v2 1/7]... That could be done for all patches with the "-v2" option of git-send-email. BTW, mention the changes from the previous version in the cover letter. You can refer to https://www.kernel.org/doc/html/latest/process/submitting-patches.html#descr... On Wed, Dec 30, 2020 at 5:24 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Per Issue 90, Libvirt does not support attaching an NFS disk even though QEMU has added support for it. This series of patches seeks to implement this support in Libvirt and begins by adding in flags for an NFS disk.
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 b06a086e18..c58f39ebf1 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 fac93118fd..103dade0e7 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, @@ -3152,6 +3153,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); @@ -4627,6 +4629,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) case VIR_STORAGE_NET_PROTOCOL_VXHS: return 9999;
+ case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Per https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/htm... */ + return 2049; + 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 87763cf389..c5d5f0233a 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
On Tue, Dec 29, 2020 at 15:21:23 -0600, Ryan Gahagan wrote:
Per Issue 90, Libvirt does not support attaching an NFS disk even though QEMU has added support for it. This series of patches seeks to implement this support in Libvirt and begins by adding in flags for an NFS disk.
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/util/virstoragefile.c b/src/util/virstoragefile.c index fac93118fd..103dade0e7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c
[...]
@@ -4627,6 +4629,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) case VIR_STORAGE_NET_PROTOCOL_VXHS: return 9999;
+ case VIR_STORAGE_NET_PROTOCOL_NFS: + /* Per https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/htm... */ + return 2049;
I don't think we want to add a default port since the qemu code will not accept it at all.
participants (5)
-
Daniel P. Berrangé -
Han Han -
Peter Krempa -
Ryan Gahagan -
Ryan Gahagan