4 a.m.
This is a request for comments on our patch for Issue 90. We didn't add
in the test yet because we've had a ton of trouble getting the testing
suite to work with our new test files, so they've omitted for now. We
also did not add any logic to the attach-disk method yet because we
wanted to focus on this set of changes first.
4 a.m.
New subject: [RFC PATCH 1/6] compatibilities: added in flags for NFS support
Signed-off-by: Ryan Gahagan <rgahagan(a)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 766f76020c..b5a4a22ed3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9629,6 +9629,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..5a57e5d12d 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:
+ /* return based on exmaple and SUSE support docs */
+ 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.0
12:08 p.m.
New subject: [RFC PATCH 1/6] compatibilities: added in flags for NFS support
On Fri, Dec 11, 2020 at 4:01 AM Ryan Gahagan <rgahagan(a)cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
The patch for compatibilities usually is on src/qemu/qemu_capabilities* and
tests/qemucapabilitiesdata/caps_*.
Because nfs disk is supported since qemu v2.9(
https://github.com/qemu/qemu/blob/master/qapi/block-core.json#L3759), the
compatibilities flags should be added to check whether the target qemu
supports nfs disk.
https://gitlab.com/libvirt/libvirt/-/commit/9825f71b53014bd2b418680ffeb2b...
is a example:
The diff in src/qemu/qemu_capabilities.* defines the name & flag of this
capability flag, and the method to query the capability. For
more information of the capability query, you can refer to the code
comments of virQEMUQAPISchemaPathGet in libvirt and the
query-qmp-schema in qemu
https://www.qemu.org/docs/master/interop/qemu-qmp-ref.html
The diff in tests/qemucapabilitiesdata/caps_*.xml defines the capability
unit tests for different arhes and qemu versions.
For this patch, the diff in src/util/* and src/libxl/* could be included in
the "util: ..." patch. The diff of src/qemu/* is better to be included
in the "qemu: ..." patch.
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 766f76020c..b5a4a22ed3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9629,6 +9629,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..5a57e5d12d 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:
+ /* return based on exmaple and SUSE support docs */
+ 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.0
5:34 p.m.
New subject: [RFC PATCH 1/6] compatibilities: added in flags for NFS support
On Thu, Dec 10, 2020 at 14:00:02 -0600, Ryan Gahagan wrote:
In subject/summary. We don't have anything which we'd prefix with
'compatibilities:'.
A better summary will be:
conf: Add NFS disk protocol
Signed-off-by: Ryan Gahagan <rgahagan(a)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(+)
6:54 a.m.
New subject: [RFC PATCH 1/6] compatibilities: added in flags for NFS support
On Fri, Dec 11, 2020 at 3:35 AM Peter Krempa <pkrempa(a)redhat.com> wrote:
In subject/summary. We don't have anything which we'd prefix
with
'compatibilities:'.
Just to confirm, does this mean that we should not implement the feedback
Han Han suggested about the NFS capability flags and instead leave the
commit as-is (except for the summary)? We didn't provide an NFS CAPS flag
because in a previous email you had suggested:
"- there's no need to add a specific capability for the NFS protocol as
it predates libvirt's use of -blockdev (QEMU_CAPS_BLOCKDEV). You have to
add a check for it to qemuDomainValidateStorageSource based on the above
capabapility."
We will provide this check in qemuDomainValidateStorageSource, but do we
need to worry about CAPS flags?
2:02 a.m.
New subject: [RFC PATCH 1/6] compatibilities: added in flags for NFS support
(answering for Peter since he is on "vacation", so it may be awhile
before he gets around to this)
On 12/14/20 5:54 PM, Ryan Gahagan wrote:
On Fri, Dec 11, 2020 at 3:35 AM Peter Krempa <pkrempa(a)redhat.com
<mailto:pkrempa@redhat.com>> wrote:
In subject/summary. We don't have anything which we'd prefix with
'compatibilities:'.
Just to confirm, does this mean that we should not implement the
feedback Han Han suggested about the NFS capability flags and instead
leave the commit as-is (except for the summary)?
Yes. What Han Han was discussing is the QEMU capabilities flags, which
indicate which of a large set of features are supported by a particular
QEMU binary. Since, as you say below, any QEMU binary that is capable of
using blockdev is by definition capable of using NFS, there is no need
to add an extra flag for NFS.
We didn't provide an NFS CAPS flag because in a previous email
you had
suggested:
"- there's no need to add a specific capability for the NFS protocol as
it predates libvirt's use of -blockdev (QEMU_CAPS_BLOCKDEV). You have
to add a check for it to qemuDomainValidateStorageSource based on the
above capabapility."
We will provide this check in qemuDomainValidateStorageSource, but do
we need to worry about CAPS flags?
4 a.m.
New subject: [RFC PATCH 2/6] util: Added nfs params to virStorageSource
Signed-off-by: Ryan Gahagan <rgahagan(a)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 5a57e5d12d..cff6dabd9e 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.0
12:19 p.m.
New subject: [RFC PATCH 2/6] util: Added nfs params to virStorageSource
On Fri, Dec 11, 2020 at 4:01 AM Ryan Gahagan <rgahagan(a)cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan(a)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 5a57e5d12d..cff6dabd9e 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;
Only one pair of (nfs_user,nfs_group) or (nfs_uid,nfs_gid) is enough to
identify the nfs connection. Please remove
a duplicated pair.
};
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref);
--
2.29.0
5:15 p.m.
New subject: [RFC PATCH 2/6] util: Added nfs params to virStorageSource
On Fri, Dec 11, 2020 at 12:19:10 +0800, Han Han wrote:
On Fri, Dec 11, 2020 at 4:01 AM Ryan Gahagan
<rgahagan(a)cs.utexas.edu> wrote:
> Signed-off-by: Ryan Gahagan <rgahagan(a)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 5a57e5d12d..cff6dabd9e 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;
>
Only one pair of (nfs_user,nfs_group) or (nfs_uid,nfs_gid) is enough to
identify the nfs connection. Please remove
a duplicated pair.
Not really. The second pair uit_t/gid_t is necessary for storing the
looked up uid/gid values. The qemu NFS driver accepts just numerics and
we must not try to look up uids/gids in the JSON prop generator.
4 a.m.
New subject: [RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
docs/formatdomain.rst | 1 +
docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 512939679b..40a1a3c1e2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2601,6 +2601,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
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 795b654feb..6b321b1ca3 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.0
12:36 p.m.
New subject: [RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS
On Fri, Dec 11, 2020 at 4:00 AM Ryan Gahagan <rgahagan(a)cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
docs/formatdomain.rst | 1 +
docs/schemas/domaincommon.rng | 38 +++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 512939679b..40a1a3c1e2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2601,6 +2601,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 2049
Mention the feature introduced version here.
========
=======================================================
============================================================
================
And provide the nfs disk examples in the chapter
hard-drives-floppy-disks-cdroms of formatdomain
gluster supports "tcp", "rdma", "unix" as valid
values for the
transport
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 795b654feb..6b321b1ca3 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.0
6:57 a.m.
New subject: [RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS
On Thu, Dec 10, 2020 at 10:37 PM Han Han <hhan(a)redhat.com> wrote:
On Fri, Dec 11, 2020 at 4:00 AM Ryan Gahagan
<rgahagan(a)cs.utexas.edu>
wrote:
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 512939679b..40a1a3c1e2 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2601,6 +2601,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 2049
>
Mention the feature introduced version here.
What version would we specify? QEMU's version 2.9.0 patch where they
introduce NFS support? The Libvirt current version 6.10.0 patch? A future
patch number (e.g. Libvirt version 6.11.0 or similar)? We were unclear on
what exact number goes here, but we were planning on putting it into the
space parallel to the :since: in the gluster line.
12:54 a.m.
New subject: [RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS
On 12/14/20 5:57 PM, Ryan Gahagan wrote:
On Thu, Dec 10, 2020 at 10:37 PM Han Han <hhan(a)redhat.com
<mailto:hhan@redhat.com>> wrote:
On Fri, Dec 11, 2020 at 4:00 AM Ryan Gahagan
<rgahagan(a)cs.utexas.edu <mailto:rgahagan@cs.utexas.edu>> wrote:
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 512939679b..40a1a3c1e2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2601,6 +2601,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 2049
Mention the feature introduced version here.
What version would we specify? QEMU's version 2.9.0 patch where they
introduce NFS support? The Libvirt current version 6.10.0 patch? A
future patch number (e.g. Libvirt version 6.11.0 or similar)? We were
unclear on what exact number goes here, but we were planning on
putting it into the space parallel to the :since: in the gluster line.
You would put the version that libvirt will be at when the feature is in
released code, so if it was pushed today, that would be 7.0.0.
(Sometimes we also indicate a minimum qemu or kernel version that's
required, which is more useful if a feature is just newly supported in
qemu/kernel but over time it just becomes extra numbers to confuse
people. In this case, my opinion would be that the qemu version is old
enough that it's not necessary to mention it.)
4 a.m.
New subject: [RFC PATCH 4/6] conf: Added NFS XML format/parse methods
Signed-off-by: Ryan Gahagan <rgahagan(a)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 23415b323c..265c7584a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7844,6 +7844,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,
@@ -9208,6 +9225,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);
@@ -24761,6 +24781,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.0
4 a.m.
New subject: [RFC PATCH 5/6] qemu: added NFS JSON props methods
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
src/qemu/qemu_block.c | 42 +++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b224a550f3..f93f675262 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -674,6 +674,36 @@ 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 =
qemuBlockStorageSourceBuildJSONInetSocketAddress(&src->hosts[0])))
+ return NULL;
+
+ /* NFS disk specification example:
+ * { driver:"nfs",
+ * user: "0",
+ * group: "0",
+ * server: {type:"tcp", host:"1.2.3.4", port:9999}}
+ */
+ ignore_value(virJSONValueObjectCreate(&ret,
+ "i:user", src->nfs_uid,
+ "i:group", src->nfs_gid,
+ "a:server", &server, NULL));
+
+ return ret;
+}
+
+
static virJSONValuePtr
qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src,
bool onlytarget)
@@ -1181,6 +1211,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 +2535,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 b5a4a22ed3..64ebfb5812 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9589,6 +9589,45 @@ 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)
+ {
+ int err = 0;
+ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK)
+ return 0;
+
+ if ((virStorageNetProtocol) src->protocol != VIR_STORAGE_NET_PROTOCOL_NFS)
+ return 0;
+
+ if (src->nfs_user) {
+ err = virGetUserID(src->nfs_user, &src->nfs_uid);
+ } else {
+ /* TODO: Provide hypervisor default value */
+ }
+
+ if (err < 0)
+ return -1;
+
+ if (src->nfs_group) {
+ err = virGetGroupID(src->nfs_group, &src->nfs_gid);
+ } else {
+ /* TODO: Provide hypervisor default value */
+ }
+
+ if (err < 0)
+ return -1;
+
+ return 0;
+}
+
+
/* qemuProcessPrepareStorageSourceTLS:
* @source: source for a disk
* @cfg: driver configuration
@@ -10400,6 +10439,9 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk,
priv) < 0)
return -1;
+ if (qemuDomainPrepareStorageSourceNFS(src) < 0)
+ return -1;
+
return 0;
}
--
2.29.0
5:30 p.m.
New subject: [RFC PATCH 5/6] qemu: added NFS JSON props methods
On Thu, Dec 10, 2020 at 14:00:06 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
src/qemu/qemu_block.c | 42 +++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b224a550f3..f93f675262 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -674,6 +674,36 @@ 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 =
qemuBlockStorageSourceBuildJSONInetSocketAddress(&src->hosts[0])))
+ return NULL;
+
+ /* NFS disk specification example:
+ * { driver:"nfs",
+ * user: "0",
+ * group: "0",
+ * server: {type:"tcp", host:"1.2.3.4", port:9999}}
+ */
+ ignore_value(virJSONValueObjectCreate(&ret,
+ "i:user", src->nfs_uid,
+ "i:group", src->nfs_gid,
+ "a:server", &server, NULL));
There's also virJSONValueObjectAdd, that might come in handy given my
comment below.
+
+ return ret;
+}
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b5a4a22ed3..64ebfb5812 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9589,6 +9589,45 @@ 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)
+ {
+ int err = 0;
+ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK)
+ return 0;
+
+ if ((virStorageNetProtocol) src->protocol != VIR_STORAGE_NET_PROTOCOL_NFS)
+ return 0;
+
+ if (src->nfs_user) {
+ err = virGetUserID(src->nfs_user, &src->nfs_uid);
You can return here directly if virGetUserID returns -1. No need for the
extra vriable.
+ } else {
+ /* TODO: Provide hypervisor default value */
The best bet is to actually avoid formatting the user/group members
formatting towards qemu.
Unfortunately 0 is a very valid uid, so you'll probably need to use -1
to signal that it's the default.
+ if (err < 0)
+ return -1;
7 a.m.
New subject: [RFC PATCH 5/6] qemu: added NFS JSON props methods
On Fri, Dec 11, 2020 at 3:30 AM Peter Krempa <pkrempa(a)redhat.com> wrote:
There's also virJSONValueObjectAdd, that might come in handy
given my
comment below.
[...]
The best bet is to actually avoid formatting the user/group members
formatting towards qemu.
Unfortunately 0 is a very valid uid, so you'll probably need to use -1
to signal that it's the default.
We wanted to make sure we understood what you meant here. We've changed our
code to store -1 into the nfs_uid and gid whenever the default is assumed,
and only called virJSONValueObjectAdd to put the group and user integer
properties into the JSON object if the uid and gid are not -1 (i.e. not
defaults). This would leave them as "undefined" values in the JSON object.
Is this what QEMU accepts when the default is needed, or have we
misunderstood?
4 a.m.
New subject: [RFC PATCH 6/6] util: Added a backing store NFS parser
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
src/util/virstoragefile.c | 42 +++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cff6dabd9e..098ec80542 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3805,6 +3805,47 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
}
+static int
+virStorageSourceParseBackingJSONNFS(virStorageSourcePtr src,
+ virJSONValuePtr json,
+ const char *jsonstr G_GNUC_UNUSED,
+ int opaque G_GNUC_UNUSED)
+{
+ const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id");
+ virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
+ int gotUID = virJSONValueObjectGetNumberInt(json, "user", (int
*)(&src->nfs_uid));
+ int gotGID = virJSONValueObjectGetNumberInt(json, "group", (int
*)(&src->nfs_gid));
+
+ if (!vdisk_id || !server) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("missing 'vdisk-id' or '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->type = VIR_STORAGE_TYPE_NETWORK;
+ src->protocol = VIR_STORAGE_NET_PROTOCOL_NFS;
+
+ src->path = g_strdup(vdisk_id);
+
+ 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 +3905,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.0
5:23 p.m.
New subject: [RFC PATCH 6/6] util: Added a backing store NFS parser
On Thu, Dec 10, 2020 at 14:00:07 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan(a)cs.utexas.edu>
---
src/util/virstoragefile.c | 42 +++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cff6dabd9e..098ec80542 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3805,6 +3805,47 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
}
+static int
+virStorageSourceParseBackingJSONNFS(virStorageSourcePtr src,
+ virJSONValuePtr json,
+ const char *jsonstr G_GNUC_UNUSED,
+ int opaque G_GNUC_UNUSED)
+{
+ const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id");
I'm fairly sure that this is a 'vxhs' protocol property.
You'd probably notice it if you'd add tests in the same commit.
+ virJSONValuePtr server = virJSONValueObjectGetObject(json,
"server");
+ int gotUID = virJSONValueObjectGetNumberInt(json, "user", (int
*)(&src->nfs_uid));
You should not typecast the pointers here since it's not guaranteed that
the uid_t/gid_t type is the same as an integer. Additionally storing
this only in the nfs_uid field will actually not show up in the VM xml
once parsed. You actually need to populate the string variants with the
uid number with + prepended so that the XML conversion works.
+ int gotGID = virJSONValueObjectGetNumberInt(json,
"group", (int *)(&src->nfs_gid));
+
+ if (!vdisk_id || !server) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("missing 'vdisk-id' or 'server' attribute
in "
+ "JSON backing definition for NFS volume"));
Our coding style mandates that error messages in new code should be on a
single line.
+ 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->type = VIR_STORAGE_TYPE_NETWORK;
+ src->protocol = VIR_STORAGE_NET_PROTOCOL_NFS;
+
+ src->path = g_strdup(vdisk_id);
+
+ 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,
7:04 a.m.
New subject: [RFC PATCH 6/6] util: Added a backing store NFS parser
On Fri, Dec 11, 2020 at 3:24 AM Peter Krempa <pkrempa(a)redhat.com> wrote:
On Thu, Dec 10, 2020 at 14:00:07 -0600, Ryan Gahagan wrote:
> + virJSONValuePtr server = virJSONValueObjectGetObject(json,
"server");
> + int gotUID = virJSONValueObjectGetNumberInt(json, "user", (int
*)(&src->nfs_uid));
You should not typecast the pointers here since it's not guaranteed that
the uid_t/gid_t type is the same as an integer. Additionally storing
this only in the nfs_uid field will actually not show up in the VM xml
once parsed. You actually need to populate the string variants with the
uid number with + prepended so that the XML conversion works.
The reason we use this hacky integer pointer cast is because the
virJSONValueObjectGetNumberInt method expects an int * as its thir
parameter, and when we tried to use &src->nfs_uid or gid directly we got a
compile error for type mismatch. This cast was the only way we could find
to work around this easily other than changing the _virStorageSource
parameters to be explicit int type, but then the virGetUserID and GroupID
methods cause the opposite type mismatch error because they return uid_t
and gid_t values. How should we actually get these numbers out if not for
this cast?
12:14 p.m.
On Fri, Dec 11, 2020 at 4:00 AM Ryan Gahagan <rgahagan(a)cs.utexas.edu> wrote:
This is a request for comments on our patch for Issue 90. We
didn't add
in the test yet because we've had a ton of trouble getting the testing
suite to work with our new test files, so they've omitted for now. We
also did not add any logic to the attach-disk method yet because we
wanted to focus on this set of changes first.
A tiny suggestion: You can use git-send-email with --cover-letter option
to send a
patch series
with a cover letter instead of an independent cover letter email.
6:25 p.m.
On Thu, Dec 10, 2020 at 14:00:01 -0600, Ryan Gahagan wrote:
This is a request for comments on our patch for Issue 90. We
didn't add
in the test yet because we've had a ton of trouble getting the testing
suite to work with our new test files, so they've omitted for now. We
also did not add any logic to the attach-disk method yet because we
attach-disk is covered by what you have. The 'prepare' step is executed
already.
wanted to focus on this set of changes first.
What you have looks mostly okay. Obviously you must add the tests for
the final submission.
You also need to add a check to 'qemuDomainValidateStorageSource' that
the NFS protocol is used only when the current qemu supports
QEMU_CAPS_BLOCKDEV since only the blockdev portion is implemented.
I'm not sure what your issues with tests are since you didn't specify
them closer, but note that tests must not rely on host state. That means
that the translation of uids must not be done in tests.
1403
days inactive
1408
days old
21 comments
5 participants
participants (5)
-
Han Han -
Laine Stump -
Peter Krempa -
Ryan Gahagan -
Ryan Gahagan