Michal Privoznik <mprivozn@xxxxxxxxxx> writes: > On 11.08.2014 16:47, Giuseppe Scrivano wrote: >> Generate the qemu command line option: >> >> -device 'usb-mtp,root=$SRC,desc=$TARGET' >> >> from the definition XML: >> >> <filesystem type='mount'> >> <source dir='$SRC'/> >> <target dir='$TARGET'/> >> <model type='mtp'/> >> </filesystem> >> >> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 >> >> Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx> >> --- >> src/qemu/qemu_command.c | 60 ++++++++++++++++++++++++++++--------------------- >> 1 file changed, 35 insertions(+), 25 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 87569b1..07f165e 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -2060,8 +2060,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, >> if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) >> continue; >> >> - /* Only support VirtIO-9p-pci so far. If that changes, >> - * we might need to skip devices here */ >> + if (def->fss[i]->model == VIR_DOMAIN_FS_MODEL_MTP) >> + continue; >> + >> if (virDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, >> flags) < 0) >> goto error; >> @@ -3956,12 +3957,6 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, >> const char *driver = qemuDomainFSDriverTypeToString(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")); >> @@ -4030,22 +4025,28 @@ qemuBuildFSDevStr(virDomainDefPtr def, >> { >> 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; >> - } >> - >> - virBufferAddLit(&opt, "virtio-9p-pci"); >> - virBufferAsprintf(&opt, ",id=%s", fs->info.alias); >> - virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); >> - virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); >> + if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { >> >> - if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) >> - goto error; >> + if (fs->model == VIR_DOMAIN_FS_MODEL_MTP) { > > I'd feel safer with: > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index ac8803a..ec269d6 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4027,10 +4027,13 @@ qemuBuildFSDevStr(virDomainDefPtr def, > > if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) { > > - if (fs->model == VIR_DOMAIN_FS_MODEL_MTP) { > + switch ((virDomainFSModel) fs->model) { > + case VIR_DOMAIN_FS_MODEL_MTP: > virBufferAddLit(&opt, "usb-mtp"); > virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); > - } else { > + break; > + case VIR_DOMAIN_FS_MODEL_DEFAULT: > + case VIR_DOMAIN_FS_MODEL_9P: > virBufferAddLit(&opt, "virtio-9p-pci"); > virBufferAsprintf(&opt, ",id=%s", fs->info.alias); > virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, > @@ -4038,6 +4041,11 @@ qemuBuildFSDevStr(virDomainDefPtr def, > virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); > if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) > goto error; > + break; > + > + case VIR_DOMAIN_FS_MODEL_LAST: > + /* nada */ > + break; > } > > if (virBufferCheckError(&opt) < 0) > >> + virBufferAddLit(&opt, "usb-mtp"); >> + virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst); >> + } else { >> + virBufferAddLit(&opt, "virtio-9p-pci"); >> + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); >> + virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, >> + fs->info.alias); >> + virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); >> + if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) >> + goto error; >> + } >> >> - if (virBufferCheckError(&opt) < 0) >> + if (virBufferCheckError(&opt) < 0) >> + goto error; >> + } else { >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("unsupported filesystem type")); >> goto error; >> + } >> >> return virBufferContentAndReset(&opt); >> >> @@ -8320,11 +8321,20 @@ qemuBuildCommandLine(virConnectPtr conn, >> char *optstr; >> virDomainFSDefPtr fs = def->fss[i]; >> >> - virCommandAddArg(cmd, "-fsdev"); >> - if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) >> + if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) { >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("only supports mount filesystem type")); > > After this check, fs->type can only be TYPE_MOUNT > >> goto error; >> - virCommandAddArg(cmd, optstr); >> - VIR_FREE(optstr); >> + } >> + >> + if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT && > > So this part of the condition is always true. > >> + fs->model != VIR_DOMAIN_FS_MODEL_MTP) { >> + virCommandAddArg(cmd, "-fsdev"); >> + if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) >> + goto error; >> + virCommandAddArg(cmd, optstr); >> + VIR_FREE(optstr); >> + } >> >> virCommandAddArg(cmd, "-device"); >> if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) >> > > Moreover, lets test this code. I'd feel more comfortable with a qemuxml2argv test case for this. I have amended your changes and added a new test. Going to send a v3 soon. Thanks, Giuseppe -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list