Could not write to device, result == -1 errno 0 null #237
Comments
|
Hey there, I think this is realted to etchdroid/etchdroid#10 Apparently under Pie this issue has gotten a lot worse. @depau and I are working on a fix but as we are quite busy, this can still take a while. I hope I will have some time over the Christmas break. |
|
Hey, it seems good. Thanks a lot! |
|
Hi, Sadly I haven't been able to work on it the last few months. Hopefully I'll have some time in February, definitely not before that :( |
|
Hi, I will try those tips and let you know if it works well for me. |
|
Those two links look unrelated to me. In case anybody wants to dig into this issue and write some code before @magnusja and I have time, a few hints: Here's some badly-taken notes: https://notes.depau.eu/ZkErsTYJQLC7AQJhcyn7VA#resetRecovery Here's some sample code that needs testing and refinement. DO NOT blindly copy-paste, thanks ;) private void resetRecovery() {
bulkOnlyMassStorageReset();
clearFeatureHALT(this.epIn);
}
private void bulkOnlyMassStorageReset() {
byte[] bArr = new byte[2];
// REQUEST_BULK_ONLY_MASS_STORAGE_RESET = 255
// REQUEST_TYPE_BULK_ONLY_MASS_STORAGE_RESET = 33
int controlTransfer = this.usbConn.controlTransfer(33, 255, 0, this.usbInterface.getId(), bArr, 0, 1000);
// check if it succeeded
}
private void clearFeatureHalt(UsbEndpoint usbEndpoint) {
byte[] bArr = new byte[2];
int address = usbEndpoint.getAddress();
// REQUEST_CLEAR_FEATURE = 1
// REQUEST_TYPE_CLEAR_FEATURE = 2
int controlTransfer = this.usbConn.controlTransfer(2, 1, 0, address, bArr, 0, 1000);
// check if it succeeded
}We basically need to implement these recovery reset methods. Whenever I think the "reset required detection" logic is best if implemented in I would then place the actual reset methods in Note that, however, we also require a @magnusja please review ↑ :) |
|
Hi, @mannyzhao! Sorry to keep you waiting. Unfortunately no. I am still trying to find a solution |
|
Hey guys, can you please try #240 |
|
Well... This is Android 10 though :P I'm going to test it on my old phone with Android 9, and maybe set a few logging breakpoints to see if the new code is actually reached. |
|
I can confirm it works on Android 9 too. However, I haven't seen the message logged for the reset procedure. The whole thing is quite weird. I'll test it on the emulator, on which is was 100% required for whatever reason. |
|
Nope, the issue eventually occurred and it looks like the patch didn't help: |
|
So, if this doesn't work, I think our last options are:
I'm aware of this policy, though trying is definitely easier than reimplementing the USB communication with raw libusb. If it doesn't work it should simply pretend the method does not exist, so handling that should be fine (in the sense that it won't get any worse than the current behavior). Side note: it's quite tedious for me to develop changes and test them on EtchDroid as I have to rebuild the shim in the middle and stuff. @kuza2010, did the issue occur to you in the libaums demo app? Does somebody have some steps to reproduce it in the demo app? |
|
Hey @depau, thanks a lot for testing. Well that is too bad :/ I will implement the first option, we can still think about the second one. We can also only move the reset then to native code. What essentially is happening is this: https://android.googlesource.com/platform/system/core/+/master/libusbhost/usbhost.c?autodive=0#682 This can be easily implement keeping the whole Kotlin (as of now :D) code! |
|
I pushed some more changes if someone wants to try again... |
Yeah, an IOCTL should be doable from Java/Kotlin. Anyway, nope, it doesn't help: |
|
BTW I'm going to implement the ioctl thing and see if it works. |
|
I actually implemented that already so dont bother. I forgot to push I guess, but will do now! |
|
Ah damn I reimplemented it in the meantime :/ |
|
Nope :( Also after the "native reset" the USB does not show up any more when querying for USB devices. (which is good to know cause I might just reset the device after writing the image successfully, so people stop complaining that their USB drive is broken when Android says the filesystem is unsupported) |
|
Ah snap, that is a pity. So this guy (https://raspberrypi.stackexchange.com/a/36664) says that we might wanna ignore the unsuccessful reset. Maybe you can comment this out (https://github.com/magnusja/libaums/pull/240/files#diff-c624845c950d217d9416054a1cb06616R41) and see what happens? Other than that I am unsure what might be worth trying next :/ |
|
Might be also worth a try only doing this native reset instead of the bulk only reset and the clear feature halt. |
|
I tested the code in that thread on my computer on a USB drive. (Here's my adapted code)#include <errno.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/ioctl.h>
#include <linux/usbdevice_fs.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
int main(int argc, char **argv)
{
char devicename[1024];
const char *filename = devicename;
int fd;
if ( 2 > argc || 3 < argc ) {
printf("Give USB device name as parameter\n");
return 1;
}
if ( 2 == argc )
filename = argv[1];
else
sprintf( devicename, "/dev/bus/usb/%s/%s", argv[1], argv[2] );
printf( "Resetting USB device '%s'\n", filename );
fd = open(filename, O_WRONLY);
if (fd <= 0) {
perror("USB device open failed");
return 2;
}
if (ioctl((long) fd, USBDEVFS_RESET, 0) == -1) {
// Don't care! It usually does, when we need to reset it.
perror("USBDEVFS_RESET device ioctl error");
//exit(3);
}
close(fd);
return 0;
}Results: sometimes the ioctl succeeds and I get this: Sometimes it fails: and when that happens Which makes me fear we need to move I'm currently fighting with gradle, I'll see what I can do. |
|
I'm noticing that, for whatever reason, I'm getting almost always successfully writes while back then I would only be able to write ~500MB before it would show the issue. Anyway I was tired of not seeing errors so I built the utility for Android and used it to reset the USB drive while the app was writing, which triggered the new code of course. I'm testing with a bunch of commits on top of your changes: https://github.com/Depau/libaums/tree/feature/bulk-only-reset Kernel log with comments: All ioctls succeeded so it didn't act like the drive was reinserted: I'll try moving the opening code inside the communication class, so the USB connection can eventually be reopened after reset. |
|
It works! The sequence reset -> close -> open seems to do it. I was able to reset the device multiple times while it was writing, libaums would handle it gracefully by performing the recovery procedure. I re-dumped the image from the USB drives and checksums were correct 🎉 At some point while testing my injected ioctl failed, though, so the device showed up as a new device. This probably needs to be handled at a higher level, like a physical reconnection. EDIT: I can't get the kernel logs from this one as it's not rooted, so I have to do that first. |
|
Hey @depau thanks a lot for your help and all the debugging! I guess you could call that progress at least :D If it is working reliably on one of your devices I guess it till makes sense merging this. Let me know if you know something more about the failing device. Thanks! |
|
I'm currently not able to root it for whatever reason I'm not aware of. I'll try it again on QEMU and see if I'm able to reproduce it there. If not, I don't think I'll be able to work on it any more for this month :/ |
|
Alright, it looks like trying over and over to reset the device works some times, I saw it resume writing after one or two attempts. In one test it halted a few times and recovered again. Sometimes though, the reset ioctl fails and the USB device "disappears". I think that for the time being the changes can be merged; in the worst case the USB device will "disappear". In the best case the communication will resume. Ultimately, looking at libusb code, I think that eventually wrapping it would solve lot of issues and that it might be worth spending time on that. They seem to have a lot of checks for "suspiciously specific" issues and comparing libusb and the Android usbhost lib it looks like Android is taking a lot of shortcuts (see libusb's reset device vs Android's for example) |
|
Hey @depau so I merged and released a new version to see if people still open bug reports regarding this error :D In general I like the idea with libusb, but I am not sure how this is affecting the license as LGPL is quite restrictive I think. In addition all the Linux fs implementations are under GPL which would make closed apps using lbaums impossible unfortunately. @kuza2010 @mannyzhao please try latest version |
|
@kuza2010 Have you ever seen it recover sometimes after an IO error? I.e. you see a bunch of those |
|
Hey @depau During the copying process, the recovery mechanism can be called successfully several times. I saw that the implemented recover mechanism works as intended and after that copy process was successfully completed. |
At least we can't tell for sure it does help sometimes, it's not completely useless. Hopefully we'll get more consistent results with the |
|
Hey @kuza2010 @mannyzhao can you please try this: https://github.com/magnusja/libaums/tree/develop/libusbcommunication |
|
@magnusja |
|
That's the same error I get on QEMU. BTW you may also want to test the latest version of EtchDroid, I shipped it with the new libusb implementation.
Xiaomi Mi 9 Lite with Pixel Experience GSI, OnePlus 3T with LineageOS MicroG, Android x86 on QEMU with a real USB drive passed-through. |
|
The latest version was tested only with Samsung. |
|
@magnusja During the process of copying one huge file (1 and 2 Gb), the recovery mechanism was never been called. I tried about 10 attempts per each file :| Host:
|
|
Okay I see, thanks a lot for testing. Maybe I can optimize the copy of multiple files somehow so that it results in fewer small writes. Can you maybe provide the code copying the 1000 files? |
|
It is very kind of your side! I think it is real, but it takes time because I need to solve a few problems. |
|
Hey @magnusja So, please find simple snippets below:
@Override
public synchronized InputStream getInputStream(UsbMassStorageDevice device, FileItem fileItem)
throws USBException, IOException {
Timber.tag(TAG).d("getInputStream: open stream %s ...", fileItem.getPath());
FileSystem fs = getFs(device);
UsbFile rootDir = fs.getRootDirectory();
// getRelativePath return /folder/file.txt for path /Usb_prefix/folder/file.txt
UsbFile srcFile = rootDir.search(getRelativePath(fs, fileItem.getPath()));
if (srcFile == null)
throw new USBException(USBErrorCode.FILE_NOT_FOUND);
return UsbFileStreamFactory.createBufferedInputStream(srcFile, fs);
}
@Override
public synchronized OutputStream getOutputStream(UsbMassStorageDevice device, FileItem fileItem)
throws IOException, USBException {
Timber.tag(TAG).d("getOutputStream: open stream %s ...", fileItem.getPath());
FileSystem fs = getFs(device);
UsbFile rootDir = fs.getRootDirectory();
UsbFile srcFile = rootDir.search(getRelativePath(fs, fileItem.getPath()));
if (srcFile != null)
throw new USBException(USBErrorCode.FILE_EXISTS);
//create parent folder for srcFile
UsbFile srcFileParentFolder = mkDirs(device, constructParentFileItem(fileItem));
srcFile = srcFileParentFolder.createFile(fileItem.getName());
return UsbFileStreamFactory.createBufferedOutputStream(srcFile, fs);
}
void doRead() throws IOException {
if (this.state.compareAndSet(State.NEW, State.RUNNING))
// some checks before read and create Output Stream
if (!prepareForRead()) return;
if ((this.state.get() == State.RUNNING)
|| this.state.compareAndSet(State.SUSPENDED, State.RUNNING)) {
try {
// Integer.maxValue
int executeCount = weight;
do {
if (failedBytes != null) {
boolean isOfferSucceed;
if (failedBytes == Constants.EOF) {
isOfferSucceed = offerEOF();
} else {
isOfferSucceed = offerNextBlock(failedBytes, failedBytes.length);
}
if (isOfferSucceed) {
failedBytes = null;
} else {
//suspend operation because failed bytes did not insert into queue
suspend();
}
} else {
//read from Is next readBlockSize bytes
byte[] contents = new byte[readBlockSize];
int readBytes = inputStream.read(contents);
boolean isEOF = checkIsEndOfFile(readBytes);
if (isEOF) {
Timber.tag(TAG).i("Achieve end of file %s", srcFileItem.getName());
//insert EOF = {}
offerEOF();
} else {
if (readBytes < contents.length) {
byte[] bytesCopied = Arrays.copyOf(contents, readBytes);
offerNextBlock(bytesCopied, readBytes);
} else {
offerNextBlock(contents, readBytes);
}
}
}
executeCount--;
} while (isExitCondition(executeCount));
state.compareAndSet(State.RUNNING, State.SUSPENDED);
} finally {
if (getState() == State.COMPLETED)
closeStreams();
}
}
}
// Offer EOF = {} into blocking queuq
private boolean offerEOF() {
boolean isAddSucceed = blockingQueue.offer(Constants.EOF);
if (isAddSucceed) {
Timber.tag(TAG).i("Read completed for the file %s", srcFileItem.getUri());
state.set(State.COMPLETED);
delegate.onReadSuccess();
} else {
failedBytes = Constants.EOF;
suspend();
}
return isAddSucceed;
}
// Offer next read data into blocking queue
boolean offerNextBlock(byte[] contents, int readBytes) {
boolean isAddSucceed = blockingQueue.offer(contents);
if (isAddSucceed) {
//delegate on progress changed will call
publishReadProgress(readBytes);
} else {
failedBytes = contents;
suspend();
}
return isAddSucceed;
}
void createStream() throws IOException, USBException {
// FileSystemService.getInputStream will be called
inputStream = api.getFileSystemService().getInputStream(usbDevice, srcFileItem);
// you should read and write in multiples of chunk size
// return device.partitions.get(0).getFileSystem().getChunkSize()
int chunkSize = storageLocation.getChunkSize();
// readblockSize = minimum of (maximum of chunk size and file size) and
// integer value from dividing the buffer size by chunk si
readBlockSize = (int) Math.min(Math.max(chunkSize, srcFileItem.getLength()),
preferences.getBufferSize() / chunkSize * chunkSize);
Timber.tag(TAG).d("createStream: IS successfully created, readBlockSize = %s", readBlockSize);
}
void closeStreams() throws IOException {
if (inputStream != null) {
inputStream.close();
inputStream = null;
}
}
@Override
public boolean suspend() {
return this.state.compareAndSet(State.RUNNING, State.SUSPENDED);
}
protected void doWrite() throws IOException {
if (this.state.compareAndSet(State.NEW, State.RUNNING)) {
// some checks before write and create Output Stream
if (!prepareForWrite()) {
return;
}
}
if ((this.state.get() == State.RUNNING)
|| this.state.compareAndSet(State.SUSPENDED, State.RUNNING)) {
byte[] nextByte;
try {
// Integer.maxValue
int executeCount = weight;
do {
// retrieves next block of data from queue
nextByte = blockingQueue.poll();
if (nextByte == null) {
//suspend because blocking queue is empty
suspend();
break;
}
if (isPoissonPill(nextByte)) {
state.set(State.COMPLETED);
Timber.tag(TAG).i("Write completed for the file %s", destFile.getPath());
break;
} else {
// write data to device
outputStream.write(nextByte);
publishProgress(nextByte.length);
executeCount--;
}
} while (isExitCondition(executeCount));
state.compareAndSet(State.RUNNING, State.SUSPENDED);
} finally {
if (State.COMPLETED == state.get()) {
closeStreams();
delegate.onWriteSuccess();
}
}
}
}
// check EOF
private boolean isPoissonPill(byte[] nextByte) {
return nextByte == Constants.EOF;
}
void createStream() throws USBException, IOException {
// FileSystemService.getOutputStream will be called
outputStream = api.getFileSystemService().getOutputStream(usbDevice, destFile);
file = api.getFileSystemService().getUsbFile(usbDevice, destFile);
}
private void closeStreams() {
try {
if (outputStream != null) {
outputStream.close();
outputStream = null;
}
} catch (IOException e) {
e.printStackTrace();
}
}
@Override
public boolean suspend() {
return this.state.compareAndSet(State.RUNNING, State.SUSPENDED);
}Some details with can help to understand how it works:
Feel free to ask questions. |
|
Hey there, thanks for the code. What you might want to try, which definitely results in fewer writes is instead of using the buffered stream factory just do: Another thing you might want to try is to set the length of the file you are copying first. This will allocate memory for the file in one run and increases performance. Otherwise you are always switching between writing the actual file and allocating new space in the FAT. So please do something like |
|
Hey @magnusja I implemented your advice above and tried a few copying attempts. But I still observe the same issue :( For tests, I used
UPD: Thanks |
|
Hey @magnusja I tested it again with some new phones. Test data: about 1000 mixed size/type files (total size 2 Gb).
And tests results very strange and unpredictable. For example, on Moto device I can copy all files successfully (5/5). But if I use Xiaomi/Samsung I can't copy the same folder (0/5), in these cases I get |
|
Hey there, I can't find any other ways to solve this problem. |
|
Unfortunately I am out of ideas :/ I would suggest that you try to debug that on your own and let us know if you have any further insights. |
|
Sorry for not showing up for a while, before resuming not showing up for another few months I'd like to add that switching to libusb did reduce by a lot the number of bug reports/bad reviews on the Play Store for EtchDroid. Now I'm basically only getting bad reviews because people flashed a Windows ISO and [surprised pikachu face] the BIOS says "no bootable medium found" (though I literally added a huge message "WINDOWS ISOs NOT SUPPORTED") or because they have no idea how filesystems work and they think the USB drive is broken when Android/Windows tells them they need to format it before using it. The issue is definitely not solved yet since it does still occur sometimes on real devices, it still occurs 100% times on Qemu, and it doesn't occur with the proprietary app. I hope I'll have some time in the summer to finish up the Qemu instrumented tests and see if we can shed a light on it. |
Great to here from you and the improved reviews! :) Hope you are staying safe.
Well people, huh? :D
Thanks a lot for your commitment! What proprietary app do you mean? USB Media Explorer, formerly Nexus Media Importer or Total Commander? |
I don't remember right now, I think we talked about it in the Qemu thread :) I remember it was a proprietary plugin for a file manager, so probably total commander |
|
Hello! I have scroll here and trying to find the problem! I have tried to transfer windows 10 from my phone to my USB drive because I need to install windows on my PC because it's corrupt and it worked first but my PC wanned not to accept it haha! So now I have delete everything on the PC and now the app wouldnt accept it! And it's the code this quote start with .. 1 errno 0 null have you guys figured out? |
|
|
@kuza2010 Can you please try again with the latest commits from You can use JitPack for a quick test: Also you should be able to run the instrumented tests, can you see if they work? Pull the develop branch and run |
|
@depau at the moment it's impossible for me to test it with the last commit. |
|
Hello, fellas!
For tests, I used the same phones set and mass storages.
|
Can you try the |
Hi, @magnusja
Libaums version 0.7.3
Hardware devices:
Problem
I try to copy about 1000 mixed size/type files (total size 2 Gb).
Expected behavior
Files were copied correctly.
Actual behavior
java.io.IOException: Could not write to device, result == -1 errno 0 nullStacktrace of Excpetion
Code where problem occurs
UsbFileOutputStream.write(bytes);
Any hints!
Thanks a lot!
The text was updated successfully, but these errors were encountered: