n absolute path.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 9846e4b513..7c07295519 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -2
gt; +if test "$fuzzing" = "yes" ; then
> + QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize=fuzzer,address
> -fprofile-instr-generate"
> + QEMU_CFLAGS="$QEMU_CFLAGS -fprofile-instr-generate -fcoverage-mapping"
What is the purpose of -fprofile-instr-generate
ename = utf16_to_str(MIN(dataset->length,
> -dlen - offsetof(ObjectInfo, filename)),
> +filename = utf16_to_str(MIN(dataset->length, filename_chars),
> dataset->filename);
>
> if (strchr(filename, '/')) {
Reviewed-by: Bandan Das
Daniel P. Berrangé writes:
>> I can't find usb-mtp sending a "I/O error" on an error condition
>> for the objectinfo phase. It might be libmtp or even the command itself
>> failing for some reason. For incomplete transfer, I just checked, it's
>> spitting out the error message correctly as INCOMP
Daniel P. Berrangé writes:
> On Tue, Apr 16, 2019 at 12:10:16PM -0400, Bandan Das wrote:
>> Daniel P. Berrangé writes:
>> ...
>> >> > The store is read only by default. Are you trying something like:
>> >> > -device usb-mtp,rootdir=/code/mtpshare,
Daniel P. Berrangé writes:
...
>> > The store is read only by default. Are you trying something like:
>> > -device usb-mtp,rootdir=/code/mtpshare,readonly=false ?
>>
>> Ah ha, I didn't realize I had to enable write support explicitly. Will
>> retry with that.
>
> Even after setting readonly=fal
ame, -1);
Nit: Might as well just remove the "-1" argument and unconditionally use
strlen in usb_mtp_object_lookup_name
Bandan
> if (o != NULL) {
> next_handle = o->handle;
> }
Daniel P. Berrangé writes:
> Two previous attempts to fix this due to GCC 9 highlighting
> unaligned data access. My attempt:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07763.html
>
> And a previous one:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07923.html
Forgot to cc people ...
Bandan Das writes:
> Commit c5ead51f90cf (usb-mtp: return incomplete transfer on a lstat
> failure) checks if lstat succeeded when updating attributes of a
> file. However, it also changed behavior to return an error by
> default. This is incorrect because
go and there won't be
an object for it.
Fixes: c5ead51f90cf
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index ebf210fbf8..5de22738ce 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-
There's no functional change but the flow is (hopefully)
more consistent for both file and folder object types.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 57 +---
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/hw/usb/dev-
make it less confusing. Applies on top of
[PATCH v3] usb-mtp: fix return status of delete
Message-ID:
Bandan Das (3):
usb-mtp: fix return status of delete
usb-mtp: remove usb_mtp_object_free_one
usb-mtp: refactor the flow of usb_mtp_write_data
hw/usb/dev-mtp.c | 133
Gerd Hoffmann writes:
> On Thu, Mar 28, 2019 at 01:37:21PM -0400, Bandan Das wrote:
>> This function is used in the delete path only and can
>> be replaced by a call to usb_mtp_object_free.
>>
>> Reviewed-by: Peter Maydell
>> Signed-off-by: Bandan Das
>
>
,
only READ_ONLY can be returned.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 62 ++--
1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 06e376bcd2..91b820baaf 100644
--- a/hw/usb/dev-mtp.c
This function is used in the delete path only and can
be replaced by a call to usb_mtp_object_free.
Reviewed-by: Peter Maydell
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
There's no functional change but the flow is (hopefully)
more consistent for both file and folder object types.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 57 +---
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/hw/usb/dev-
This function is used in the delete path only and can
be replaced by a call to usb_mtp_object_free.
Reviewed-by: Peter Maydell
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
confusing. Applies on top of
[PATCH v3] usb-mtp: fix return status of delete
Message-ID:
Bandan Das (2):
usb-mtp: remove usb_mtp_object_free_one
usb-mtp: refactor the flow of usb_mtp_write_data
hw/usb/dev-mtp.c | 71 ++--
1 file changed, 32 insertions
Peter Maydell writes:
> On Tue, 26 Mar 2019 at 17:58, Bandan Das wrote:
>>
>
...
> Doesn't this mean you're no longer sending the RES_OK result
> for this case ?
>
Ugh, I messed up this version. I will send a v3 and
fix the indentation to
Message-ID:
Bandan Das (2):
usb-mtp: remove usb_mtp_object_free_one
usb-mtp: refactor the flow of usb_mtp_write_data
hw/usb/dev-mtp.c | 67
1 file changed, 28 insertions(+), 39 deletions(-)
--
2.19.2
This function is used in the delete path only and can
be replaced by a call to usb_mtp_object_free.
Reviewed-by: Peter Maydell
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
There's no functional change but the flow is (hopefully)
more consistent for both file and folder object types.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 53
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/hw/usb/dev-
The first patch removes a unnecessary function
and the second is just a code reorg of usb_mtp_write_data
to make it less confusing. Applies on top of
[PATCH v3] usb-mtp: fix return status of delete
Message-ID:
Bandan Das (2):
usb-mtp: remove usb_mtp_object_free_one
usb-mtp: refactor the flow
There's no functional change but the flow is (hopefully)
more consistent for both file and folder object types.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 58 +---
1 file changed, 30 insertions(+), 28 deletions(-)
diff --git a/hw/usb/dev-
This function is used in the delete path only and can
be replaced by a call to usb_mtp_object_free.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 91b820baaf..4dc1317e2e
y sets watches on the directory as a
> whole, not files within, so there is no risk of guest
> triggered wrap around on the low 32 bits.
>
> Signed-off-by: Daniel P. Berrangé
> ---
>
> Depends: <20190314151527.25533-1-berra...@redhat.com>
>
Reviewed-by: Bandan Da
Daniel P. Berrangé writes:
> On Fri, Mar 15, 2019 at 01:24:42PM -0400, Bandan Das wrote:
>> Daniel P. Berrangé writes:
>> ...
>> >> Thanks, this fixes it! I had a related question about the way
>> >> qemu_file_monitor_add_watch works.
>> >>
&
Peter Maydell writes:
...
>> +} else {
>> +usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
>> + 0, 0, 0, 0);
>> +}
>
> Presumably one of these should be RES_OK of some kind ?
>
Ah, yes, that's a typo.
>
Daniel P. Berrangé writes:
...
>> Thanks, this fixes it! I had a related question about the way
>> qemu_file_monitor_add_watch works.
>>
>> Am I correct in understanding that if there is already a watch on a dir,
>> we return back mon->nextid++ i.e the next free id. Why don't we return
>> back th
filemonitor-inotify.c b/util/filemonitor-inotify.c
> index 3a72be037f..3eb29f860b 100644
> --- a/util/filemonitor-inotify.c
> +++ b/util/filemonitor-inotify.c
> @@ -29,7 +29,7 @@
>
> struct QFileMonitor {
> int fd;
> -
> +int nextid; /* watch ID counter */
> QemuMutex lock; /* protects dirs & idmap */
> GHashTable *dirs; /* dirname => QFileMonitorDir */
> GHashTable *idmap; /* inotify ID => dirname */
> @@ -47,7 +47,6 @@ typedef struct {
> typedef struct {
> char *path;
> int id; /* inotify ID */
> -int nextid; /* watch ID counter */
> GArray *watches; /* QFileMonitorWatch elements */
> } QFileMonitorDir;
>
> @@ -277,7 +276,7 @@ qemu_file_monitor_add_watch(QFileMonitor *mon,
> }
> }
>
> -watch.id = dir->nextid++;
> +watch.id = mon->nextid++;
> watch.filename = g_strdup(filename);
> watch.cb = cb;
> watch.opaque = opaque;
Thanks, this fixes it! I had a related question about the way
qemu_file_monitor_add_watch works.
Am I correct in understanding that if there is already a watch on a dir,
we return back mon->nextid++ i.e the next free id. Why don't we return
back the originally assigned watchid ?
Reviewed-and-tested-by: Bandan Das
Peter Maydell writes:
> On Fri, 8 Mar 2019 at 19:39, Bandan Das wrote:
>>
>> Peter Maydell writes:
>> > But the two places in usb_mtp_get_data() that call
>> > usb_mtp_write_metadata() still don't check its return
>> > value: don't they need
Daniel P. Berrangé writes:
> On Tue, Mar 12, 2019 at 07:07:42PM -0400, Bandan Das wrote:
>> Daniel P. Berrangé writes:
>> ...
>> > +
>> > +int
>> > +qemu_file_monitor_add_watch(QFileMonitor *mon,
>> > +const char *di
watch.opaque = opaque;
> +
> +g_array_append_val(dir->watches, watch);
> +
> +trace_qemu_file_monitor_add_watch(mon, dirpath,
> + filename ? filename : "",
> + cb, opaque, watch.id);
> +
> +ret
Peter Maydell writes:
> On Tue, 12 Mar 2019 at 18:25, Bandan Das wrote:
>>
>>
>> Spotted by Coverity: CID 1399414
>>
>> mtp delete allows the return status of delete succeeded,
>> partial_delete or readonly - when none of the objects could be
>>
case,
only READ_ONLY can be returned.
Signed-off-by: Bandan Das
---
v3:
fix typo
use g_assert_not_reached
v2:
Change the enum variable names and specify them as bits
Add a comment describing the bit definitions
Modify commit message slightly
hw/usb/dev-mtp.c | 62
case,
only READ_ONLY can be returned.
Signed-off-by: Bandan Das
---
v2:
Change the enum variable names and specify them as bits
Add a comment describing the bit definitions
Modify commit message slightly
hw/usb/dev-mtp.c | 62 ++--
1 file changed, 34
top of the function or documenting
> the meaning of the enum values.
>
Peter, thank you for the thorough review, I really appreciate it.
I definitely want to make this code less confusing to the next
potential reviewer. I will address all your suggestions in the
next version of the patch.
Bandan
> thanks
> -- PMM
Peter Maydell writes:
...
>> >
>> >> > It might be useful to take a step back -- what are
>> >> > the different possible outcomes from this function that
>> >> > we need to distinguish, and when should we be returning
>> >> > which outcome?
>> >
>> They are what the variable names signify.
>
> Tha
Peter Maydell writes:
> On Mon, 11 Mar 2019 at 16:43, Bandan Das wrote:
>> Peter Maydell writes:
>> > On Mon, 11 Mar 2019 at 16:14, Bandan Das wrote:
>> > Generally, if you have multiple bits X, Y in a return
>> > value, they should be independent. Sometime
Peter Maydell writes:
> On Mon, 11 Mar 2019 at 16:14, Bandan Das wrote:
>>
>> Peter Maydell writes:
>> > At the moment PARTIAL_DELETE is "ALL_DELETE | READ_ONLY", which
>> > doesn't seem like it makes much sense.
>> >
>>
>>
Peter Maydell writes:
> On Fri, 8 Mar 2019 at 22:14, Bandan Das wrote:
>>
>>
>> Spotted by Coverity: CID 1399414
>>
>> mtp delete allows the a return status of delete succeeded,
>> partial_delete or readonly - when none of the objects could be
>> de
Spotted by Coverity: CID 1399414
mtp delete allows the a return status of delete succeeded,
partial_delete or readonly - when none of the objects could be
deleted.
Some initiators recurse over the objects themselves. In that case,
only READ_ONLY can be returned.
Signed-off-by: Bandan Das
Peter Maydell writes:
> On Thu, 7 Mar 2019 at 09:56, Gerd Hoffmann wrote:
>>
>> From: Bandan Das
>>
>> Spotted by Coverity: CID 1399144
>>
>> Signed-off-by: Bandan Das
>> Message-id: 20190306210409.14842-4-...@redhat.com
>> Signed-off
Peter Maydell writes:
> On Thu, 7 Mar 2019 at 09:58, Gerd Hoffmann wrote:
>>
>> From: Bandan Das
>>
>> During a write, free up the "path" before getting more data.
>> Also, while we at it, remove the confusing usage of d->fd for
>> storing
During a write, free up the "path" before getting more data.
Also, while we at it, remove the confusing usage of d->fd for
storing mkdir status
Spotted by Coverity: CID 1398642
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 18 ++
1 file changed, 10 insertions(+)
Spotted by Coverity: CID 1399144
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 1f22284949..06e376bcd2 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1177,9 +1177,7
MTP writes objects in small chunks and at the end gets the
real file size to update the object metadata. If this fails for
any reason, return an INCOMPLETE_TRANSFER to the initiator
Spotted by Coverity: CID 1398651
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 19 ++-
1 file
v2:
Rebase
Prevent a null dereference when deleting objects
Fix some coverity issues and a few others pointed out by Peter
Bandan Das (3):
usb-mtp: return incomplete transfer on a lstat failure
usb-mtp: fix some usb_mtp_write_data return paths
usb-mtp: prevent null dereference while
ectInfo struct is (eg "format" and "size" and see
>> whether we should be doing byte swapping here.
>>
>
> I don't have any idea on that... the code just seems to assume
> everything is host endian.
>
>> PS: it is a bit confusing that in this function the local
>> variable "dataset" is a pointer to a struct of entirely
>> different type to the one that s->dataset is.
>>
>
> Maybe Gerd or Bandan can comment on that.
>
>> thanks
>> -- PMM
MTP writes objects in small chunks and at the end gets the
real file size to update the object metadata. If this fails for
any reason, return an INCOMPLETE_TRANSFER to the initiator
Spotted by Coverity: CID 1398651
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 20 +++-
1
Fix some coverity issues and a few others pointed out by Peter
Bandan Das (2):
usb-mtp: return incomplete transfer on a lstat failure
usb-mtp: fix some usb_mtp_write_data return paths
hw/usb/dev-mtp.c | 38 +-
1 file changed, 25 insertions(+), 13
During a write, free up the "path" before getting more data.
Also, while we at it, remove the confusing usage of d->fd for
storing mkdir status
Spotted by Coverity: CID 1398642
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 18 ++
1 file changed, 10 insertions(+)
Peter Maydell writes:
> On Wed, 30 Jan 2019 at 07:41, Gerd Hoffmann wrote:
>>
>> From: Bandan Das
>>
>> For every MTP_WRITE_BUF_SZ copied, this patch writes it to file before
>> getting the next block of data. The file is kept opened for the
>> duration
Peter Maydell writes:
> On Fri, 15 Feb 2019 at 18:45, Bandan Das wrote:
>>
...
>> I believe this is a false positive, there's still more data incoming
>> and we have successfully written the data we got this time, so we return
>> without freeing up any of the
file size in the object metadata once the file has
completely been written.
Suggested-by: Gerd Hoffman
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 134 ---
1 file changed, 91 insertions(+), 43 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev
qemu_write_full takes care of partial blocking writes,
as in cases of larger file sizes
Suggested-by: Peter Maydell
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index
Eric Blake 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
>> mul
file upto a certain data size we have received so far and second,
reuse the buffer again instead of reallocating to a larger buffer size.
Tested with different file sizes on a Linux guest.
Bandan Das (3):
usb-mtp: Reallocate buffer in multiples of MTP_WRITE_BUF_SZ
usb-mtp: breakup MTP write
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
---
hw/usb/dev-
file size in the object metadata once the file has
completely been written.
Suggested-by: Gerd Hoffman
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 134 ---
1 file changed, 91 insertions(+), 43 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev
qemu_write_full takes care of partial blocking writes,
as in cases of larger file sizes
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index b607c7be05..f4d9470493 100644
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
---
hw/usb/dev-
buffer again instead of reallocating to a larger buffer size.
Tested with different file sizes on a Linux guest.
Bandan Das (3):
usb-mtp: Reallocate buffer in multiples of MTP_WRITE_BUF_SZ
usb-mtp: breakup MTP write into smaller chunks
usb-mtp: replace the homebrew write with qemu_write_full
Gerd Hoffmann writes:
> On Tue, Jan 22, 2019 at 07:41:16AM -0500, Bandan Das wrote:
>>
>> qemu_write_full takes care of partial blocking writes,
>> as in cases of larger file sizes
>>
>> Suggested-by: Peter Maydell
>> Signed-off-by: Bandan
>
> H
qemu_write_full takes care of partial blocking writes,
as in cases of larger file sizes
Suggested-by: Peter Maydell
Signed-off-by: Bandan
---
hw/usb/dev-mtp.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index
if ((ret == -1) && (errno != EINTR || errno != EAGAIN ||
>> errno != EWOULDBLOCK)) {
>>
>> Maybe better code
>>
>> if ((ret == -1) && (errno != EINTR && errno != EAGAIN &&
>&
to the GSoC/Outreachy project, I think the mentors (me?)
> need to figure this out beforehand by experimentation. The QEMU folks
> don't know the details of oss-fuzz and vice versa. But with a weekend
> or two's worth of playing around we could figure out a reasonable way of
> i
Eric Blake writes:
> On 1/11/19 2:20 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
>> mul
file size in the object metadata once the file has
completely been written.
Suggested-by: Gerd Hoffman
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 134 ---
1 file changed, 91 insertions(+), 43 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev
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
---
hw/usb/dev-
reallocating to a larger buffer size.
Tested with different file sizes on a Linux guest.
Bandan Das (2):
usb-mtp: Reallocate buffer in multiples of MTP_WRITE_BUF_SZ
usb-mtp: breakup MTP write into smaller chunks
hw/usb/dev-mtp.c | 154 ++-
1 file
uble with when I was trying to hack
on this, especially fuzzing Qemu as a whole is what would the run environment
be like ? Would Qemu attempt to run a regular guest in oss-fuzz mode or only
a certain part of Qemu (emulated devices for example) be somehow run without
interacting with other dependent
}
> +
> o = usb_mtp_object_lookup_name(p, filename, dataset->length);
> if (o != NULL) {
> next_handle = o->handle;
Should we return PARAMETER_NOT_SUPPORTED instead of INVALID_PARAMETER ?
That's a valid response code for SendObjectInfo acc to the spec.
Bandan
x27;t even be Unicode. It might well be on any host we care
> for, but are you *sure*?
>
> I guess g_utf16_to_utf8() would be differently wrong: it converts to
> UTF-8, but we need to convert to the current locale's multibyte string.
>
I couldn't find an existi
When writing to guest's MSR_IA32_ARCH_CAPABILITIES, check whether it's
supported in the guest using the KVM_GET_MSR_INDEX_LIST ioctl.
Fixes: d86f963694df27f11b3681ffd225c9362de1b634
Suggested-by: Eduardo Habkost
Tested-by: baldu...@units.it
Signed-off-by: Bandan Das
---
target/
baldu...@units.it writes:
> hello
>
>> incomplete because it can return 0 for data. Can you try this:
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index f524e7d929..4878ffb90b 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -2002,14 +2002,9 @@ static int kvm_put_m
Paolo Bonzini writes:
...
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f524e7d929..4878ffb90b 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -2002,14 +2002,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> #endif
>
> /*
baldu...@units.it writes:
> hello
>
> I'm building qemu from source and happily using it since a bit
> (2.3.0)
>
> Since 3.1.0-rc0 (including latest 3.1.0-rc1) I'm no more able to start
> qemu, getting:
>
> 8<
> install:115> qemu
> qemu: error: failed to set MSR 0x10a to 0x0
>
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
---
hw/usb/dev-
file size in the object metadata once the file has
completely been written.
Suggested-by: Gerd Hoffman
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 133 +--
1 file changed, 90 insertions(+), 43 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw
with different file sizes on a Linux guest.
Bandan Das (2):
usb-mtp: Reallocate buffer in multiples of MTP_WRITE_BUF_SZ
usb-mtp: breakup MTP write into smaller chunks
hw/usb/dev-mtp.c | 153 +++
1 file changed, 99 insertions(+), 54 deletions
From: Bandan
Return STORE_FULL if we can't write all the bytes but
return incomplete transfer if data received is less then
what was specified in the metadata. Also, use d->offset
as the file size which is valid for all file sizes.
Signed-off-by: Bandan
---
hw/usb/dev-mtp.c | 7 ++
Stale values in this field may result in qemu
expecting more data on the next operation
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 15edf3bb82..00a3691bae 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw
From: Bandan
Signed-off-by: Bandan
---
qemu-doc.texi | 2 +-
scripts/device-crash-test | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f74542a0e9..cc7d81181c 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -943,7 +943,7
v2:
Same as v1 but with another minor cleanup
patch. The write buffer breakup is still WIP.
A documentation fix and changes to return the
right error code on write failures.
Bandan (2):
usb-mtp: fix error conditions for write operation
doc: replace x-root with rootdir for usb-mtp
Bandan Das
Signed-off-by: Bandan
---
qemu-doc.texi | 2 +-
scripts/device-crash-test | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 7bd449f398..f7ad1dfe4b 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -943,7 +943,7 @@ for details
A documentation fix and changes to return the
right error code on write failures.
Bandan (2):
usb-mtp: fix error conditions for write operation
doc: replace x-root with rootdir for usb-mtp
hw/usb/dev-mtp.c | 7 ---
qemu-doc.texi | 2 +-
scripts/device-crash-test | 2
Return STORE_FULL if we can't write all the bytes but
return incomplete transfer if data received is less then
what was specified in the metadata. Also, use d->offset
as the file size which is valid for all file sizes.
Signed-off-by: Bandan
---
hw/usb/dev-mtp.c | 7 ---
1 file ch
Gerd Hoffmann writes:
> On Fri, Jul 20, 2018 at 05:40:18PM -0400, Bandan Das wrote:
>> For large buffers, write may not copy the full buffer. For example,
>> on Linux, write imposes a limit of 0x7000. Note that this does
>> not fix >4G transfers but ~>2G files
x-root was renamed as such owing to the experimental nature of the
property; the underlying filesystem semantics were undecided
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index
usb_mtp_realloc() was being incorrectly used when allocating
buffer for incoming data. Set d->length only after resizing
the buffer.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
in
Patch 1 adds support for canceling an ongoing transaction.
2,3 and 4 fix writes for large transfers. For > 4G file transfers,
the logic has been modified to check for the end of the data phase
by checking for a short packet. Patch 5 renames x-root to a more
meaningful rootdir.
Bandan Das
For large buffers, write may not copy the full buffer. For example,
on Linux, write imposes a limit of 0x7000. Note that this does
not fix >4G transfers but ~>2G files will transfer successfully.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 22 --
1 file chang
The initiator can choose to cancel an ongoing request which
is specified by bRequest=0x64. If such a request arrives,
free up any pending state
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/hw
To support larger file transfers, rely on a short packet
to detect end of the data phase and rewrite d->length to
the size received
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 31 +++
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/hw/usb/
Peter Maydell writes:
> On 18 May 2018 at 19:38, Bandan Das wrote:
>> Peter Maydell writes:
>>
>>> On 18 May 2018 at 19:22, Bandan Das wrote:
>>>>
>>>> CID 1390604
>>>> If the initiator sends a packet with TYPE_DATA s
CID 1390604
If the initiator sends a packet with TYPE_DATA set without
initiating a CMD_GET_OBJECT_INFO first, then usb_mtp_get_data
can trip on a null s->data_out.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/usb/dev-mtp.c b/hw/
Peter Maydell writes:
> On 18 May 2018 at 19:22, Bandan Das wrote:
>>
>> CID 1390604
>> If the initiator sends a packet with TYPE_DATA set without
>> initiating a CMD_GET_OBJECT_INFO first, then usb_mtp_get_data
>> can trip on a null s->data_out.
>>
>
Bandan Das writes:
>> If this is a "can't happen" situation we can mark it as a false
>> positive in coverity.
I posted a patch with an assert added in usb_mtp_get_data. I believe CID
1390604 can be
marked as a false positive.
Thanks,
Bandan
> The protocol ofco
CID 1390604
If the initiator sends a packet with TYPE_DATA set without
initiating a CMD_GET_OBJECT_INFO first, then usb_mtp_get_data
can trip on a null s->data_out.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/
1 - 100 of 381 matches
Mail list logo
