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

19 Aug 2014

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

    

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

Michal Privoznik