Re: [PATCH] libxl: add support for mounts in HVM guests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]