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

Re: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem



On 19.08.2014 22:11, 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                        | 65 ++++++++++++++++----------
  tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args |  6 +++
  tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml  | 30 ++++++++++++
  3 files changed, 77 insertions(+), 24 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b68695d..850fd71 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,36 @@ 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;
-    }
+    if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) {

-    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);
+        switch ((virDomainFSModel) fs->model) {
+        case VIR_DOMAIN_FS_MODEL_MTP:
+            virBufferAddLit(&opt, "usb-mtp");
+            virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src, fs->dst);

Reading qemu sources the root property is called 'x-root'. And indeed domain fails to start:

virsh # start gentoo
error: Failed to start domain gentoo
error: internal error: early end of file from monitor: possible problem:
2014-08-20T09:24:26.862418Z qemu-system-x86_64: -device usb-mtp,root=/some/path,desc=mtpfs: Property '.root' not found

Which makes me wonder more. So me digging through qemu git blame output I've found this qemu commit:

commit cf679caf911aa49a25477b3aa20468ee50ed6c89
Author:     Gerd Hoffmann <kraxel redhat com>
AuthorDate: Tue Jul 22 09:30:12 2014 +0200
Commit:     Gerd Hoffmann <kraxel redhat com>
CommitDate: Wed Jul 23 08:55:40 2014 +0200

    usb: mtp: tag root property as experimental

    Reason: we don't want commit to that interface yet.  Possibly
    the implementation will be switched over to use fsdev.

    Suggested-by: Paolo Bonzini <pbonzini redhat com>
    Signed-off-by: Gerd Hoffmann <kraxel redhat com>

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 1b51a90..384d4a5 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1090,7 +1090,7 @@ static const VMStateDescription vmstate_usb_mtp = {
 };

 static Property mtp_properties[] = {
-    DEFINE_PROP_STRING("root", MTPState, root),
+    DEFINE_PROP_STRING("x-root", MTPState, root),
     DEFINE_PROP_STRING("desc", MTPState, desc),
     DEFINE_PROP_END_OF_LIST(),
 };


Question that pops up immediately: do we want to commit to something that even qemu developers don't believe yet? What will happen when qemu decides to switch to 'root' attribute again? Libvirt will have to adapt which won't work with older qemus supporting 'x-root' only.

And what's this desc= attribute for anyway? I've put in several different values and it seems to me like nothing changed.

Maybe I don't know enough about MTP, but shouldn't the following be enough?

    <filesystem type='mount' accessmode='passthrough'>
      <source dir='/some/path'/>
      <model type='mtp'/>
    </filesystem>

QEMU (and subsequently guest) knows where to fetch files from.

+            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,
+                              fs->info.alias);
+            virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst);
+            if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
+                goto error;
+            break;

-    if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
-        goto error;
+        case VIR_DOMAIN_FS_MODEL_LAST:
+            /* nada */
+            break;
+        }

-    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 +8329,19 @@ 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"));
                  goto error;
-            virCommandAddArg(cmd, optstr);
-            VIR_FREE(optstr);
+            }
+
+            if (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)))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args b/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args
new file mode 100644
index 0000000..8a1dfeb
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
+/dev/HostVG/QEMUGuest1 -device virtio-mtp,root=/tmp/mtp,desc=mtpfs \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml b/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml
new file mode 100644
index 0000000..e811183
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <filesystem type='mount'>
+      <source dir='/tmp/mtp'/>
+      <target dir='mtpfs'/>
+      <model type='mtp'/>
+    </filesystem>
+  </devices>
+</domain>


Michal


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