Version: GIT master OS: Linux When the disk on which the XML database resides is full, saving the database may fail. In this case, an error message is displayed, along with a warning that the xml file is corrupted (and that the user should save the file again). From the mail-diskussion: >>> A bit tired here..so I just ask about it. How is it handled if the >>> write of index.xml will fail. Will the original one stay current or do >>> we get a corrupted index.xml? >> >> Unfortunately we get a corrupted file. That's why I added a big fat warning to >> the error message in this situation. It's certainly an improvement to the >> previous situation, where KPA would silently fail and leave the file corrupted >> (and because of the bugs in thumbnail-management it have an increased risk of >> crashing in the same situation). >> >> Handling that situation cleanly would require writing to a temporary file, >> copying the new file over the previous version and then deleting the temporary >> file. > >Yep, but you should move the temporary file instead of copying to >minimize the risk of error. Would you be willing to write this patch >also? Reproducible: Always Steps to Reproduce: Fill up the disk, add data to the database, save database. Expected Results: After saving failed, DB-file should be the same as before saving.
Created attachment 68936 [details] Save xml to temporary file first (prevent DB corruption).
Git commit fd1fe4b9989d911d773ef40e21937fc4a17e0cc2 by Miika Turkia, on behalf of Johannes Zarl. Committed on 19/02/2012 at 22:03. Pushed by mturkia into branch 'master'. Save xml to temporary file first (prevent DB corruption). M +49 -18 XMLDB/FileWriter.cpp http://commits.kde.org/kphotoalbum/fd1fe4b9989d911d773ef40e21937fc4a17e0cc2
FYI, there is no need to go manually closing the bug when you add a fixing patch there. It is actually better to close it only after the patch has been applied to git. The BUG: <id> -keyword will automatically close the bug when the patch is applied (if the committer has proper rights in bugs.kde.org) Anyway, thanks for the patch! miika On Sun, Feb 19, 2012 at 11:07 PM, Johannes Zarl <isilmendil@gmx.net> wrote: > https://bugs.kde.org/show_bug.cgi?id=294059 > > > Johannes Zarl <isilmendil@gmx.net> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Status|UNCONFIRMED |RESOLVED > Resolution| |FIXED > > > > > -- > Configure bugmail: https://bugs.kde.org/userprefs.cgi?tab=email > ------- You are receiving this mail because: ------- > You are the assignee for the bug. > _______________________________________________ > Kphotoalbum-bugs mailing list > Kphotoalbum-bugs@mail.kdab.com > https://mail.kdab.com/mailman/listinfo/kphotoalbum-bugs
Created attachment 68969 [details] Don't fail if old xml file is removed during saving It seems that the previous patch does not work well with the autosave feature, leading to spurious error-messages: "Failed to remove old version of image database. Please try again or replace the file .#index.xml with file .#index.xml.tmp manually!" This happened to me twice, but I could not trigger it another time. My best guess would be that the KPA sometimes removes the previous autosave file (.#index.xml) while an autosave is in progress. The attached patch makes FileWriter::save() robust against a vanishing database file...
I noticed the same bug but forgot to commit my fix yesterday. Now it is done in your way. However, test whether file exists is done before attempting to remove it. (The file never exists after the removal.) http://commits.kde.org/kphotoalbum/0aa7f4dcefaf5b01bd7f8c4fd4fb9240bd024b67
The test has to be done _after_ removal. Otherwise you still have the race-condition (although with a smaller window of opportunity than before)...
You are absolutely right. I should have read the error message and have another espresso before doing reading the code. Fixed now.