[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]