Re: [libvirt PATCHv3 08/12] qemu: validate virtiofs filesystems
On Thu, Jan 30, 2020 at 06:06:24PM +0100, Ján Tomko wrote:
Reject unsupported configurations.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6367f5394e..b5d5812ff8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8314,8 +8314,32 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
return -1;
case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
- /* TODO: vhost-user-fs-pci */
- return 0;
+ if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtiofs only supports passthrough
accessmode"));
+ return -1;
+ }
+ if (fs->wrpolicy != VIR_DOMAIN_FS_WRPOLICY_DEFAULT) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtiofs does not support wrpolicy"));
+ return -1;
+ }
+ if (fs->model != VIR_DOMAIN_FS_MODEL_DEFAULT) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtiofs does not support model"));
+ return -1;
+ }
+ if (fs->format != VIR_STORAGE_FILE_NONE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtiofs does not support format"));
+ return -1;
+ }
+ if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtiofs requires shared memory"));
+ return -1;
+ }
This requires the following <memoryBacking> config, right?
<memoryBacking>
<access mode='shared'/>
</memoryBacking>
I think the <memoryBacking> isn't mandatory because the memory access policy
can be over written by <numa> config like as:
<numa>
<cell id='0' cpus='0-31' memory='8912896'
unit='KiB' memAccess='shared'/>
</numa>
And, I suppose virtiofsd requires one or more numa nodes.
How about adding following function to check the mem
config instead of the if statement?
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 493864b854..b3b5cb2054 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8203,6 +8203,39 @@ qemuDomainDeviceDefValidateIOMMU(const virDomainIOMMUDef *iommu,
return 0;
}
+static int
+memconfIsSufficientForVirtiofs(const virDomainDef *def)
+{
+ virDomainNumaPtr numa = def->numa;
+ virDomainMemoryAccess memAccess = def->mem.access;
+ virDomainMemoryAccess NodememAccess;
+ size_t nodes;
+ size_t i;
+
+ nodes = virDomainNumaGetNodeCount(numa);
+ if (!nodes) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("virtiofs requires one or more numa nodes"));
+ return 0;
+ }
+
+ for (i = 0; i < nodes; i++) {
+ NodememAccess = virDomainNumaGetNodeMemoryAccessMode(numa, i);
+ if (NodememAccess == VIR_DOMAIN_MEMORY_ACCESS_PRIVATE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("virtiofs requires shared memory for node[%ld]"),
i);
+ return 0;
+ } else if ((NodememAccess == VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) &&
+ (memAccess != VIR_DOMAIN_MEMORY_ACCESS_SHARED)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("virtiofs requires shared memory for node[%ld]"),
i);
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
static int
qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
@@ -8256,9 +8289,7 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
_("virtiofs does not support format"));
return -1;
}
- if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtiofs requires shared memory"));
+ if (!memconfIsSufficientForVirtiofs(def)) {
return -1;
}
break;
---
Thanks,
Masa