[libvirt] [PATCH v2 libvirt 0/8] Add support for qemu usb-mtp device
This series adds support for qemu usb-mtp devices. This new series addresses comments for v1: https://www.redhat.com/archives/libvir-list/2014-August/msg00381.html Now the XML looks like: <filesystem type='mount'> <source dir='/tmp/mtp_root'/> <model type='mtp'/> <target dir='mtp share'/> </filesystem> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1121781 Giuseppe Scrivano (8): conf: add <model> child element to <filesystem> conf, virDomainFSDefPtr: rename "path" argument to "target" qemu_command: fix block indentation qemu: add support for MTP filesystem docs: add documentation and schema for <filesystem>/<model> docs: relax <filesystem>/<target> dir element attribute to string conf: fix comment tests: add tests for filesystem model mtp docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 17 ++++++-- src/conf/domain_conf.c | 29 ++++++++++++- src/conf/domain_conf.h | 15 ++++++- src/qemu/qemu_command.c | 74 +++++++++++++++++++--------------- tests/domainconfdata/getfilesystem.xml | 5 +++ tests/domainconftest.c | 1 + 7 files changed, 108 insertions(+), 39 deletions(-) -- 1.9.3
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7016f3..9252ffa 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", @@ -6458,6 +6463,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; @@ -6535,6 +6541,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; @@ -6556,6 +6565,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, @@ -15795,6 +15812,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 ff7d640..d7664e4 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; }; @@ -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) -- 1.9.3
On 2014/8/11 22:47, Giuseppe Scrivano wrote:
@@ -6458,6 +6463,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; @@ -6535,6 +6541,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; @@ -6556,6 +6565,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,
The memory allocated by model should be freed in cleanup.
Wang Rui <moon.wangrui@huawei.com> writes:
On 2014/8/11 22:47, Giuseppe Scrivano wrote:
@@ -6458,6 +6463,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; @@ -6535,6 +6541,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; @@ -6556,6 +6565,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,
The memory allocated by model should be freed in cleanup.
ping? Any other comment on this series? Thanks, Giuseppe
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++++ 2 files changed, 36 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7016f3..9252ffa 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", @@ -6458,6 +6463,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; @@ -6535,6 +6541,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");
When introducing a new element to the XML it should always go with RNG schema adjustment, docs change and at least one test case. That is squash 5/8 and 8/8 into this patch.
} } cur = cur->next; @@ -6556,6 +6565,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, @@ -15795,6 +15812,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 ff7d640..d7664e4 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; };
@@ -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)
And here are some small nits that you should squash in as well: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c1a9950..d05fabc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6625,6 +6625,7 @@ virDomainFSDefParseXML(xmlNodePtr node, cleanup: ctxt->node = save_node; VIR_FREE(type); + VIR_FREE(model); VIR_FREE(fsdriver); VIR_FREE(target); VIR_FREE(source); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d7664e4..80e0c9a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -767,8 +767,8 @@ typedef enum { /* 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_9P, /* 9P network filesystem */ + VIR_DOMAIN_FS_MODEL_MTP, /* MTP usb filesystem */ VIR_DOMAIN_FS_MODEL_LAST } virDomainFSModel; @@ -817,7 +817,7 @@ struct _virDomainFSDef { virDomainDeviceInfo info; unsigned long long space_hard_limit; /* in bytes */ unsigned long long space_soft_limit; /* in bytes */ - int model; + int model; /* enum virDomainFSModel */ }; Otherwise looking good. Michal
Since the target for MTP is a name and not a path, make the function more generic. Signed-off-by: Giuseppe Scrivano <gscrivan@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 9252ffa..20de23a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18925,12 +18925,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 d7664e4..748dea7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2495,7 +2495,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
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Since the target for MTP is a name and not a path, make the function more generic.
Signed-off-by: Giuseppe Scrivano <gscrivan@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 9252ffa..20de23a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18925,12 +18925,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 d7664e4..748dea7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2495,7 +2495,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);
ACK and can be pushed now. Michal
Signed-off-by: Giuseppe Scrivano <gscrivan@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 8a69976..87569b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3989,13 +3989,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
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@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 8a69976..87569b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3989,13 +3989,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);
ACK. Again independent patch. Michal
Michal Privoznik <mprivozn@redhat.com> writes:
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@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 8a69976..87569b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3989,13 +3989,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);
ACK. Again independent patch.
going to push it soon. Thanks, Giuseppe
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) { + 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")); goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); + } + + if (fs->type == VIR_DOMAIN_FS_TYPE_MOUNT && + 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))) -- 1.9.3
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
Michal Privoznik <mprivozn@redhat.com> writes:
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.
I have amended your changes and added a new test. Going to send a v3 soon. Thanks, Giuseppe
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b5df51..c2d04a9 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"> -- 1.9.3
It is used by mtp model type to specify the mtp share name. Signed-off-by: Giuseppe Scrivano <gscrivan@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
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 748dea7..32ca32d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -752,7 +752,7 @@ struct _virDomainControllerDef { }; -/* Two types of disk backends */ +/* Types of disk backends */ typedef enum { VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir */ VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */ -- 1.9.3
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 748dea7..32ca32d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -752,7 +752,7 @@ struct _virDomainControllerDef { };
-/* Two types of disk backends */ +/* Types of disk backends */ typedef enum { VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir */ VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */
ACK, again can be pushed out of order. Michal
Michal Privoznik <mprivozn@redhat.com> writes:
On 11.08.2014 16:47, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 748dea7..32ca32d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -752,7 +752,7 @@ struct _virDomainControllerDef { };
-/* Two types of disk backends */ +/* Types of disk backends */ typedef enum { VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir */ VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */
ACK, again can be pushed out of order.
thanks, I am going to push it soon. Giuseppe
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- tests/domainconfdata/getfilesystem.xml | 5 +++++ tests/domainconftest.c | 1 + 2 files changed, 6 insertions(+) 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
participants (3)
-
Giuseppe Scrivano -
Michal Privoznik -
Wang Rui