9:10 a.m.
This series adds support for qemu usb-mtp devices.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781
Giuseppe Scrivano (6):
conf: add MTP filesystem support to the parser
conf, virDomainFSDefPtr: rename "path" argument to "target"
qemu_command: fix block indentation
qemu: add support for MTP filesystem
docs: add documentation and schema for MTP filesystem type
tests: add tests for MTP filesystem
docs/formatdomain.html.in | 9 +++++
docs/schemas/domaincommon.rng | 15 ++++++--
src/conf/domain_conf.c | 38 +++++++++++++------
src/conf/domain_conf.h | 3 +-
src/qemu/qemu_command.c | 67 +++++++++++++++++++---------------
tests/domainconfdata/getfilesystem.xml | 4 ++
tests/domainconftest.c | 2 +
7 files changed, 93 insertions(+), 45 deletions(-)
--
1.9.3
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
src/conf/domain_conf.c | 34 +++++++++++++++++++++++++---------
src/conf/domain_conf.h | 1 +
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c25c74b..3bdf46a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"file",
"template",
"ram",
- "bind")
+ "bind",
+ "mtp")
VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"default",
@@ -6404,7 +6405,8 @@ virDomainFSDefParseXML(xmlNodePtr node,
xmlStrEqual(cur->name, BAD_CAST "source")) {
if (def->type == VIR_DOMAIN_FS_TYPE_MOUNT ||
- def->type == VIR_DOMAIN_FS_TYPE_BIND)
+ def->type == VIR_DOMAIN_FS_TYPE_BIND ||
+ def->type == VIR_DOMAIN_FS_TYPE_MTP)
source = virXMLPropString(cur, "dir");
else if (def->type == VIR_DOMAIN_FS_TYPE_FILE)
source = virXMLPropString(cur, "file");
@@ -6418,7 +6420,10 @@ virDomainFSDefParseXML(xmlNodePtr node,
}
} else if (!target &&
xmlStrEqual(cur->name, BAD_CAST "target")) {
- target = virXMLPropString(cur, "dir");
+ if (def->type == VIR_DOMAIN_FS_TYPE_MTP)
+ target = virXMLPropString(cur, "name");
+ else
+ target = virXMLPropString(cur, "dir");
} else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
def->readonly = true;
} else if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
@@ -15668,9 +15673,11 @@ virDomainFSDefFormat(virBufferPtr buf,
}
- virBufferAsprintf(buf,
- "<filesystem type='%s'
accessmode='%s'>\n",
- type, accessmode);
+ virBufferAsprintf(buf, "<filesystem type='%s'", type);
+ if (def->type != VIR_DOMAIN_FS_TYPE_MTP)
+ virBufferAsprintf(buf, " accessmode='%s'", accessmode);
+ virBufferAddLit(buf, ">\n");
+
virBufferAdjustIndent(buf, 2);
if (def->fsdriver) {
virBufferAsprintf(buf, "<driver type='%s'", fsdriver);
@@ -15712,15 +15719,24 @@ virDomainFSDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<source usage='%lld'
units='KiB'/>\n",
def->usage / 1024);
break;
+ case VIR_DOMAIN_FS_TYPE_MTP:
+ virBufferEscapeString(buf, "<source dir='%s'/>\n",
+ def->src);
+ break;
}
- virBufferEscapeString(buf, "<target dir='%s'/>\n",
- def->dst);
+ if (def->type == VIR_DOMAIN_FS_TYPE_MTP)
+ virBufferEscapeString(buf, "<target name='%s'/>\n",
+ def->dst);
+ else
+ virBufferEscapeString(buf, "<target dir='%s'/>\n",
+ def->dst);
if (def->readonly)
virBufferAddLit(buf, "<readonly/>\n");
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ if (def->type != VIR_DOMAIN_FS_TYPE_MTP &&
+ virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bffc0a5..a4d8a76 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -760,6 +760,7 @@ typedef enum {
VIR_DOMAIN_FS_TYPE_TEMPLATE, /* Expands a OS template to a guest dir */
VIR_DOMAIN_FS_TYPE_RAM, /* Mount a RAM filesystem on a guest dir */
VIR_DOMAIN_FS_TYPE_BIND, /* Binds a guest dir to another guest dir */
+ VIR_DOMAIN_FS_TYPE_MTP, /* Binds a host dir to a MTP guest device */
VIR_DOMAIN_FS_TYPE_LAST
} virDomainFSType;
--
1.9.3
4:30 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
On Thu, Aug 07, 2014 at 04:10:31PM +0200, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
src/conf/domain_conf.c | 34 +++++++++++++++++++++++++---------
src/conf/domain_conf.h | 1 +
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c25c74b..3bdf46a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"file",
"template",
"ram",
- "bind")
+ "bind",
+ "mtp")
I don't think this is the right way to represent it.
The 'type' attribute on <filesystem> represents where the backing store
for the filesystem comes from.
The distinction of 9p vs mtp reflects the type of guest device to expose
it as.
We shouldn't try to overload these two concepts in the same attribute.
We should instead try to add a <device> or <model> child element as we
have for some other device types.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:15 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
On Thu, Aug 07, 2014 at 04:10:31PM +0200, Giuseppe Scrivano wrote:
> Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
> ---
> src/conf/domain_conf.c | 34 +++++++++++++++++++++++++---------
> src/conf/domain_conf.h | 1 +
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c25c74b..3bdf46a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
> "file",
> "template",
> "ram",
> - "bind")
> + "bind",
> + "mtp")
I don't think this is the right way to represent it.
The 'type' attribute on <filesystem> represents where the backing store
for the filesystem comes from.
The distinction of 9p vs mtp reflects the type of guest device to expose
it as.
We shouldn't try to overload these two concepts in the same attribute.
We should instead try to add a <device> or <model> child element as we
have for some other device types.
I see, thanks for the clarification.
Would you agree with something like this?
<filesystem type='mount'>
<device name="mtp share">mtp</device>
...
Regards,
Giuseppe
5:24 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
On Mon, Aug 11, 2014 at 12:15:41PM +0200, Giuseppe Scrivano wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
> On Thu, Aug 07, 2014 at 04:10:31PM +0200, Giuseppe Scrivano wrote:
>> Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
>> ---
>> src/conf/domain_conf.c | 34 +++++++++++++++++++++++++---------
>> src/conf/domain_conf.h | 1 +
>> 2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index c25c74b..3bdf46a 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>> "file",
>> "template",
>> "ram",
>> - "bind")
>> + "bind",
>> + "mtp")
>
> I don't think this is the right way to represent it.
>
> The 'type' attribute on <filesystem> represents where the backing
store
> for the filesystem comes from.
>
> The distinction of 9p vs mtp reflects the type of guest device to expose
> it as.
>
> We shouldn't try to overload these two concepts in the same attribute.
> We should instead try to add a <device> or <model> child element as we
> have for some other device types.
I see, thanks for the clarification.
Would you agree with something like this?
<filesystem type='mount'>
<device name="mtp share">mtp</device>
...
What is the name="mtp share" bit trying to reflect ?
It seems we're mostly biased towards <model> so I think we should aim
for <model type='mtp|9p'/>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:32 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
> Would you agree with something like this?
>
> <filesystem type='mount'>
> <device name="mtp share">mtp</device>
> ...
What is the name="mtp share" bit trying to reflect ?
It seems we're mostly biased towards <model> so I think we should aim
for <model type='mtp|9p'/>
it is the name of the MTP share within the guest. How will it look with
<model>?
Thanks,
Giuseppe
5:35 a.m.
New subject: [libvirt] [PATCH libvirt 1/6] conf: add MTP filesystem support to the parser
On Mon, Aug 11, 2014 at 12:32:17PM +0200, Giuseppe Scrivano wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
>> Would you agree with something like this?
>>
>> <filesystem type='mount'>
>> <device name="mtp share">mtp</device>
>> ...
>
> What is the name="mtp share" bit trying to reflect ?
>
> It seems we're mostly biased towards <model> so I think we should aim
> for <model type='mtp|9p'/>
it is the name of the MTP share within the guest. How will it look with
<model>?
For the name of the guest share we'll use the existing <target
path='....'>
element. Yes 'path' is a misleading name - it doesn't have to be a path it
can be any string - this is what we do for 9p already.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 2/6] conf, virDomainFSDefPtr: rename "path" argument to "target"
Since the target for MTP is a name and not a path, make the function
more generic.
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
src/conf/domain_conf.c | 4 ++--
src/conf/domain_conf.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3bdf46a..e8abc1a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18782,12 +18782,12 @@ virDomainFSRemove(virDomainDefPtr def, size_t i)
virDomainFSDefPtr
virDomainGetFilesystemForTarget(virDomainDefPtr def,
- const char *path)
+ const char *target)
{
size_t i;
for (i = 0; i < def->nfss; i++) {
- if (STREQ(def->fss[i]->dst, path))
+ if (STREQ(def->fss[i]->dst, target))
return def->fss[i];
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4d8a76..48968f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2485,7 +2485,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
int *devIdx);
virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def,
- const char *path);
+ const char *target);
int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs);
int virDomainFSIndexByName(virDomainDefPtr def, const char *name);
virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i);
--
1.9.3
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 3/6] qemu_command: fix block indentation
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
src/qemu/qemu_command.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a5ff10a..716be0a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3988,13 +3988,13 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
}
if (fs->wrpolicy) {
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) {
- virBufferAsprintf(&opt, ",writeout=%s", wrpolicy);
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("filesystem writeout not supported"));
- goto error;
- }
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_WRITEOUT)) {
+ virBufferAsprintf(&opt, ",writeout=%s", wrpolicy);
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("filesystem writeout not supported"));
+ goto error;
+ }
}
virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX,
fs->info.alias);
--
1.9.3
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 4/6] qemu: add support for MTP filesystem
Generate the qemu command line option:
-device 'usb-mtp,root=$SRC,desc=$TARGET'
from the definition XML:
<filesystem type='mtp'>
<source dir='$SRC'/>
<target name='$TARGET'/>
</filesystem>
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
src/qemu/qemu_command.c | 53 ++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 716be0a..ad5b318 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3955,12 +3955,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"));
@@ -4029,22 +4023,26 @@ 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);
- 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 (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
+ goto error;
- if (virBufferCheckError(&opt) < 0)
+ if (virBufferCheckError(&opt) < 0)
+ goto error;
+ }
+ else if (fs->type == VIR_DOMAIN_FS_TYPE_MTP) {
+ virBufferAddLit(&opt, "usb-mtp");
+ virBufferAsprintf(&opt, ",root=%s,desc=%s", fs->src,
fs->dst);
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported filesystem type"));
goto error;
+ }
return virBufferContentAndReset(&opt);
@@ -8310,11 +8308,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 &&
+ fs->type != VIR_DOMAIN_FS_TYPE_MTP) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("only supports mount or mtp filesystem
type"));
goto error;
- virCommandAddArg(cmd, optstr);
- VIR_FREE(optstr);
+ }
+
+ if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT) {
+ 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)))
--
1.9.3
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 5/6] docs: add documentation and schema for MTP filesystem type
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/formatdomain.html.in | 9 +++++++++
docs/schemas/domaincommon.rng | 15 ++++++++++++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e5b1adb..938409b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2309,6 +2309,10 @@
<target dir='/import/from/host'/>
<readonly/>
</filesystem>
+ <filesystem type='mtp'>
+ <source dir='/export/to/guest'/>
+ <target name='my-vm-template'/>
+ </filesystem>
...
</devices>
...</pre>
@@ -2367,6 +2371,11 @@
A directory inside the guest will be bound to another
directory inside the guest. Only used by LXC driver
<span class="since"> (since 0.9.13)</span></dd>
+ <dt><code>type='mtp'</code></dt>
+ <dd>
+ A host directory will be exposed to the guest as a MTP
+ device. Only used by QEMU/KVM driver
+ <span class="since"> (since 1.2.8)</span></dd>
</dl>
The filesystem block has an optional attribute <code>accessmode</code>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 11f0fd0..dd870ef 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1899,9 +1899,18 @@
</choice>
<interleave>
<element name="target">
- <attribute name="dir">
- <ref name="absDirPath"/>
- </attribute>
+ <choice>
+ <group>
+ <attribute name="dir">
+ <ref name="absDirPath"/>
+ </attribute>
+ </group>
+ <group>
+ <attribute name="name">
+ <ref name="genericName"/>
+ </attribute>
+ </group>
+ </choice>
<empty/>
</element>
<optional>
--
1.9.3
9:10 a.m.
New subject: [libvirt] [PATCH libvirt 6/6] tests: add tests for MTP filesystem
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
tests/domainconfdata/getfilesystem.xml | 4 ++++
tests/domainconftest.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/tests/domainconfdata/getfilesystem.xml
b/tests/domainconfdata/getfilesystem.xml
index 2ee78b4..efcd83b 100644
--- a/tests/domainconfdata/getfilesystem.xml
+++ b/tests/domainconfdata/getfilesystem.xml
@@ -21,6 +21,10 @@
<source dir='/'/>
<target dir='/dev'/>
</filesystem>
+ <filesystem type='mtp'>
+ <source dir='/'/>
+ <target name='mtp share'/>
+ </filesystem>
<console type='pty'>
<target type='lxc' port='0'/>
</console>
diff --git a/tests/domainconftest.c b/tests/domainconftest.c
index 3d6ebe1..eee3193 100644
--- a/tests/domainconftest.c
+++ b/tests/domainconftest.c
@@ -111,6 +111,8 @@ mymain(void)
DO_TEST_GET_FS("/dev", true);
DO_TEST_GET_FS("/dev/pts", false);
DO_TEST_GET_FS("/doesnotexist", false);
+ DO_TEST_GET_FS("mtp share", true);
+ DO_TEST_GET_FS("mtp not existing share", false);
virObjectUnref(caps);
virObjectUnref(xmlopt);
--
1.9.3
3756
days inactive
3760
days old
11 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Giuseppe Scrivano