Re: [libvirt] [PATCH v2 libvirt 4/8] qemu: add support for MTP filesystem

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(a)redhat.com&gt; > --- > 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.