3:11 p.m.
This series adds support for qemu usb-mtp devices.
This new version addresses comments for v2:
https://www.redhat.com/archives/libvir-list/2014-August/msg00471.html
Giuseppe Scrivano (3):
conf: add <model> child element to <filesystem>
docs: relax <filesystem>/<target> dir element attribute to string
qemu: add support for MTP filesystem
docs/formatdomain.html.in | 6 +++
docs/schemas/domaincommon.rng | 17 +++++--
src/conf/domain_conf.c | 26 +++++++++++
src/conf/domain_conf.h | 11 +++++
src/qemu/qemu_command.c | 65 ++++++++++++++++----------
tests/domainconfdata/getfilesystem.xml | 5 ++
tests/domainconftest.c | 1 +
tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args | 6 +++
tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml | 30 ++++++++++++
9 files changed, 140 insertions(+), 27 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-fsmtp.xml
--
1.9.3
3:11 p.m.
New subject: [libvirt] [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/formatdomain.html.in | 6 ++++++
docs/schemas/domaincommon.rng | 13 +++++++++++++
src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
src/conf/domain_conf.h | 11 +++++++++++
tests/domainconfdata/getfilesystem.xml | 5 +++++
tests/domainconftest.c | 1 +
6 files changed, 62 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ed17389..d0c1ce2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2300,6 +2300,8 @@
<driver type='path' wrpolicy='immediate'/>
<source dir='/export/to/guest'/>
<target dir='/import/from/host'/>
+ <model type='9p'/>
+ <model type='mtp'/>
<readonly/>
</filesystem>
<filesystem type='file' accessmode='passthrough'>
@@ -2337,6 +2339,10 @@
while the value <code>immediate</code> means that a host writeback
is immediately triggered for all pages touched during a guest file
write operation <span class="since">(since 0.9.10)</span>.
+ A "filesystem" element has an optional
+ attribute <code>model</code><span class="since">
(since
+ 1.2.8)</span>, which is one of "9p", "mtp" (used by
QEMU/KVM),
+ if this element is not specified the default is "9p".
</dd>
<dt><code>type='template'</code></dt>
<dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 033f2f6..684acec 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1937,6 +1937,19 @@
</element>
</optional>
</interleave>
+ <interleave>
+ <optional>
+ <element name="model">
+ <attribute name="type">
+ <choice>
+ <value>9p</value>
+ <value>mtp</value>
+ </choice>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ </interleave>
</element>
</define>
<define name="fsDriver">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 14338ba..98dbe14 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"ram",
"bind")
+VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST,
+ "default",
+ "9p",
+ "mtp")
+
VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"default",
"path",
@@ -6459,6 +6464,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
virDomainFSDefPtr def;
xmlNodePtr cur, save_node = ctxt->node;
char *type = NULL;
+ char *model = NULL;
char *fsdriver = NULL;
char *source = NULL;
char *target = NULL;
@@ -6536,6 +6542,9 @@ virDomainFSDefParseXML(xmlNodePtr node,
wrpolicy = virXMLPropString(cur, "wrpolicy");
if (!format)
format = virXMLPropString(cur, "format");
+ } else if (!model &&
+ xmlStrEqual(cur->name, BAD_CAST "model")) {
+ model = virXMLPropString(cur, "type");
}
}
cur = cur->next;
@@ -6557,6 +6566,14 @@ virDomainFSDefParseXML(xmlNodePtr node,
}
}
+ if (model) {
+ if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown model value '%s'"), model);
+ goto error;
+ }
+ }
+
if (wrpolicy) {
if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -6616,6 +6633,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
VIR_FREE(usage);
VIR_FREE(units);
VIR_FREE(format);
+ VIR_FREE(model);
return def;
@@ -15786,6 +15804,14 @@ virDomainFSDefFormat(virBufferPtr buf,
switch (def->type) {
case VIR_DOMAIN_FS_TYPE_MOUNT:
+ virBufferEscapeString(buf, "<source dir='%s'/>\n",
+ def->src);
+ if (def->model) {
+ virBufferEscapeString(buf, "<model type='%s'/>\n",
+ virDomainFSModelTypeToString(def->model));
+ }
+ break;
+
case VIR_DOMAIN_FS_TYPE_BIND:
virBufferEscapeString(buf, "<source dir='%s'/>\n",
def->src);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4cf56c9..df7d019 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -764,6 +764,15 @@ typedef enum {
VIR_DOMAIN_FS_TYPE_LAST
} virDomainFSType;
+/* Filesystem model */
+typedef enum {
+ VIR_DOMAIN_FS_MODEL_DEFAULT = 0,
+ VIR_DOMAIN_FS_MODEL_9P, /* 9P network filesystem */
+ VIR_DOMAIN_FS_MODEL_MTP, /* MTP usb filesystem */
+
+ VIR_DOMAIN_FS_MODEL_LAST
+} virDomainFSModel;
+
/* Filesystem driver type */
typedef enum {
VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0,
@@ -808,6 +817,7 @@ struct _virDomainFSDef {
virDomainDeviceInfo info;
unsigned long long space_hard_limit; /* in bytes */
unsigned long long space_soft_limit; /* in bytes */
+ int model; /* enum virDomainFSModel */
};
@@ -2585,6 +2595,7 @@ VIR_ENUM_DECL(virDomainControllerModelPCI)
VIR_ENUM_DECL(virDomainControllerModelSCSI)
VIR_ENUM_DECL(virDomainControllerModelUSB)
VIR_ENUM_DECL(virDomainFS)
+VIR_ENUM_DECL(virDomainFSModel)
VIR_ENUM_DECL(virDomainFSDriver)
VIR_ENUM_DECL(virDomainFSAccessMode)
VIR_ENUM_DECL(virDomainFSWrpolicy)
diff --git a/tests/domainconfdata/getfilesystem.xml
b/tests/domainconfdata/getfilesystem.xml
index 2ee78b4..3203666 100644
--- a/tests/domainconfdata/getfilesystem.xml
+++ b/tests/domainconfdata/getfilesystem.xml
@@ -21,6 +21,11 @@
<source dir='/'/>
<target dir='/dev'/>
</filesystem>
+ <filesystem type='mount'>
+ <model type="mtp"/>
+ <source dir='/'/>
+ <target dir='mtp share'/>
+ </filesystem>
<console type='pty'>
<target type='lxc' port='0'/>
</console>
diff --git a/tests/domainconftest.c b/tests/domainconftest.c
index 3d6ebe1..6b65f09 100644
--- a/tests/domainconftest.c
+++ b/tests/domainconftest.c
@@ -111,6 +111,7 @@ 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);
virObjectUnref(caps);
virObjectUnref(xmlopt);
--
1.9.3
3:46 a.m.
New subject: [libvirt] [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>
On Tue, Aug 19, 2014 at 10:11:35PM +0200, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/formatdomain.html.in | 6 ++++++
docs/schemas/domaincommon.rng | 13 +++++++++++++
src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
src/conf/domain_conf.h | 11 +++++++++++
tests/domainconfdata/getfilesystem.xml | 5 +++++
tests/domainconftest.c | 1 +
6 files changed, 62 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ed17389..d0c1ce2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2300,6 +2300,8 @@
<driver type='path' wrpolicy='immediate'/>
<source dir='/export/to/guest'/>
<target dir='/import/from/host'/>
+ <model type='9p'/>
+ <model type='mtp'/>
<readonly/>
</filesystem>
<filesystem type='file' accessmode='passthrough'>
@@ -2337,6 +2339,10 @@
while the value <code>immediate</code> means that a host writeback
is immediately triggered for all pages touched during a guest file
write operation <span class="since">(since
0.9.10)</span>.
+ A "filesystem" element has an optional
+ attribute <code>model</code><span class="since">
(since
+ 1.2.8)</span>, which is one of "9p", "mtp" (used by
QEMU/KVM),
+ if this element is not specified the default is "9p".
</dd>
<dt><code>type='template'</code></dt>
<dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 033f2f6..684acec 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1937,6 +1937,19 @@
</element>
</optional>
</interleave>
+ <interleave>
+ <optional>
+ <element name="model">
+ <attribute name="type">
+ <choice>
+ <value>9p</value>
+ <value>mtp</value>
+ </choice>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ </interleave>
</element>
</define>
<define name="fsDriver">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 14338ba..98dbe14 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"ram",
"bind")
+VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST,
+ "default",
+ "9p",
+ "mtp")
+
VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"default",
"path",
@@ -6459,6 +6464,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
virDomainFSDefPtr def;
xmlNodePtr cur, save_node = ctxt->node;
char *type = NULL;
+ char *model = NULL;
char *fsdriver = NULL;
char *source = NULL;
char *target = NULL;
@@ -6536,6 +6542,9 @@ virDomainFSDefParseXML(xmlNodePtr node,
wrpolicy = virXMLPropString(cur, "wrpolicy");
if (!format)
format = virXMLPropString(cur, "format");
+ } else if (!model &&
+ xmlStrEqual(cur->name, BAD_CAST "model")) {
+ model = virXMLPropString(cur, "type");
}
}
cur = cur->next;
@@ -6557,6 +6566,14 @@ virDomainFSDefParseXML(xmlNodePtr node,
}
}
+ if (model) {
+ if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown model value '%s'"), model);
+ goto error;
+ }
+ }
+
if (wrpolicy) {
if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -6616,6 +6633,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
VIR_FREE(usage);
VIR_FREE(units);
VIR_FREE(format);
+ VIR_FREE(model);
return def;
@@ -15786,6 +15804,14 @@ virDomainFSDefFormat(virBufferPtr buf,
switch (def->type) {
case VIR_DOMAIN_FS_TYPE_MOUNT:
+ virBufferEscapeString(buf, "<source dir='%s'/>\n",
+ def->src);
+ if (def->model) {
+ virBufferEscapeString(buf, "<model type='%s'/>\n",
+ virDomainFSModelTypeToString(def->model));
+ }
From the XML pov, we shouldn't restrict use of <model> to
only be
for FS_TYPE_MOUNT. That is a QEMU driver implementation restriction,
so just make the QEMU driver raise VIR_ERR_CONFIG_UNSUPPORTED for
the cases we don't want, when building the CLI args
+ break;
+
case VIR_DOMAIN_FS_TYPE_BIND:
virBufferEscapeString(buf, "<source dir='%s'/>\n",
def->src);
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 :|
4:21 a.m.
New subject: [libvirt] [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
From the XML pov, we shouldn't restrict use of <model> to
only be
for FS_TYPE_MOUNT. That is a QEMU driver implementation restriction,
so just make the QEMU driver raise VIR_ERR_CONFIG_UNSUPPORTED for
the cases we don't want, when building the CLI args
OK to amend this piece? The QEMU driver supports only FS_TYPE_MOUNT so
the additional check seems reduntant there.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 98dbe14..c897f1b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15806,10 +15806,6 @@ virDomainFSDefFormat(virBufferPtr buf,
case VIR_DOMAIN_FS_TYPE_MOUNT:
virBufferEscapeString(buf, "<source dir='%s'/>\n",
def->src);
- if (def->model) {
- virBufferEscapeString(buf, "<model type='%s'/>\n",
- virDomainFSModelTypeToString(def->model));
- }
break;
case VIR_DOMAIN_FS_TYPE_BIND:
@@ -15838,6 +15834,11 @@ virDomainFSDefFormat(virBufferPtr buf,
break;
}
+ if (def->model) {
+ virBufferEscapeString(buf, "<model type='%s'/>\n",
+ virDomainFSModelTypeToString(def->model));
+ }
+
virBufferEscapeString(buf, "<target dir='%s'/>\n",
def->dst);
Thanks,
Giuseppe
4:36 a.m.
New subject: [libvirt] [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>
On Wed, Aug 20, 2014 at 11:21:24AM +0200, Giuseppe Scrivano wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
> From the XML pov, we shouldn't restrict use of <model> to only be
> for FS_TYPE_MOUNT. That is a QEMU driver implementation restriction,
> so just make the QEMU driver raise VIR_ERR_CONFIG_UNSUPPORTED for
> the cases we don't want, when building the CLI args
OK to amend this piece? The QEMU driver supports only FS_TYPE_MOUNT so
the additional check seems reduntant there.
Yes, I see QEMU driver already has sufficient checks for this.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 98dbe14..c897f1b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15806,10 +15806,6 @@ virDomainFSDefFormat(virBufferPtr buf,
case VIR_DOMAIN_FS_TYPE_MOUNT:
virBufferEscapeString(buf, "<source dir='%s'/>\n",
def->src);
- if (def->model) {
- virBufferEscapeString(buf, "<model type='%s'/>\n",
- virDomainFSModelTypeToString(def->model));
- }
break;
case VIR_DOMAIN_FS_TYPE_BIND:
@@ -15838,6 +15834,11 @@ virDomainFSDefFormat(virBufferPtr buf,
break;
}
+ if (def->model) {
+ virBufferEscapeString(buf, "<model type='%s'/>\n",
+ virDomainFSModelTypeToString(def->model));
+ }
+
virBufferEscapeString(buf, "<target dir='%s'/>\n",
def->dst);
ACK to the whole patch with this applied.
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 :|
6:46 a.m.
New subject: [libvirt] [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>
On 19.08.2014 22:11, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/formatdomain.html.in | 6 ++++++
docs/schemas/domaincommon.rng | 13 +++++++++++++
src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
src/conf/domain_conf.h | 11 +++++++++++
tests/domainconfdata/getfilesystem.xml | 5 +++++
tests/domainconftest.c | 1 +
6 files changed, 62 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ed17389..d0c1ce2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2300,6 +2300,8 @@
<driver type='path' wrpolicy='immediate'/>
<source dir='/export/to/guest'/>
<target dir='/import/from/host'/>
+ <model type='9p'/>
+ <model type='mtp'/>
No. Having two <model/> elements is not supported. Just choose one and
describe other in the paragraph below (like you did).
<readonly/>
</filesystem>
<filesystem type='file' accessmode='passthrough'>
@@ -2337,6 +2339,10 @@
while the value <code>immediate</code> means that a host writeback
is immediately triggered for all pages touched during a guest file
write operation <span class="since">(since
0.9.10)</span>.
+ A "filesystem" element has an optional
+ attribute <code>model</code><span class="since">
(since
+ 1.2.8)</span>, which is one of "9p", "mtp" (used by
QEMU/KVM),
+ if this element is not specified the default is "9p".
</dd>
<dt><code>type='template'</code></dt>
<dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 033f2f6..684acec 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1937,6 +1937,19 @@
</element>
</optional>
</interleave>
+ <interleave>
+ <optional>
+ <element name="model">
+ <attribute name="type">
+ <choice>
+ <value>9p</value>
+ <value>mtp</value>
+ </choice>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ </interleave>
</element>
</define>
<define name="fsDriver">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 14338ba..98dbe14 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
"ram",
"bind")
+VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST,
+ "default",
+ "9p",
+ "mtp")
+
VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"default",
"path",
@@ -6459,6 +6464,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
virDomainFSDefPtr def;
xmlNodePtr cur, save_node = ctxt->node;
char *type = NULL;
+ char *model = NULL;
char *fsdriver = NULL;
char *source = NULL;
char *target = NULL;
@@ -6536,6 +6542,9 @@ virDomainFSDefParseXML(xmlNodePtr node,
wrpolicy = virXMLPropString(cur, "wrpolicy");
if (!format)
format = virXMLPropString(cur, "format");
+ } else if (!model &&
+ xmlStrEqual(cur->name, BAD_CAST "model")) {
+ model = virXMLPropString(cur, "type");
}
}
cur = cur->next;
@@ -6557,6 +6566,14 @@ virDomainFSDefParseXML(xmlNodePtr node,
}
}
+ if (model) {
+ if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown model value '%s'"), model);
+ goto error;
+ }
+ }
+
if (wrpolicy) {
if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0)
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -6616,6 +6633,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
VIR_FREE(usage);
VIR_FREE(units);
VIR_FREE(format);
+ VIR_FREE(model);
Maybe a nitpick, but I'd love to keep an ordering here. I see two
possible ways of doing free over multiple variables. Either the same
oder in which they are declared, or the reverse one. So this VIR_FREE()
needs to go in between VIR_FREE(type) and VIR_FREE(fsdriver).
return def;
@@ -15786,6 +15804,14 @@ virDomainFSDefFormat(virBufferPtr buf,
switch (def->type) {
case VIR_DOMAIN_FS_TYPE_MOUNT:
+ virBufferEscapeString(buf, "<source dir='%s'/>\n",
+ def->src);
+ if (def->model) {
+ virBufferEscapeString(buf, "<model type='%s'/>\n",
+ virDomainFSModelTypeToString(def->model));
+ }
+ break;
+
case VIR_DOMAIN_FS_TYPE_BIND:
virBufferEscapeString(buf, "<source dir='%s'/>\n",
def->src);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4cf56c9..df7d019 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -764,6 +764,15 @@ typedef enum {
VIR_DOMAIN_FS_TYPE_LAST
} virDomainFSType;
+/* Filesystem model */
+typedef enum {
+ VIR_DOMAIN_FS_MODEL_DEFAULT = 0,
+ VIR_DOMAIN_FS_MODEL_9P, /* 9P network filesystem */
+ VIR_DOMAIN_FS_MODEL_MTP, /* MTP usb filesystem */
+
+ VIR_DOMAIN_FS_MODEL_LAST
+} virDomainFSModel;
+
/* Filesystem driver type */
typedef enum {
VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0,
@@ -808,6 +817,7 @@ struct _virDomainFSDef {
virDomainDeviceInfo info;
unsigned long long space_hard_limit; /* in bytes */
unsigned long long space_soft_limit; /* in bytes */
+ int model; /* enum virDomainFSModel */
};
@@ -2585,6 +2595,7 @@ VIR_ENUM_DECL(virDomainControllerModelPCI)
VIR_ENUM_DECL(virDomainControllerModelSCSI)
VIR_ENUM_DECL(virDomainControllerModelUSB)
VIR_ENUM_DECL(virDomainFS)
+VIR_ENUM_DECL(virDomainFSModel)
VIR_ENUM_DECL(virDomainFSDriver)
VIR_ENUM_DECL(virDomainFSAccessMode)
VIR_ENUM_DECL(virDomainFSWrpolicy)
diff --git a/tests/domainconfdata/getfilesystem.xml
b/tests/domainconfdata/getfilesystem.xml
index 2ee78b4..3203666 100644
--- a/tests/domainconfdata/getfilesystem.xml
+++ b/tests/domainconfdata/getfilesystem.xml
@@ -21,6 +21,11 @@
<source dir='/'/>
<target dir='/dev'/>
</filesystem>
+ <filesystem type='mount'>
+ <model type="mtp"/>
+ <source dir='/'/>
+ <target dir='mtp share'/>
+ </filesystem>
<console type='pty'>
<target type='lxc' port='0'/>
</console>
diff --git a/tests/domainconftest.c b/tests/domainconftest.c
index 3d6ebe1..6b65f09 100644
--- a/tests/domainconftest.c
+++ b/tests/domainconftest.c
@@ -111,6 +111,7 @@ 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);
virObjectUnref(caps);
virObjectUnref(xmlopt);
ACK with this and nits pointed out by Daniel fixed.
Michal
3:11 p.m.
New subject: [libvirt] [PATCH v3 libvirt 2/3] docs: relax <filesystem>/<target> dir element attribute to string
It is used by mtp model type to specify the mtp share name.
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/schemas/domaincommon.rng | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 684acec..4178644 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1899,9 +1899,7 @@
</choice>
<interleave>
<element name="target">
- <attribute name="dir">
- <ref name="absDirPath"/>
- </attribute>
+ <attribute name="dir"/>
<empty/>
</element>
<optional>
--
1.9.3
6:46 a.m.
New subject: [libvirt] [PATCH v3 libvirt 2/3] docs: relax <filesystem>/<target> dir element attribute to string
On 19.08.2014 22:11, Giuseppe Scrivano wrote:
It is used by mtp model type to specify the mtp share name.
Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com>
---
docs/schemas/domaincommon.rng | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 684acec..4178644 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1899,9 +1899,7 @@
</choice>
<interleave>
<element name="target">
- <attribute name="dir">
- <ref name="absDirPath"/>
- </attribute>
+ <attribute name="dir"/>
<empty/>
</element>
<optional>
See my comment to 3/3
Michal
3:11 p.m.
New subject: [libvirt] [PATCH v3 libvirt 3/3] 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='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(a)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);
+ 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>
--
1.9.3
6:46 a.m.
New subject: [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(a)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(a)redhat.com>
AuthorDate: Tue Jul 22 09:30:12 2014 +0200
Commit: Gerd Hoffmann <kraxel(a)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(a)redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel(a)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
7:19 a.m.
New subject: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
Michal Privoznik <mprivozn(a)redhat.com> writes:
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.
I am not sure how usually libvirt deals with these cases, but to me it
seems like that we should use the "stable" attribute name; or just
postpone this series/feature.
And what's this desc= attribute for anyway? I've put in several
different values and it seems to me like nothing changed.
it is the name given to the MTP share on the guest, I see it displayed
in the guest, at least Nautilus shows it.
Regards,
Giuseppe
7:41 a.m.
New subject: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
On 08/20/2014 06:19 AM, Giuseppe Scrivano wrote:
Michal Privoznik <mprivozn(a)redhat.com> writes:
> 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.
I am not sure how usually libvirt deals with these cases, but to me it
seems like that we should use the "stable" attribute name; or just
postpone this series/feature.
>
> And what's this desc= attribute for anyway? I've put in several
> different values and it seems to me like nothing changed.
it is the name given to the MTP share on the guest, I see it displayed
in the guest, at least Nautilus shows it.
Is the use of 'root'/'x-root' mandatory? Is it something where we can
commit to only a stable subset of the interface, with no way to name the
root, and extend it later?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:46 a.m.
New subject: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
On 20.08.2014 14:41, Eric Blake wrote:
On 08/20/2014 06:19 AM, Giuseppe Scrivano wrote:
> Michal Privoznik <mprivozn(a)redhat.com> writes:
>
>> 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.
>
> I am not sure how usually libvirt deals with these cases, but to me it
> seems like that we should use the "stable" attribute name; or just
> postpone this series/feature.
>
>>
>> And what's this desc= attribute for anyway? I've put in several
>> different values and it seems to me like nothing changed.
>
> it is the name given to the MTP share on the guest, I see it displayed
> in the guest, at least Nautilus shows it.
Is the use of 'root'/'x-root' mandatory? Is it something where we can
commit to only a stable subset of the interface, with no way to name the
root, and extend it later?
In fact, it's the most interesting attribute as it denotes the path that
is to be shared via MTP. Without it, the whole MTP device is useless.
Having said that, I vote for postponing this patch until the dust on
qemu side settles down.
Michal
7:40 a.m.
New subject: [libvirt] [PATCH v3 libvirt 3/3] qemu: add support for MTP filesystem
On 08/20/2014 05:46 AM, Michal Privoznik wrote:
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(a)redhat.com>
> ---
> + 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:
Ouch. Qemu has explicitly documented that anything starting with x- is
unstable, and may change. We probably should NOT be targetting this in
libvirt API just yet, in case qemu changes their mind. It's nice that
you've done the patch as a proof of concept, but I don't think we want
it in the tree yet :(
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(a)redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel(a)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.
Yep - anything in qemu with x- is not a candidate for libvirt support
(for the longest time, we refused to support RDMA migration because it
still had an x- prefix).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3747
days inactive
3748
days old
13 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Giuseppe Scrivano -
Michal Privoznik