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

[Giuseppe Scrivano]

Michal Privoznik <mprivozn at redhat.com> 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 at redhat.com>
>> ---
>>   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