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