From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59469) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSVdk-0006QO-E0 for qemu-devel@nongnu.org; Wed, 27 Jul 2016 16:41:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSVdg-0005Op-CH for qemu-devel@nongnu.org; Wed, 27 Jul 2016 16:41:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSVdg-0005Oh-3x for qemu-devel@nongnu.org; Wed, 27 Jul 2016 16:41:12 -0400 From: Bandan Das References: <1b3ed86694a85165388a5efcd351cfaa19b59ad4.1468876280.git.109lozanoi@gmail.com> Date: Wed, 27 Jul 2016 16:41:10 -0400 In-Reply-To: <1b3ed86694a85165388a5efcd351cfaa19b59ad4.1468876280.git.109lozanoi@gmail.com> (Isaac Lozano's message of "Thu, 21 Jul 2016 02:56:34 -0700") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2 2/2] usb-mtp: added object properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaac Lozano <109lozanoi@gmail.com> Cc: qemu-devel@nongnu.org, kraxel@redhat.com Isaac Lozano <109lozanoi@gmail.com> writes: > Windows uses object properties to determine the size of a file, so to > add object properties, we must also add a minimum set of new commands > and object properties. Most object properties are data that we already > have, except for the unique persistant object identifier. Windows > doesn't use this property, it seems, so we can cheat a bit and just use > the object handle for it. > > Signed-off-by: Isaac Lozano <109lozanoi@gmail.com> > --- > hw/usb/dev-mtp.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 181 insertions(+), 5 deletions(-) > > diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c > index 6f6a270..fa0c81e 100644 > --- a/hw/usb/dev-mtp.c > +++ b/hw/usb/dev-mtp.c > @@ -48,6 +48,9 @@ enum mtp_code { > CMD_GET_OBJECT_INFO = 0x1008, > CMD_GET_OBJECT = 0x1009, > CMD_GET_PARTIAL_OBJECT = 0x101b, > + CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801, > + CMD_GET_OBJECT_PROP_DESC = 0x9802, > + CMD_GET_OBJECT_PROP_VALUE = 0x9803, > > /* response codes */ > RES_OK = 0x2001, > @@ -59,10 +62,12 @@ enum mtp_code { > RES_INCOMPLETE_TRANSFER = 0x2007, > RES_INVALID_STORAGE_ID = 0x2008, > RES_INVALID_OBJECT_HANDLE = 0x2009, > + RES_INVALID_OBJECT_FORMAT_CODE = 0x200b, > RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014, > RES_INVALID_PARENT_OBJECT = 0x201a, > RES_INVALID_PARAMETER = 0x201d, > RES_SESSION_ALREADY_OPEN = 0x201e, > + RES_INVALID_OBJECT_PROP_CODE = 0xA801, > > /* format codes */ > FMT_UNDEFINED_OBJECT = 0x3000, > @@ -72,6 +77,22 @@ enum mtp_code { > EVT_OBJ_ADDED = 0x4002, > EVT_OBJ_REMOVED = 0x4003, > EVT_OBJ_INFO_CHANGED = 0x4007, > + > + /* object properties */ > + PROP_STORAGE_ID = 0xDC01, > + PROP_OBJECT_FORMAT = 0xDC02, > + PROP_OBJECT_COMPRESSED_SIZE = 0xDC04, > + PROP_PARENT_OBJECT = 0xDC0B, > + PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER = 0xDC41, > + PROP_NAME = 0xDC44, > +}; > + > +enum mtp_data_type { > + DATA_TYPE_UINT16 = 0x0004, > + DATA_TYPE_UINT32 = 0x0006, > + DATA_TYPE_UINT64 = 0x0008, > + DATA_TYPE_UINT128 = 0x000a, > + DATA_TYPE_STRING = 0xffff, > }; > > typedef struct { > @@ -778,6 +799,9 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, MTPControl *c) > CMD_GET_OBJECT_INFO, > CMD_GET_OBJECT, > CMD_GET_PARTIAL_OBJECT, > + CMD_GET_OBJECT_PROPS_SUPPORTED, > + CMD_GET_OBJECT_PROP_DESC, > + CMD_GET_OBJECT_PROP_VALUE, > }; > static const uint16_t fmt[] = { > FMT_UNDEFINED_OBJECT, > @@ -886,8 +910,7 @@ static MTPData *usb_mtp_get_object_info(MTPState *s, MTPControl *c, > > if (o->stat.st_size > 0xFFFFFFFF) { > usb_mtp_add_u32(d, 0xFFFFFFFF); > - } > - else { > + } else { > usb_mtp_add_u32(d, o->stat.st_size); > } > > @@ -972,6 +995,122 @@ static MTPData *usb_mtp_get_partial_object(MTPState *s, MTPControl *c, > return d; > } > > +static MTPData *usb_mtp_get_object_props_supported(MTPState *s, MTPControl *c) > +{ > + static const uint16_t props[] = { > + PROP_STORAGE_ID, > + PROP_OBJECT_FORMAT, > + PROP_OBJECT_COMPRESSED_SIZE, > + PROP_PARENT_OBJECT, > + PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER, > + PROP_NAME, > + }; > + MTPData *d = usb_mtp_data_alloc(c); > + usb_mtp_add_u16_array(d, ARRAY_SIZE(props), props); > + > + return d; > +} > + > +static MTPData *usb_mtp_get_object_prop_desc(MTPState *s, MTPControl *c) > +{ > + MTPData *d = usb_mtp_data_alloc(c); > + switch (c->argv[0]) { > + case PROP_STORAGE_ID: > + usb_mtp_add_u16(d, PROP_STORAGE_ID); > + usb_mtp_add_u16(d, DATA_TYPE_UINT32); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; Stylistic: Can rewrite all 0 valued calls to usb_mtp_add_u32(d, 0) and the like but this is fine too :) > + case PROP_OBJECT_FORMAT: > + usb_mtp_add_u16(d, PROP_OBJECT_FORMAT); > + usb_mtp_add_u16(d, DATA_TYPE_UINT16); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u16(d, 0x0000); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; > + case PROP_OBJECT_COMPRESSED_SIZE: > + usb_mtp_add_u16(d, PROP_OBJECT_COMPRESSED_SIZE); > + usb_mtp_add_u16(d, DATA_TYPE_UINT64); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u64(d, 0x0000000000000000); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; > + case PROP_PARENT_OBJECT: > + usb_mtp_add_u16(d, PROP_PARENT_OBJECT); > + usb_mtp_add_u16(d, DATA_TYPE_UINT32); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; > + case PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER: > + usb_mtp_add_u16(d, PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER); > + usb_mtp_add_u16(d, DATA_TYPE_UINT128); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u64(d, 0x0000000000000000); > + usb_mtp_add_u64(d, 0x0000000000000000); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; > + case PROP_NAME: > + usb_mtp_add_u16(d, PROP_NAME); > + usb_mtp_add_u16(d, DATA_TYPE_STRING); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u8(d, 0x00); > + usb_mtp_add_u32(d, 0x00000000); > + usb_mtp_add_u8(d, 0x00); > + break; > + default: > + usb_mtp_data_free(d); > + return NULL; > + } > + > + return d; > +} > + > +static MTPData *usb_mtp_get_object_prop_value(MTPState *s, MTPControl *c, > + MTPObject *o) > +{ > + MTPData *d = usb_mtp_data_alloc(c); > + switch (c->argv[1]) { > + case PROP_STORAGE_ID: > + usb_mtp_add_u32(d, QEMU_STORAGE_ID); > + break; > + case PROP_OBJECT_FORMAT: > + usb_mtp_add_u16(d, o->format); > + break; > + case PROP_OBJECT_COMPRESSED_SIZE: > + usb_mtp_add_u64(d, o->stat.st_size); > + break; > + case PROP_PARENT_OBJECT: > + if (o->parent == NULL) { > + usb_mtp_add_u32(d, 0x00000000); > + } else { > + usb_mtp_add_u32(d, o->parent->handle); > + } > + break; > + case PROP_PERSISTENT_UNIQUE_OBJECT_IDENTIFIER: > + /* Should be persistant between sessions, > + * but using our objedt ID is "good enough" s/objedt/object > + * for now */ > + usb_mtp_add_u64(d, 0x0000000000000000); > + usb_mtp_add_u64(d, o->handle); I thought we decided on the inode number ? Probably harmless but can't be handle be different across sessions ? For example, a new file is added followed by a new session. Bandan > + break; > + case PROP_NAME: > + usb_mtp_add_str(d, o->name); > + break; > + default: > + usb_mtp_data_free(d); > + return NULL; > + } > + > + return d; > +} > + > static void usb_mtp_command(MTPState *s, MTPControl *c) > { > MTPData *data_in = NULL; > @@ -1119,6 +1258,43 @@ static void usb_mtp_command(MTPState *s, MTPControl *c) > nres = 1; > res0 = data_in->length; > break; > + case CMD_GET_OBJECT_PROPS_SUPPORTED: > + 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); > + return; > + } > + data_in = usb_mtp_get_object_props_supported(s, c); > + break; > + case CMD_GET_OBJECT_PROP_DESC: > + 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); > + 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); > + return; > + } > + break; > + case CMD_GET_OBJECT_PROP_VALUE: > + 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); > + 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); > + return; > + } > + break; > default: > trace_usb_mtp_op_unknown(s->dev.addr, c->code); > usb_mtp_queue_result(s, RES_OPERATION_NOT_SUPPORTED, > @@ -1204,9 +1380,9 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p) > trace_usb_mtp_data_in(s->dev.addr, d->trans, d->length); > if (d->length + sizeof(container) > 0xFFFFFFFF) { > container.length = cpu_to_le32(0xFFFFFFFF); > - } > - else { > - container.length = cpu_to_le32(d->length + sizeof(container)); > + } else { > + container.length = cpu_to_le32(d->length + > + sizeof(container)); > } > container.type = cpu_to_le16(TYPE_DATA); > container.code = cpu_to_le16(d->code);