This is a "pre-patch" to breaking up the write buffer for
MTP writes. Instead of allocating a mtp buffer equal to size
sent by the initiator, we start with a small size and reallocate
multiples (of that small size) as needed.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/usb/dev-mtp.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 68c5eb8eaa..2f3536a26d 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -152,7 +152,6 @@ struct MTPData {
     bool         first;
     /* Used for >4G file sizes */
     bool         pending;
-    uint64_t     cached_length;
     int          fd;
 };
 
@@ -244,6 +243,7 @@ typedef struct {
 
 #define MTP_MANUFACTURER  "QEMU"
 #define MTP_PRODUCT       "QEMU filesharing"
+#define MTP_WRITE_BUF_SZ  512000
 
 enum {
     STR_MANUFACTURER = 1,
@@ -1658,7 +1658,7 @@ static void usb_mtp_write_data(MTPState *s)
             d->fd = mkdir(path, mask);
             goto free;
         }
-        if ((s->dataset.size != 0xFFFFFFFF) && (s->dataset.size < d->length)) {
+        if ((s->dataset.size != 0xFFFFFFFF) && (s->dataset.size != d->offset)) {
             usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
                                  0, 0, 0, 0);
             goto done;
@@ -1776,17 +1776,21 @@ static void usb_mtp_get_data(MTPState *s, mtp_container *container,
         total_len = cpu_to_le32(container->length) - sizeof(mtp_container);
         /* Length of data in this packet */
         data_len -= sizeof(mtp_container);
-        usb_mtp_realloc(d, total_len);
-        d->length += total_len;
+        if (total_len < MTP_WRITE_BUF_SZ) {
+                usb_mtp_realloc(d, total_len);
+                d->length += total_len;
+        } else {
+                usb_mtp_realloc(d, MTP_WRITE_BUF_SZ - sizeof(mtp_container));
+                d->length += MTP_WRITE_BUF_SZ - sizeof(mtp_container);
+        }
         d->offset = 0;
-        d->cached_length = total_len;
         d->first = false;
         d->pending = false;
     }
 
     if (d->pending) {
-        usb_mtp_realloc(d, d->cached_length);
-        d->length += d->cached_length;
+        usb_mtp_realloc(d, MTP_WRITE_BUF_SZ);
+        d->length += MTP_WRITE_BUF_SZ;
         d->pending = false;
     }
 
@@ -1794,12 +1798,6 @@ static void usb_mtp_get_data(MTPState *s, mtp_container *container,
         dlen = data_len;
     } else {
         dlen = d->length - d->offset;
-        /* Check for cached data for large files */
-        if ((s->dataset.size == 0xFFFFFFFF) && (dlen < p->iov.size)) {
-            usb_mtp_realloc(d, p->iov.size - dlen);
-            d->length += p->iov.size - dlen;
-            dlen = p->iov.size;
-        }
     }
 
     switch (d->code) {
@@ -1821,7 +1819,7 @@ static void usb_mtp_get_data(MTPState *s, mtp_container *container,
         d->offset += dlen;
         if ((p->iov.size % 64) || !p->iov.size) {
             assert((s->dataset.size == 0xFFFFFFFF) ||
-                   (s->dataset.size == d->length));
+                   (s->dataset.size == d->offset));
 
             usb_mtp_write_data(s);
             usb_mtp_data_free(s->data_out);
-- 
2.19.2

On 1/28/19 8:24 AM, Bandan Das wrote:
> This is a "pre-patch" to breaking up the write buffer for
> MTP writes. Instead of allocating a mtp buffer equal to size
> sent by the initiator, we start with a small size and reallocate
> multiples (of that small size) as needed.
> 
> Signed-off-by: Bandan Das <bsd@redhat.com>
> ---
>  hw/usb/dev-mtp.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 68c5eb8eaa..2f3536a26d 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -152,7 +152,6 @@ struct MTPData {
>      bool         first;
>      /* Used for >4G file sizes */
>      bool         pending;
> -    uint64_t     cached_length;
>      int          fd;
>  };
>  
> @@ -244,6 +243,7 @@ typedef struct {
>  
>  #define MTP_MANUFACTURER  "QEMU"
>  #define MTP_PRODUCT       "QEMU filesharing"
> +#define MTP_WRITE_BUF_SZ  512000

Why a non-power-of-2 instead of using units.h and writing (512 * KiB)?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Eric Blake <eblake@redhat.com> writes:

> On 1/28/19 8:24 AM, Bandan Das wrote:
>> This is a "pre-patch" to breaking up the write buffer for
>> MTP writes. Instead of allocating a mtp buffer equal to size
>> sent by the initiator, we start with a small size and reallocate
>> multiples (of that small size) as needed.
>> 
>> Signed-off-by: Bandan Das <bsd@redhat.com>
>> ---
>>  hw/usb/dev-mtp.c | 26 ++++++++++++--------------
>>  1 file changed, 12 insertions(+), 14 deletions(-)
>> 
>> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
>> index 68c5eb8eaa..2f3536a26d 100644
>> --- a/hw/usb/dev-mtp.c
>> +++ b/hw/usb/dev-mtp.c
>> @@ -152,7 +152,6 @@ struct MTPData {
>>      bool         first;
>>      /* Used for >4G file sizes */
>>      bool         pending;
>> -    uint64_t     cached_length;
>>      int          fd;
>>  };
>>  
>> @@ -244,6 +243,7 @@ typedef struct {
>>  
>>  #define MTP_MANUFACTURER  "QEMU"
>>  #define MTP_PRODUCT       "QEMU filesharing"
>> +#define MTP_WRITE_BUF_SZ  512000
>
> Why a non-power-of-2 instead of using units.h and writing (512 * KiB)?

Thank you for the reminder, Eric. I posted a v4.