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