* [Qemu-devel] [PULL 1/5] usb-mtp: Add one more argument when building results
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
@ 2018-02-20 15:28 ` Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 2/5] usb-mtp: print parent path in IN_IGNORED trace fn Gerd Hoffmann
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-02-20 15:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Bandan Das, Gerd Hoffmann
From: Bandan Das <bsd@redhat.com>
The response to a SendObjectInfo consists of the storageid,
parent obejct handle and the handle reserved for the new
incoming object
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180215231129.14710-2-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-mtp.c | 50 +++++++++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 94c2e94f10..b55aa8205e 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -765,7 +765,8 @@ static void usb_mtp_add_time(MTPData *data, time_t time)
/* ----------------------------------------------------------------------- */
static void usb_mtp_queue_result(MTPState *s, uint16_t code, uint32_t trans,
- int argc, uint32_t arg0, uint32_t arg1)
+ int argc, uint32_t arg0, uint32_t arg1,
+ uint32_t arg2)
{
MTPControl *c = g_new0(MTPControl, 1);
@@ -778,6 +779,9 @@ static void usb_mtp_queue_result(MTPState *s, uint16_t code, uint32_t trans,
if (argc > 1) {
c->argv[1] = arg1;
}
+ if (argc > 2) {
+ c->argv[2] = arg2;
+ }
assert(s->result == NULL);
s->result = c;
@@ -1119,7 +1123,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
/* sanity checks */
if (c->code >= CMD_CLOSE_SESSION && s->session == 0) {
usb_mtp_queue_result(s, RES_SESSION_NOT_OPEN,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
@@ -1131,12 +1135,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
case CMD_OPEN_SESSION:
if (s->session) {
usb_mtp_queue_result(s, RES_SESSION_ALREADY_OPEN,
- c->trans, 1, s->session, 0);
+ c->trans, 1, s->session, 0, 0);
return;
}
if (c->argv[0] == 0) {
usb_mtp_queue_result(s, RES_INVALID_PARAMETER,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
trace_usb_mtp_op_open_session(s->dev.addr);
@@ -1165,7 +1169,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
if (c->argv[0] != QEMU_STORAGE_ID &&
c->argv[0] != 0xffffffff) {
usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_storage_info(s, c);
@@ -1175,12 +1179,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
if (c->argv[0] != QEMU_STORAGE_ID &&
c->argv[0] != 0xffffffff) {
usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
if (c->argv[1] != 0x00000000) {
usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
if (c->argv[2] == 0x00000000 ||
@@ -1191,12 +1195,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
}
if (o == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
if (o->format != FMT_ASSOCIATION) {
usb_mtp_queue_result(s, RES_INVALID_PARENT_OBJECT,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
usb_mtp_object_readdir(s, o);
@@ -1212,7 +1216,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
o = usb_mtp_object_lookup(s, c->argv[0]);
if (o == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_object_info(s, c, o);
@@ -1221,18 +1225,18 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
o = usb_mtp_object_lookup(s, c->argv[0]);
if (o == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
if (o->format == FMT_ASSOCIATION) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_object(s, c, o);
if (data_in == NULL) {
usb_mtp_queue_result(s, RES_GENERAL_ERROR,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
break;
@@ -1240,18 +1244,18 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
o = usb_mtp_object_lookup(s, c->argv[0]);
if (o == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
if (o->format == FMT_ASSOCIATION) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_partial_object(s, c, o);
if (data_in == NULL) {
usb_mtp_queue_result(s, RES_GENERAL_ERROR,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
nres = 1;
@@ -1261,7 +1265,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
c->argv[0] != FMT_ASSOCIATION) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_object_props_supported(s, c);
@@ -1270,13 +1274,13 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
if (c->argv[1] != FMT_UNDEFINED_OBJECT &&
c->argv[1] != FMT_ASSOCIATION) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_FORMAT_CODE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_object_prop_desc(s, c);
if (data_in == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_PROP_CODE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
break;
@@ -1284,20 +1288,20 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
o = usb_mtp_object_lookup(s, c->argv[0]);
if (o == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
data_in = usb_mtp_get_object_prop_value(s, c, o);
if (data_in == NULL) {
usb_mtp_queue_result(s, RES_INVALID_OBJECT_PROP_CODE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
break;
default:
trace_usb_mtp_op_unknown(s->dev.addr, c->code);
usb_mtp_queue_result(s, RES_OPERATION_NOT_SUPPORTED,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
return;
}
@@ -1306,7 +1310,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
assert(s->data_in == NULL);
s->data_in = data_in;
}
- usb_mtp_queue_result(s, RES_OK, c->trans, nres, res0, 0);
+ usb_mtp_queue_result(s, RES_OK, c->trans, nres, res0, 0, 0);
}
/* ----------------------------------------------------------------------- */
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PULL 2/5] usb-mtp: print parent path in IN_IGNORED trace fn
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 1/5] usb-mtp: Add one more argument when building results Gerd Hoffmann
@ 2018-02-20 15:28 ` Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects Gerd Hoffmann
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-02-20 15:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Bandan Das, Gerd Hoffmann
From: Bandan Das <bsd@redhat.com>
Fix a possible null dereference when deleting a folder and
its contents. An ignored event might be received for its contents
after the parent folder is deleted which will return a null object.
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180215231129.14710-3-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-mtp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index b55aa8205e..63f8f3b90b 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -540,9 +540,8 @@ static void inotify_watchfn(void *arg)
break;
case IN_IGNORED:
- o = usb_mtp_object_lookup_name(parent, event->name, event->len);
- trace_usb_mtp_inotify_event(s->dev.addr, o->path,
- event->mask, "Obj ignored");
+ trace_usb_mtp_inotify_event(s->dev.addr, parent->path,
+ event->mask, "Obj parent dir ignored");
break;
default:
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 1/5] usb-mtp: Add one more argument when building results Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 2/5] usb-mtp: print parent path in IN_IGNORED trace fn Gerd Hoffmann
@ 2018-02-20 15:28 ` Gerd Hoffmann
2018-02-20 15:28 ` [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects Gerd Hoffmann
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-02-20 15:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Bandan Das, Gerd Hoffmann
From: Bandan Das <bsd@redhat.com>
Write of existing objects by the initiator is acheived by
making a temporary buffer with the new changes, deleting the
old file and then writing a new file with the same name.
Also, add a "readonly" property which needs to be set to false
for deletion to work.
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180215231129.14710-4-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-mtp.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 63f8f3b90b..5ef77f3e9f 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -46,6 +46,7 @@ enum mtp_code {
CMD_GET_OBJECT_HANDLES = 0x1007,
CMD_GET_OBJECT_INFO = 0x1008,
CMD_GET_OBJECT = 0x1009,
+ CMD_DELETE_OBJECT = 0x100b,
CMD_GET_PARTIAL_OBJECT = 0x101b,
CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
CMD_GET_OBJECT_PROP_DESC = 0x9802,
@@ -62,6 +63,8 @@ enum mtp_code {
RES_INVALID_STORAGE_ID = 0x2008,
RES_INVALID_OBJECT_HANDLE = 0x2009,
RES_INVALID_OBJECT_FORMAT_CODE = 0x200b,
+ RES_STORE_READ_ONLY = 0x200e,
+ RES_PARTIAL_DELETE = 0x2012,
RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
RES_INVALID_PARENT_OBJECT = 0x201a,
RES_INVALID_PARAMETER = 0x201d,
@@ -172,6 +175,7 @@ struct MTPState {
MTPControl *result;
uint32_t session;
uint32_t next_handle;
+ bool readonly;
QTAILQ_HEAD(, MTPObject) objects;
#ifdef CONFIG_INOTIFY1
@@ -799,6 +803,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c)
CMD_GET_NUM_OBJECTS,
CMD_GET_OBJECT_HANDLES,
CMD_GET_OBJECT_INFO,
+ CMD_DELETE_OBJECT,
CMD_GET_OBJECT,
CMD_GET_PARTIAL_OBJECT,
CMD_GET_OBJECT_PROPS_SUPPORTED,
@@ -1113,6 +1118,116 @@ static MTPData *usb_mtp_get_object_prop_value(MTPState *s, MTPControl *c,
return d;
}
+/* Return correct return code for a delete event */
+enum {
+ ALL_DELETE,
+ PARTIAL_DELETE,
+ READ_ONLY,
+};
+
+/* Assumes that children, if any, have been already freed */
+static void usb_mtp_object_free_one(MTPState *s, MTPObject *o)
+{
+#ifndef CONFIG_INOTIFY1
+ assert(o->nchildren == 0);
+ QTAILQ_REMOVE(&s->objects, o, next);
+ g_free(o->name);
+ g_free(o->path);
+ g_free(o);
+#endif
+}
+
+static int usb_mtp_deletefn(MTPState *s, MTPObject *o, uint32_t trans)
+{
+ MTPObject *iter, *iter2;
+ bool partial_delete = false;
+ bool success = false;
+
+ /*
+ * TODO: Add support for Protection Status
+ */
+
+ QLIST_FOREACH(iter, &o->children, list) {
+ if (iter->format == FMT_ASSOCIATION) {
+ QLIST_FOREACH(iter2, &iter->children, list) {
+ usb_mtp_deletefn(s, iter2, trans);
+ }
+ }
+ }
+
+ if (o->format == FMT_UNDEFINED_OBJECT) {
+ if (remove(o->path)) {
+ partial_delete = true;
+ } else {
+ usb_mtp_object_free_one(s, o);
+ success = true;
+ }
+ }
+
+ if (o->format == FMT_ASSOCIATION) {
+ if (rmdir(o->path)) {
+ partial_delete = true;
+ } else {
+ usb_mtp_object_free_one(s, o);
+ success = true;
+ }
+ }
+
+ if (success && partial_delete) {
+ return PARTIAL_DELETE;
+ }
+ if (!success && partial_delete) {
+ return READ_ONLY;
+ }
+ return ALL_DELETE;
+}
+
+static void usb_mtp_object_delete(MTPState *s, uint32_t handle,
+ uint32_t format_code, uint32_t trans)
+{
+ MTPObject *o;
+ int ret;
+
+ /* Return error if store is read-only */
+ if (!FLAG_SET(s, MTP_FLAG_WRITABLE)) {
+ usb_mtp_queue_result(s, RES_STORE_READ_ONLY,
+ trans, 0, 0, 0, 0);
+ return;
+ }
+
+ if (format_code != 0) {
+ usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
+ trans, 0, 0, 0, 0);
+ return;
+ }
+
+ if (handle == 0xFFFFFFF) {
+ o = QTAILQ_FIRST(&s->objects);
+ } else {
+ o = usb_mtp_object_lookup(s, handle);
+ }
+ if (o == NULL) {
+ usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
+ trans, 0, 0, 0, 0);
+ return;
+ }
+
+ ret = usb_mtp_deletefn(s, o, trans);
+ if (ret == PARTIAL_DELETE) {
+ usb_mtp_queue_result(s, RES_PARTIAL_DELETE,
+ trans, 0, 0, 0, 0);
+ return;
+ } else if (ret == READ_ONLY) {
+ usb_mtp_queue_result(s, RES_STORE_READ_ONLY, trans,
+ 0, 0, 0, 0);
+ return;
+ } else {
+ usb_mtp_queue_result(s, RES_OK, trans,
+ 0, 0, 0, 0);
+ return;
+ }
+}
+
static void usb_mtp_command(MTPState *s, MTPControl *c)
{
MTPData *data_in = NULL;
@@ -1239,6 +1354,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
return;
}
break;
+ case CMD_DELETE_OBJECT:
+ usb_mtp_object_delete(s, c->argv[0], c->argv[1], c->trans);
+ return;
case CMD_GET_PARTIAL_OBJECT:
o = usb_mtp_object_lookup(s, c->argv[0]);
if (o == NULL) {
@@ -1545,6 +1663,10 @@ static void usb_mtp_realize(USBDevice *dev, Error **errp)
return;
}
s->desc = strrchr(s->root, '/');
+ /* Mark store as RW */
+ if (!s->readonly) {
+ s->flags |= (1 << MTP_FLAG_WRITABLE);
+ }
if (s->desc && s->desc[0]) {
s->desc = g_strdup(s->desc + 1);
} else {
@@ -1567,6 +1689,7 @@ static const VMStateDescription vmstate_usb_mtp = {
static Property mtp_properties[] = {
DEFINE_PROP_STRING("x-root", MTPState, root),
DEFINE_PROP_STRING("desc", MTPState, desc),
+ DEFINE_PROP_BOOL("readonly", MTPState, readonly, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
` (2 preceding siblings ...)
2018-02-20 15:28 ` [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects Gerd Hoffmann
@ 2018-02-20 15:28 ` Gerd Hoffmann
2018-02-20 20:47 ` Eric Blake
2018-02-20 15:28 ` [Qemu-devel] [PULL 5/5] usb-mtp: Advertise SendObjectInfo for write support Gerd Hoffmann
2018-02-20 17:03 ` [Qemu-devel] [PULL 0/5] Usb 20180220 patches Peter Maydell
5 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-02-20 15:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Bandan Das, Gerd Hoffmann
From: Bandan Das <bsd@redhat.com>
Allow write operations on behalf of the initiator. The
precursor to write is the sending of the write metadata
that consists of the ObjectInfo dataset. This patch introduces
a flag that is set when the responder is ready to receive
write data based on a previous SendObjectInfo operation by
the initiator (The SendObjectInfo implementation is in a
later patch)
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180215231129.14710-5-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-mtp.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 157 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 5ef77f3e9f..4f369e1277 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -47,6 +47,7 @@ enum mtp_code {
CMD_GET_OBJECT_INFO = 0x1008,
CMD_GET_OBJECT = 0x1009,
CMD_DELETE_OBJECT = 0x100b,
+ CMD_SEND_OBJECT = 0x100d,
CMD_GET_PARTIAL_OBJECT = 0x101b,
CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
CMD_GET_OBJECT_PROP_DESC = 0x9802,
@@ -63,9 +64,11 @@ enum mtp_code {
RES_INVALID_STORAGE_ID = 0x2008,
RES_INVALID_OBJECT_HANDLE = 0x2009,
RES_INVALID_OBJECT_FORMAT_CODE = 0x200b,
+ RES_STORE_FULL = 0x200c,
RES_STORE_READ_ONLY = 0x200e,
RES_PARTIAL_DELETE = 0x2012,
RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
+ RES_INVALID_OBJECTINFO = 0x2015,
RES_INVALID_PARENT_OBJECT = 0x201a,
RES_INVALID_PARAMETER = 0x201d,
RES_SESSION_ALREADY_OPEN = 0x201e,
@@ -183,6 +186,14 @@ struct MTPState {
int inotifyfd;
QTAILQ_HEAD(events, MTPMonEntry) events;
#endif
+ /* Responder is expecting a write operation */
+ bool write_pending;
+ struct {
+ uint32_t parent_handle;
+ uint16_t format;
+ uint32_t size;
+ char *filename;
+ } dataset;
};
#define TYPE_USB_MTP "usb-mtp"
@@ -804,6 +815,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c)
CMD_GET_OBJECT_HANDLES,
CMD_GET_OBJECT_INFO,
CMD_DELETE_OBJECT,
+ CMD_SEND_OBJECT,
CMD_GET_OBJECT,
CMD_GET_PARTIAL_OBJECT,
CMD_GET_OBJECT_PROPS_SUPPORTED,
@@ -1378,6 +1390,14 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
nres = 1;
res0 = data_in->length;
break;
+ case CMD_SEND_OBJECT:
+ if (!s->write_pending) {
+ usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO,
+ c->trans, 0, 0, 0, 0);
+ return;
+ }
+ s->data_out = usb_mtp_data_alloc(c);
+ return;
case CMD_GET_OBJECT_PROPS_SUPPORTED:
if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
c->argv[0] != FMT_ASSOCIATION) {
@@ -1472,12 +1492,133 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
fprintf(stderr, "%s\n", __func__);
}
+mode_t getumask(void)
+{
+ mode_t mask = umask(0);
+ umask(mask);
+ return mask;
+}
+
+static void usb_mtp_write_data(MTPState *s)
+{
+ MTPData *d = s->data_out;
+ MTPObject *parent =
+ usb_mtp_object_lookup(s, s->dataset.parent_handle);
+ char *path = NULL;
+ int rc = -1;
+ mode_t mask = ~getumask() & 0666;
+
+ assert(d != NULL);
+
+ if (parent == NULL || !s->write_pending) {
+ usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO, d->trans,
+ 0, 0, 0, 0);
+ return;
+ }
+
+ if (s->dataset.filename) {
+ path = g_strdup_printf("%s/%s", parent->path, s->dataset.filename);
+ if (s->dataset.format == FMT_ASSOCIATION) {
+ d->fd = mkdir(path, mask);
+ goto free;
+ }
+ if (s->dataset.size < d->length) {
+ usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+ goto done;
+ }
+ d->fd = open(path, O_CREAT | O_WRONLY, mask);
+ if (d->fd == -1) {
+ usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+ goto done;
+ }
+
+ /*
+ * Return success if initiator sent 0 sized data
+ */
+ if (!s->dataset.size) {
+ goto success;
+ }
+
+ rc = write(d->fd, d->data, s->dataset.size);
+ if (rc == -1) {
+ usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+ goto done;
+ }
+ if (rc != s->dataset.size) {
+ usb_mtp_queue_result(s, RES_INCOMPLETE_TRANSFER, d->trans,
+ 0, 0, 0, 0);
+ goto done;
+ }
+ }
+
+success:
+ usb_mtp_queue_result(s, RES_OK, d->trans,
+ 0, 0, 0, 0);
+
+done:
+ /*
+ * The write dataset is kept around and freed only
+ * on success or if another write request comes in
+ */
+ if (d->fd != -1) {
+ close(d->fd);
+ }
+free:
+ g_free(s->dataset.filename);
+ g_free(path);
+ s->write_pending = false;
+}
+
+static void usb_mtp_get_data(MTPState *s, mtp_container *container,
+ USBPacket *p)
+{
+ MTPData *d = s->data_out;
+ uint64_t dlen;
+ uint32_t data_len = p->iov.size;
+
+ if (d->first) {
+ /* Total length of incoming data */
+ d->length = cpu_to_le32(container->length) - sizeof(mtp_container);
+ /* Length of data in this packet */
+ data_len -= sizeof(mtp_container);
+ usb_mtp_realloc(d, d->length);
+ d->offset = 0;
+ d->first = false;
+ }
+
+ if (d->length - d->offset > data_len) {
+ dlen = data_len;
+ } else {
+ dlen = d->length - d->offset;
+ }
+
+ switch (d->code) {
+ case CMD_SEND_OBJECT:
+ usb_packet_copy(p, d->data + d->offset, dlen);
+ d->offset += dlen;
+ if (d->offset == d->length) {
+ usb_mtp_write_data(s);
+ usb_mtp_data_free(s->data_out);
+ s->data_out = NULL;
+ return;
+ }
+ break;
+ default:
+ p->status = USB_RET_STALL;
+ return;
+ }
+}
+
static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
{
MTPState *s = USB_MTP(dev);
MTPControl cmd;
mtp_container container;
uint32_t params[5];
+ uint16_t container_type;
int i, rc;
switch (p->ep->nr) {
@@ -1567,8 +1708,13 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
p->status = USB_RET_STALL;
return;
}
- usb_packet_copy(p, &container, sizeof(container));
- switch (le16_to_cpu(container.type)) {
+ if (s->data_out && !s->data_out->first) {
+ container_type = TYPE_DATA;
+ } else {
+ usb_packet_copy(p, &container, sizeof(container));
+ container_type = le16_to_cpu(container.type);
+ }
+ switch (container_type) {
case TYPE_COMMAND:
if (s->data_in || s->data_out || s->result) {
trace_usb_mtp_stall(s->dev.addr, "transaction inflight");
@@ -1599,6 +1745,15 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
(cmd.argc > 4) ? cmd.argv[4] : 0);
usb_mtp_command(s, &cmd);
break;
+ case TYPE_DATA:
+ /* One of the previous transfers has already errored but the
+ * responder is still sending data associated with it
+ */
+ if (s->result != NULL) {
+ return;
+ }
+ usb_mtp_get_data(s, &container, p);
+ break;
default:
/* not needed as long as the mtp device is read-only */
p->status = USB_RET_STALL;
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects
2018-02-20 15:28 ` [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects Gerd Hoffmann
@ 2018-02-20 20:47 ` Eric Blake
2018-02-20 21:27 ` Bandan Das
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2018-02-20 20:47 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Bandan Das
On 02/20/2018 09:28 AM, Gerd Hoffmann wrote:
> From: Bandan Das <bsd@redhat.com>
>
> Allow write operations on behalf of the initiator. The
> precursor to write is the sending of the write metadata
> that consists of the ObjectInfo dataset. This patch introduces
> a flag that is set when the responder is ready to receive
> write data based on a previous SendObjectInfo operation by
> the initiator (The SendObjectInfo implementation is in a
> later patch)
>
> Signed-off-by: Bandan Das <bsd@redhat.com>
> Message-id: 20180215231129.14710-5-bsd@redhat.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/usb/dev-mtp.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 157 insertions(+), 2 deletions(-)
>
> @@ -1472,12 +1492,133 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
> fprintf(stderr, "%s\n", __func__);
> }
>
> +mode_t getumask(void)
> +{
> + mode_t mask = umask(0);
> + umask(mask);
> + return mask;
> +}
This is dangerous. 'man getumask' on my Fedora machine states:
CONFORMING TO
This is a vaporware GNU extension.
NOTES
This function is documented in the glibc manual, but, as at
glibc ver‐
sion 2.24, it is not implemented on Linux. (See umask(2) for a
thread-
safe method of discovering a process's umask.)
and 'man 2 umask' concurs:
It is impossible to use umask() to fetch a process's umask
without at
the same time changing it. A second call to umask() would
then be
needed to restore the umask. The nonatomicity of these two
steps pro‐
vides the potential for races in multithreaded programs.
It is ONLY safe to grab umask() prior to spawning threads, cache that
value, and refer to the cache at all later points.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects
2018-02-20 20:47 ` Eric Blake
@ 2018-02-20 21:27 ` Bandan Das
0 siblings, 0 replies; 9+ messages in thread
From: Bandan Das @ 2018-02-20 21:27 UTC (permalink / raw)
To: Eric Blake; +Cc: Gerd Hoffmann, qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On 02/20/2018 09:28 AM, Gerd Hoffmann wrote:
>> From: Bandan Das <bsd@redhat.com>
>>
>> Allow write operations on behalf of the initiator. The
>> precursor to write is the sending of the write metadata
>> that consists of the ObjectInfo dataset. This patch introduces
>> a flag that is set when the responder is ready to receive
>> write data based on a previous SendObjectInfo operation by
>> the initiator (The SendObjectInfo implementation is in a
>> later patch)
>>
>> Signed-off-by: Bandan Das <bsd@redhat.com>
>> Message-id: 20180215231129.14710-5-bsd@redhat.com
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> hw/usb/dev-mtp.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 157 insertions(+), 2 deletions(-)
>>
>
>> @@ -1472,12 +1492,133 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
>> fprintf(stderr, "%s\n", __func__);
>> }
>> +mode_t getumask(void)
>> +{
>> + mode_t mask = umask(0);
>> + umask(mask);
>> + return mask;
>> +}
>
> This is dangerous. 'man getumask' on my Fedora machine states:
Thanks for the pointer, Eric. Indeed, this doesn't look right.
I am inclined to just set the default
permission of created files to 0644 for the mtp share
and not implement a thread safe getumask.
Bandan
> CONFORMING TO
> This is a vaporware GNU extension.
>
> NOTES
> This function is documented in the glibc manual, but, as at
> glibc ver‐
> sion 2.24, it is not implemented on Linux. (See umask(2) for a
> thread-
> safe method of discovering a process's umask.)
>
>
> and 'man 2 umask' concurs:
>
> It is impossible to use umask() to fetch a process's umask
> without at
> the same time changing it. A second call to umask() would
> then be
> needed to restore the umask. The nonatomicity of these two
> steps pro‐
> vides the potential for races in multithreaded programs.
>
> It is ONLY safe to grab umask() prior to spawning threads, cache that
> value, and refer to the cache at all later points.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 5/5] usb-mtp: Advertise SendObjectInfo for write support
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
` (3 preceding siblings ...)
2018-02-20 15:28 ` [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects Gerd Hoffmann
@ 2018-02-20 15:28 ` Gerd Hoffmann
2018-02-20 17:03 ` [Qemu-devel] [PULL 0/5] Usb 20180220 patches Peter Maydell
5 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-02-20 15:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Bandan Das, Gerd Hoffmann
From: Bandan Das <bsd@redhat.com>
This patch implements a dummy ObjectInfo structure so that
it's easy to typecast the incoming data. If the metadata is
valid, write_pending is set. Also, the incoming filename
is utf-16, so, instead of depending on external libraries, just
implement a simple function to get the filename
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180215231129.14710-6-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-mtp.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 129 insertions(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 4f369e1277..e015bbe771 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -47,6 +47,7 @@ enum mtp_code {
CMD_GET_OBJECT_INFO = 0x1008,
CMD_GET_OBJECT = 0x1009,
CMD_DELETE_OBJECT = 0x100b,
+ CMD_SEND_OBJECT_INFO = 0x100c,
CMD_SEND_OBJECT = 0x100d,
CMD_GET_PARTIAL_OBJECT = 0x101b,
CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
@@ -67,8 +68,10 @@ enum mtp_code {
RES_STORE_FULL = 0x200c,
RES_STORE_READ_ONLY = 0x200e,
RES_PARTIAL_DELETE = 0x2012,
+ RES_STORE_NOT_AVAILABLE = 0x2013,
RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
RES_INVALID_OBJECTINFO = 0x2015,
+ RES_DESTINATION_UNSUPPORTED = 0x2020,
RES_INVALID_PARENT_OBJECT = 0x201a,
RES_INVALID_PARAMETER = 0x201d,
RES_SESSION_ALREADY_OPEN = 0x201e,
@@ -196,6 +199,34 @@ struct MTPState {
} dataset;
};
+/*
+ * ObjectInfo dataset received from initiator
+ * Fields we don't care about are ignored
+ */
+typedef struct {
+ uint32_t storage_id; /*unused*/
+ uint16_t format;
+ uint16_t protection_status; /*unused*/
+ uint32_t size;
+ uint16_t thumb_format; /*unused*/
+ uint32_t thumb_comp_sz; /*unused*/
+ uint32_t thumb_pix_width; /*unused*/
+ uint32_t thumb_pix_height; /*unused*/
+ uint32_t image_pix_width; /*unused*/
+ uint32_t image_pix_height; /*unused*/
+ uint32_t image_bit_depth; /*unused*/
+ uint32_t parent; /*unused*/
+ uint16_t assoc_type;
+ uint32_t assoc_desc;
+ uint32_t seq_no; /*unused*/
+ uint8_t length; /*part of filename field*/
+ uint16_t filename[0];
+ char date_created[0]; /*unused*/
+ char date_modified[0]; /*unused*/
+ char keywords[0]; /*unused*/
+ /* string and other data follows */
+} QEMU_PACKED ObjectInfo;
+
#define TYPE_USB_MTP "usb-mtp"
#define USB_MTP(obj) OBJECT_CHECK(MTPState, (obj), TYPE_USB_MTP)
@@ -815,6 +846,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c)
CMD_GET_OBJECT_HANDLES,
CMD_GET_OBJECT_INFO,
CMD_DELETE_OBJECT,
+ CMD_SEND_OBJECT_INFO,
CMD_SEND_OBJECT,
CMD_GET_OBJECT,
CMD_GET_PARTIAL_OBJECT,
@@ -1243,7 +1275,7 @@ static void usb_mtp_object_delete(MTPState *s, uint32_t handle,
static void usb_mtp_command(MTPState *s, MTPControl *c)
{
MTPData *data_in = NULL;
- MTPObject *o;
+ MTPObject *o = NULL;
uint32_t nres = 0, res0 = 0;
/* sanity checks */
@@ -1390,6 +1422,37 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
nres = 1;
res0 = data_in->length;
break;
+ case CMD_SEND_OBJECT_INFO:
+ /* First parameter points to storage id or is 0 */
+ if (c->argv[0] && (c->argv[0] != QEMU_STORAGE_ID)) {
+ usb_mtp_queue_result(s, RES_STORE_NOT_AVAILABLE, c->trans,
+ 0, 0, 0, 0);
+ } else if (c->argv[1] && !c->argv[0]) {
+ /* If second parameter is specified, first must also be specified */
+ usb_mtp_queue_result(s, RES_DESTINATION_UNSUPPORTED, c->trans,
+ 0, 0, 0, 0);
+ } else {
+ uint32_t handle = c->argv[1];
+ if (handle == 0xFFFFFFFF || handle == 0) {
+ /* root object */
+ o = QTAILQ_FIRST(&s->objects);
+ } else {
+ o = usb_mtp_object_lookup(s, handle);
+ }
+ if (o == NULL) {
+ usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE, c->trans,
+ 0, 0, 0, 0);
+ }
+ if (o->format != FMT_ASSOCIATION) {
+ usb_mtp_queue_result(s, RES_INVALID_PARENT_OBJECT, c->trans,
+ 0, 0, 0, 0);
+ }
+ }
+ if (o) {
+ s->dataset.parent_handle = o->handle;
+ }
+ s->data_out = usb_mtp_data_alloc(c);
+ return;
case CMD_SEND_OBJECT:
if (!s->write_pending) {
usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO,
@@ -1499,6 +1562,19 @@ mode_t getumask(void)
return mask;
}
+static void utf16_to_str(uint8_t len, uint16_t *arr, char *name)
+{
+ int count;
+ wchar_t *wstr = g_new0(wchar_t, len);
+
+ for (count = 0; count < len; count++) {
+ wstr[count] = (wchar_t)arr[count];
+ }
+
+ wcstombs(name, wstr, len);
+ g_free(wstr);
+}
+
static void usb_mtp_write_data(MTPState *s)
{
MTPData *d = s->data_out;
@@ -1572,6 +1648,45 @@ free:
s->write_pending = false;
}
+static void usb_mtp_write_metadata(MTPState *s)
+{
+ MTPData *d = s->data_out;
+ ObjectInfo *dataset = (ObjectInfo *)d->data;
+ char *filename = g_new0(char, dataset->length);
+ MTPObject *o;
+ MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
+ uint32_t next_handle = s->next_handle;
+
+ assert(!s->write_pending);
+
+ utf16_to_str(dataset->length, dataset->filename, filename);
+
+ o = usb_mtp_object_lookup_name(p, filename, dataset->length);
+ if (o != NULL) {
+ next_handle = o->handle;
+ }
+
+ s->dataset.filename = filename;
+ s->dataset.format = dataset->format;
+ s->dataset.size = dataset->size;
+ s->dataset.filename = filename;
+ s->write_pending = true;
+
+ if (s->dataset.format == FMT_ASSOCIATION) {
+ usb_mtp_write_data(s);
+ /* next_handle will be allocated to the newly created dir */
+ if (d->fd == -1) {
+ usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+ return;
+ }
+ d->fd = -1;
+ }
+
+ usb_mtp_queue_result(s, RES_OK, d->trans, 3, QEMU_STORAGE_ID,
+ s->dataset.parent_handle, next_handle);
+}
+
static void usb_mtp_get_data(MTPState *s, mtp_container *container,
USBPacket *p)
{
@@ -1596,6 +1711,19 @@ static void usb_mtp_get_data(MTPState *s, mtp_container *container,
}
switch (d->code) {
+ case CMD_SEND_OBJECT_INFO:
+ usb_packet_copy(p, d->data + d->offset, dlen);
+ d->offset += dlen;
+ if (d->offset == d->length) {
+ /* The operation might have already failed */
+ if (!s->result) {
+ usb_mtp_write_metadata(s);
+ }
+ usb_mtp_data_free(s->data_out);
+ s->data_out = NULL;
+ return;
+ }
+ break;
case CMD_SEND_OBJECT:
usb_packet_copy(p, d->data + d->offset, dlen);
d->offset += dlen;
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 0/5] Usb 20180220 patches
2018-02-20 15:28 [Qemu-devel] [PULL 0/5] Usb 20180220 patches Gerd Hoffmann
` (4 preceding siblings ...)
2018-02-20 15:28 ` [Qemu-devel] [PULL 5/5] usb-mtp: Advertise SendObjectInfo for write support Gerd Hoffmann
@ 2018-02-20 17:03 ` Peter Maydell
5 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2018-02-20 17:03 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers, Bandan Das
On 20 February 2018 at 15:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit b487e2b2ff6e9fff8a45a9600d6de3a7bf82666a:
>
> Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging (2018-02-20 11:52:24 +0000)
>
> are available in the git repository at:
>
> git://git.kraxel.org/qemu tags/usb-20180220-pull-request
>
> for you to fetch changes up to 18c7310e9982b71d17e76648acfec0be42a57aea:
>
> usb-mtp: Advertise SendObjectInfo for write support (2018-02-20 16:06:56 +0100)
>
> ----------------------------------------------------------------
> usb: Add write support to MTP.
>
> ----------------------------------------------------------------
>
> Bandan Das (5):
> usb-mtp: Add one more argument when building results
> usb-mtp: print parent path in IN_IGNORED trace fn
> usb-mtp: Support delete of mtp objects
> usb-mtp: Introduce write support for MTP objects
> usb-mtp: Advertise SendObjectInfo for write support
Hi. This fails to build on the BSDs, I'm afraid.
FreeBSD:
CC hw/usb/dev-mtp.o
/root/qemu/hw/usb/dev-mtp.c:1558:8: warning: no previous prototype for
function 'getumask' [-Wmissing-prototypes]
mode_t getumask(void)
^
/root/qemu/hw/usb/dev-mtp.c:1664:9: warning: implicit declaration of
function 'usb_mtp_object_lookup_name' is invalid in C99
[-Wimplicit-function-declaration]
o = usb_mtp_object_lookup_name(p, filename, dataset->length);
^
/root/qemu/hw/usb/dev-mtp.c:1664:7: warning: incompatible integer to
pointer conversion assigning to 'MTPObject *' (aka 'struct MTPObject
*') from 'int' [-Wint-conversion]
o = usb_mtp_object_lookup_name(p, filename, dataset->length);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
and then all the link steps fail with:
../hw/usb/dev-mtp.o: In function `usb_mtp_write_metadata':
/root/qemu/hw/usb/dev-mtp.c:1664: undefined reference to
`usb_mtp_object_lookup_name'
NetBSD:
CC hw/usb/dev-mtp.o
/root/qemu/hw/usb/dev-mtp.c:1558:8: warning: no previous prototype for
'getumask' [-Wmissing-prototypes]
mode_t getumask(void)
^
/root/qemu/hw/usb/dev-mtp.c: In function 'usb_mtp_write_metadata':
/root/qemu/hw/usb/dev-mtp.c:1664:5: warning: implicit declaration of
function 'usb_mtp_object_lookup_name'
[-Wimplicit-function-declaration]
o = usb_mtp_object_lookup_name(p, filename, dataset->length);
^
/root/qemu/hw/usb/dev-mtp.c:1664:5: warning: nested extern declaration
of 'usb_mtp_object_lookup_name' [-Wnested-externs]
/root/qemu/hw/usb/dev-mtp.c:1664:7: warning: assignment makes pointer
from integer without a cast [enabled by default]
o = usb_mtp_object_lookup_name(p, filename, dataset->length);
^
OpenBSD and OSX fail similarly.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread