[libvirt] [PATCH] libxl: add support for mounts in HVM guests
Xen HVM guests are running a QEMU, libxl allows passing QEMU parameters in the d_config.b_info.extra list. Take advantage of that to implement support for 9pfs mounts. For this to work, xen's qemu needs to be built with virtfs enabled, or an upstream qemu needs to be used. --- Note that two functions have been picked from the qemu driver's code. src/libxl/libxl_conf.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4eed5ca..4437e8f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -55,6 +55,7 @@ VIR_LOG_INIT("libxl.libxl_conf"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40 +#define LIBXL_FSDEV_HOST_PREFIX "fsdev-" struct guest_arch { virArch arch; @@ -84,6 +85,15 @@ static int libxlConfigOnceInit(void) VIR_ONCE_GLOBAL_INIT(libxlConfig) +VIR_ENUM_DECL(libxlDomainFSDriver) +VIR_ENUM_IMPL(libxlDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, + "local", + "local", + "handle", + NULL, + NULL, + NULL); + static void libxlDriverConfigDispose(void *obj) { @@ -1854,6 +1864,167 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; } +static char * +libxlBuildQemuFSStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *driver = libxlDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); + goto error; + } + + if (!driver) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Filesystem driver type not supported")); + goto error; + } + virBufferAdd(&opt, driver, -1); + + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always + * be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } + } + + if (fs->wrpolicy) + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + + virBufferAsprintf(&opt, ",id=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",path=%s", fs->src); + + if (fs->readonly) + virBufferAddLit(&opt, ",readonly"); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +} + + +static char * +libxlBuildQemuFSDevStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("can only passthrough directories")); + goto error; + } + + if (fs->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) + virBufferAddLit(&opt, "virtio-9p-ccw"); + else + virBufferAddLit(&opt, "virtio-9p-pci"); + + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); + virBufferAsprintf(&opt, ",fsdev=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +} + +static int +libxlMakeMountList(virDomainDefPtr def, + libxl_domain_config *d_config) +{ + int ret = -1; + size_t i; + size_t nextra = 0; + char *fsstr = NULL; + char *fsdevstr = NULL; + char *fsdevargstr = NULL; + char *deviceargstr = NULL; + + if (def->nfss > 0 && def->os.type != VIR_DOMAIN_OSTYPE_HVM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only HVM guests supports mounts")); + return ret; + } + + /* Make sure we have an allocated array, not ending with NULL */ + if (!d_config->b_info.extra) { + if (VIR_ALLOC(d_config->b_info.extra) < 0) + return ret; + nextra = 0; + } else { + nextra = virStringListLength(d_config->b_info.extra) + 1; + if (VIR_DELETE_ELEMENT(d_config->b_info.extra, nextra, nextra) < 0) + return ret; + } + + /* Add the QEMU options to d_config->b_info->extra + * -fsdev local,id=virtio9p,path=%s,security_model=none + * -device virtio-9p-pci,fsdev=virtio9p,mount_tag=%s + */ + for (i = 0; i < def->nfss; i++) { + virDomainFSDefPtr fs = def->fss[i]; + + /* Assign an alias */ + if (virAsprintf(&fs->info.alias, "fs%zu", i) < 0) + goto cleanup; + + if (!(fsstr = libxlBuildQemuFSStr(fs))) + goto cleanup; + if (!(fsdevstr = libxlBuildQemuFSDevStr(fs))) + goto cleanup; + + if (VIR_STRDUP(fsdevargstr, "-fsdev") < 0 || + VIR_STRDUP(deviceargstr, "-device") < 0) + goto cleanup; + + + /* Append to the extras before terminating NULL */ + if (VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, deviceargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevstr) < 0) + goto cleanup; + } + ret = 0; + + /* Add the terminating NULL item */ + if (VIR_RESIZE_N(d_config->b_info.extra, nextra, nextra, 1) < 0) + goto cleanup; + d_config->b_info.extra[nextra++] = NULL; + + cleanup: + VIR_FREE(fsstr); + VIR_FREE(fsdevstr); + VIR_FREE(fsdevargstr); + VIR_FREE(deviceargstr); + return ret; +} + int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, @@ -1883,6 +2054,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1; + if (libxlMakeMountList(def, d_config) < 0) + return -1; + /* * Now that any potential VFBs are defined, update the build info with * the data of the primary display. Some day libxl might implicitely do -- 2.1.4
On 12/01/2015 08:04 AM, Cédric Bosdonnat wrote:
Xen HVM guests are running a QEMU, libxl allows passing QEMU parameters in the d_config.b_info.extra list. Take advantage of that to implement support for 9pfs mounts. For this to work, xen's qemu needs to be built with virtfs enabled, or an upstream qemu needs to be used.
What if the qemu doesn't have virtfs enabled? Does it fail to start? Is a reasonable error returned to the user or available in /var/log/libvirt/libxl/libxl-driver.log or /var/log/xen/qemu-dm-<vmname>.log? The reason for a qemu start failure can usually be found in /var/log/xen/qemu-dm-<vmname>.log. If a reasonable error is reported there, I think we are fine.
--- Note that two functions have been picked from the qemu driver's code.
src/libxl/libxl_conf.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4eed5ca..4437e8f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -55,6 +55,7 @@ VIR_LOG_INIT("libxl.libxl_conf"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+#define LIBXL_FSDEV_HOST_PREFIX "fsdev-"
struct guest_arch { virArch arch; @@ -84,6 +85,15 @@ static int libxlConfigOnceInit(void)
VIR_ONCE_GLOBAL_INIT(libxlConfig)
+VIR_ENUM_DECL(libxlDomainFSDriver) +VIR_ENUM_IMPL(libxlDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, + "local", + "local", + "handle", + NULL, + NULL, + NULL); + static void libxlDriverConfigDispose(void *obj) { @@ -1854,6 +1864,167 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; }
+static char * +libxlBuildQemuFSStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *driver = libxlDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); + goto error; + } + + if (!driver) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Filesystem driver type not supported")); + goto error; + } + virBufferAdd(&opt, driver, -1); + + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always + * be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } + } + + if (fs->wrpolicy) + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + + virBufferAsprintf(&opt, ",id=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",path=%s", fs->src); + + if (fs->readonly) + virBufferAddLit(&opt, ",readonly"); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +} + + +static char * +libxlBuildQemuFSDevStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("can only passthrough directories")); + goto error; + } + + if (fs->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) + virBufferAddLit(&opt, "virtio-9p-ccw"); + else + virBufferAddLit(&opt, "virtio-9p-pci"); + + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); + virBufferAsprintf(&opt, ",fsdev=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +}
I suppose the copy and paste is fine, but would like to hear what others have to say. We could revisit the idea of a 'qemu command line builder' utility lib if we find ourselves copying more and more of the code.
+ +static int +libxlMakeMountList(virDomainDefPtr def, + libxl_domain_config *d_config) +{ + int ret = -1; + size_t i; + size_t nextra = 0; + char *fsstr = NULL; + char *fsdevstr = NULL; + char *fsdevargstr = NULL; + char *deviceargstr = NULL; + + if (def->nfss > 0 && def->os.type != VIR_DOMAIN_OSTYPE_HVM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only HVM guests supports mounts")); + return ret; + } + + /* Make sure we have an allocated array, not ending with NULL */ + if (!d_config->b_info.extra) { + if (VIR_ALLOC(d_config->b_info.extra) < 0) + return ret; + nextra = 0; + } else { + nextra = virStringListLength(d_config->b_info.extra) + 1; + if (VIR_DELETE_ELEMENT(d_config->b_info.extra, nextra, nextra) < 0) + return ret; + } + + /* Add the QEMU options to d_config->b_info->extra + * -fsdev local,id=virtio9p,path=%s,security_model=none + * -device virtio-9p-pci,fsdev=virtio9p,mount_tag=%s + */ + for (i = 0; i < def->nfss; i++) { + virDomainFSDefPtr fs = def->fss[i]; + + /* Assign an alias */ + if (virAsprintf(&fs->info.alias, "fs%zu", i) < 0) + goto cleanup; + + if (!(fsstr = libxlBuildQemuFSStr(fs))) + goto cleanup; + if (!(fsdevstr = libxlBuildQemuFSDevStr(fs))) + goto cleanup; + + if (VIR_STRDUP(fsdevargstr, "-fsdev") < 0 || + VIR_STRDUP(deviceargstr, "-device") < 0) + goto cleanup; + + + /* Append to the extras before terminating NULL */ + if (VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, deviceargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevstr) < 0) + goto cleanup; + } + ret = 0; + + /* Add the terminating NULL item */ + if (VIR_RESIZE_N(d_config->b_info.extra, nextra, nextra, 1) < 0) + goto cleanup; + d_config->b_info.extra[nextra++] = NULL; + + cleanup: + VIR_FREE(fsstr); + VIR_FREE(fsdevstr); + VIR_FREE(fsdevargstr); + VIR_FREE(deviceargstr); + return ret; +} + int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, @@ -1883,6 +2054,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1;
+ if (libxlMakeMountList(def, d_config) < 0) + return -1; +
Otherwise this looks good! Weak ACK based on answer to qemu not supporting virtfs, and another opinion on the copied code. Regards, Jim
On Tue, 2015-12-01 at 21:59 -0700, Jim Fehlig wrote:
On 12/01/2015 08:04 AM, Cédric Bosdonnat wrote:
Xen HVM guests are running a QEMU, libxl allows passing QEMU parameters in the d_config.b_info.extra list. Take advantage of that to implement support for 9pfs mounts. For this to work, xen's qemu needs to be built with virtfs enabled, or an upstream qemu needs to be used.
What if the qemu doesn't have virtfs enabled? Does it fail to start? Is a reasonable error returned to the user or available in /var/log/libvirt/libxl/libxl-driver.log or /var/log/xen/qemu-dm-<vmname>.log? The reason for a qemu start failure can usually be found in /var/log/xen/qemu-dm-<vmname>.log. If a reasonable error is reported there, I think we are fine.
The domain fails to start in that case, but the error isn't understandable (even a developer like me couldn't figure out what it was). I'll see if I can add a test for that to fail earlier with a meaningful error message.
--- Note that two functions have been picked from the qemu driver's code.
src/libxl/libxl_conf.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4eed5ca..4437e8f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -55,6 +55,7 @@ VIR_LOG_INIT("libxl.libxl_conf"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+#define LIBXL_FSDEV_HOST_PREFIX "fsdev-"
struct guest_arch { virArch arch; @@ -84,6 +85,15 @@ static int libxlConfigOnceInit(void)
VIR_ONCE_GLOBAL_INIT(libxlConfig)
+VIR_ENUM_DECL(libxlDomainFSDriver) +VIR_ENUM_IMPL(libxlDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, + "local", + "local", + "handle", + NULL, + NULL, + NULL); + static void libxlDriverConfigDispose(void *obj) { @@ -1854,6 +1864,167 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; }
+static char * +libxlBuildQemuFSStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *driver = libxlDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); + goto error; + } + + if (!driver) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Filesystem driver type not supported")); + goto error; + } + virBufferAdd(&opt, driver, -1); + + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always + * be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } + } + + if (fs->wrpolicy) + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + + virBufferAsprintf(&opt, ",id=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",path=%s", fs->src); + + if (fs->readonly) + virBufferAddLit(&opt, ",readonly"); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +} + + +static char * +libxlBuildQemuFSDevStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("can only passthrough directories")); + goto error; + } + + if (fs->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) + virBufferAddLit(&opt, "virtio-9p-ccw"); + else + virBufferAddLit(&opt, "virtio-9p-pci"); + + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); + virBufferAsprintf(&opt, ",fsdev=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +}
I suppose the copy and paste is fine, but would like to hear what others have to say. We could revisit the idea of a 'qemu command line builder' utility lib if we find ourselves copying more and more of the code.
I was hesitating doing that too... Daniel pushed me into that. However I removed the qemuCaps-related pieces from the qemu driver original code, not sure if we can factorize this. -- Cedric
+ +static int +libxlMakeMountList(virDomainDefPtr def, + libxl_domain_config *d_config) +{ + int ret = -1; + size_t i; + size_t nextra = 0; + char *fsstr = NULL; + char *fsdevstr = NULL; + char *fsdevargstr = NULL; + char *deviceargstr = NULL; + + if (def->nfss > 0 && def->os.type != VIR_DOMAIN_OSTYPE_HVM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only HVM guests supports mounts")); + return ret; + } + + /* Make sure we have an allocated array, not ending with NULL */ + if (!d_config->b_info.extra) { + if (VIR_ALLOC(d_config->b_info.extra) < 0) + return ret; + nextra = 0; + } else { + nextra = virStringListLength(d_config->b_info.extra) + 1; + if (VIR_DELETE_ELEMENT(d_config->b_info.extra, nextra, nextra) < 0) + return ret; + } + + /* Add the QEMU options to d_config->b_info->extra + * -fsdev local,id=virtio9p,path=%s,security_model=none + * -device virtio-9p-pci,fsdev=virtio9p,mount_tag=%s + */ + for (i = 0; i < def->nfss; i++) { + virDomainFSDefPtr fs = def->fss[i]; + + /* Assign an alias */ + if (virAsprintf(&fs->info.alias, "fs%zu", i) < 0) + goto cleanup; + + if (!(fsstr = libxlBuildQemuFSStr(fs))) + goto cleanup; + if (!(fsdevstr = libxlBuildQemuFSDevStr(fs))) + goto cleanup; + + if (VIR_STRDUP(fsdevargstr, "-fsdev") < 0 || + VIR_STRDUP(deviceargstr, "-device") < 0) + goto cleanup; + + + /* Append to the extras before terminating NULL */ + if (VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, deviceargstr) < 0 || + VIR_APPEND_ELEMENT(d_config->b_info.extra, nextra, fsdevstr) < 0) + goto cleanup; + } + ret = 0; + + /* Add the terminating NULL item */ + if (VIR_RESIZE_N(d_config->b_info.extra, nextra, nextra, 1) < 0) + goto cleanup; + d_config->b_info.extra[nextra++] = NULL; + + cleanup: + VIR_FREE(fsstr); + VIR_FREE(fsdevstr); + VIR_FREE(fsdevargstr); + VIR_FREE(deviceargstr); + return ret; +} + int libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, virDomainDefPtr def, @@ -1883,6 +2054,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1;
+ if (libxlMakeMountList(def, d_config) < 0) + return -1; +
Otherwise this looks good! Weak ACK based on answer to qemu not supporting virtfs, and another opinion on the copied code.
Regards, Jim
On 02.12.2015 09:15, Cedric Bosdonnat wrote:
On Tue, 2015-12-01 at 21:59 -0700, Jim Fehlig wrote:
On 12/01/2015 08:04 AM, Cédric Bosdonnat wrote:
Xen HVM guests are running a QEMU, libxl allows passing QEMU parameters in the d_config.b_info.extra list. Take advantage of that to implement support for 9pfs mounts. For this to work, xen's qemu needs to be built with virtfs enabled, or an upstream qemu needs to be used.
What if the qemu doesn't have virtfs enabled? Does it fail to start? Is a reasonable error returned to the user or available in /var/log/libvirt/libxl/libxl-driver.log or /var/log/xen/qemu-dm-<vmname>.log? The reason for a qemu start failure can usually be found in /var/log/xen/qemu-dm-<vmname>.log. If a reasonable error is reported there, I think we are fine.
The domain fails to start in that case, but the error isn't understandable (even a developer like me couldn't figure out what it was).
I'll see if I can add a test for that to fail earlier with a meaningful error message.
Please do.
--- Note that two functions have been picked from the qemu driver's code.
src/libxl/libxl_conf.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4eed5ca..4437e8f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -55,6 +55,7 @@ VIR_LOG_INIT("libxl.libxl_conf"); /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+#define LIBXL_FSDEV_HOST_PREFIX "fsdev-"
struct guest_arch { virArch arch; @@ -84,6 +85,15 @@ static int libxlConfigOnceInit(void)
VIR_ONCE_GLOBAL_INIT(libxlConfig)
+VIR_ENUM_DECL(libxlDomainFSDriver) +VIR_ENUM_IMPL(libxlDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, + "local", + "local", + "handle", + NULL, + NULL, + NULL); + static void libxlDriverConfigDispose(void *obj) { @@ -1854,6 +1864,167 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; }
+static char * +libxlBuildQemuFSStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *driver = libxlDomainFSDriverTypeToString(fs->fsdriver); + const char *wrpolicy = virDomainFSWrpolicyTypeToString(fs->wrpolicy); + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports mount filesystem type")); + goto error; + } + + if (!driver) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Filesystem driver type not supported")); + goto error; + } + virBufferAdd(&opt, driver, -1); + + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) { + if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) { + virBufferAddLit(&opt, ",security_model=mapped"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virBufferAddLit(&opt, ",security_model=passthrough"); + } else if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) { + virBufferAddLit(&opt, ",security_model=none"); + } + } else { + /* For other fs drivers, default(passthru) should always + * be supported */ + if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only supports passthrough accessmode")); + goto error; + } + } + + if (fs->wrpolicy) + virBufferAsprintf(&opt, ",writeout=%s", wrpolicy); + + virBufferAsprintf(&opt, ",id=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",path=%s", fs->src); + + if (fs->readonly) + virBufferAddLit(&opt, ",readonly"); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +} + + +static char * +libxlBuildQemuFSDevStr(virDomainFSDefPtr fs) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("can only passthrough directories")); + goto error; + } + + if (fs->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) + virBufferAddLit(&opt, "virtio-9p-ccw"); + else + virBufferAddLit(&opt, "virtio-9p-pci"); + + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); + virBufferAsprintf(&opt, ",fsdev=%s%s", LIBXL_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + + if (virBufferCheckError(&opt) < 0) + goto error; + + return virBufferContentAndReset(&opt); + + error: + virBufferFreeAndReset(&opt); + return NULL; +}
I suppose the copy and paste is fine, but would like to hear what others have to say. We could revisit the idea of a 'qemu command line builder' utility lib if we find ourselves copying more and more of the code.
I was hesitating doing that too... Daniel pushed me into that. However I removed the qemuCaps-related pieces from the qemu driver original code, not sure if we can factorize this.
Yeah, I don't think we will need much of qemu command line code elsewhere, so right now turning it into a module seems like an overkill to me. ACK. Michal
participants (4)
-
Cedric Bosdonnat -
Cédric Bosdonnat
-
Jim Fehlig -
Michal Privoznik