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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list