[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: branches/KDE/3.5/kdebase/kioslave/media [POSSIBLY UNSAFE]
From: Fathi Boudra <fboudra () free ! fr>
Date: 2006-11-01 18:55:14
Message-ID: 1162407314.686266.1164.nullmailer () svn ! kde ! org
[Download RAW message or body]
SVN commit 601028 by fabo:
halbackend fstab improvements (approved by Kevin Ottens)
* fixes HALBackend::inInFstab() to correctly match the fstab device node
that is a symlink.
* Improve error messages for HAL/fstab unmount failures. Include a list
of processes using the device in them (if there are any).
* When handling fstab devices, make HALBackend::mount() and
HALBackend::umount() calls block until the operation actually
completes (the same behaviour as with HAL devices).
* In mounthelper always unmount a device before ejecting it. However,
if unmount fails, still try to eject the device. In case of failure,
display the unmount error message which is more informative.
Thanks to Modestas Vainius.
--Cette lgne, et les suivantes ci-dessous, seront ignorées--
M kioslave/media/mounthelper/kio_media_mounthelper.cpp
M kioslave/media/mounthelper/kio_media_mounthelper.h
M kioslave/media/mediamanager/halbackend.cpp
M kioslave/media/mediamanager/halbackend.h
M +118 -35 mediamanager/halbackend.cpp [POSSIBLY UNSAFE: popen]
M +16 -1 mediamanager/halbackend.h
M +32 -52 mounthelper/kio_media_mounthelper.cpp
M +1 -3 mounthelper/kio_media_mounthelper.h
--- branches/KDE/3.5/kdebase/kioslave/media/mediamanager/halbackend.cpp \
#601027:601028 @@ -21,6 +21,8 @@
#include <stdlib.h>
+#include <kapplication.h>
+#include <qeventloop.h>
#include <qfile.h>
#include <klocale.h>
#include <kurl.h>
@@ -978,15 +980,79 @@
}
+QString HALBackend::listUsingProcesses(const Medium* medium)
+{
+ QString proclist, fullmsg;
+ QString cmdline = QString("/usr/bin/env fuser -vm %1 \
2>&1").arg(KProcess::quote(medium->mountPoint())); + FILE *fuser = \
popen(cmdline.latin1(), "r"); +
+ uint counter = 0;
+ if (fuser) {
+ proclist += "<pre>";
+ QTextIStream is(fuser);
+ QString tmp;
+ while (!is.atEnd()) {
+ tmp = is.readLine();
+ tmp = QStyleSheet::escape(tmp) + "\n";
+
+ proclist += tmp;
+ if (counter++ > 10)
+ {
+ proclist += "...";
+ break;
+ }
+ }
+ proclist += "</pre>";
+ (void)pclose( fuser );
+ }
+ if (counter) {
+ fullmsg = i18n("Moreover, programs still using the device "
+ "have been detected. They are listed below. You have to "
+ "close them or change their working directory before "
+ "attempting to unmount the device again.");
+ fullmsg += "<br>" + proclist;
+ return fullmsg;
+ } else {
+ return QString::null;
+ }
+}
+
void HALBackend::slotResult(KIO::Job *job)
{
kdDebug() << "slotResult " << mount_jobs[job] << endl;
- if (job->error())
- {
- KMessageBox::error(0, job->errorText());
+
+ struct mount_job_data *data = mount_jobs[job];
+ QString& qerror = data->errorMessage;
+ const Medium* medium = data->medium;
+
+ if (job->error() == KIO::ERR_COULD_NOT_UNMOUNT) {
+ QString proclist(listUsingProcesses(medium));
+
+ qerror = "<qt>";
+ qerror += i18n("Unfortunately, the device <b>%1</b> (%2) named <b>'%3'</b> \
and " + "currently mounted at <b>%4</b> could not be unmounted. \
").arg( + "system:/media/" + medium->name(),
+ medium->deviceNode(),
+ medium->prettyLabel(),
+ medium->prettyBaseURL().pathOrURL());
+ qerror += i18n("The following error was returned by umount command:");
+ qerror += "<br><pre>" + job->errorText() + "</pre>";
+
+ if (!proclist.isEmpty()) {
+ qerror += proclist;
+ }
+ qerror += "</qt>";
+ } else if (job->error()) {
+ qerror = job->errorText();
}
- ResetProperties( mount_jobs[job].latin1() );
+
+ ResetProperties( medium->id().latin1() );
mount_jobs.remove(job);
+
+ /* Job completed. Notify the caller */
+ data->error = job->error();
+ data->completed = true;
+ kapp->eventLoop()->exitLoop();
}
QString HALBackend::isInFstab(const Medium *medium)
@@ -1002,7 +1068,7 @@
if ( reald.endsWith( "/" ) )
reald = reald.left( reald.length() - 1 );
kdDebug() << "isInFstab -" << medium->deviceNode() << "- -" << reald << "- \
-" << (*it)->mountedFrom() << "-" << endl;
- if ((*it)->mountedFrom() == medium->deviceNode() || ( \
!medium->deviceNode().isEmpty() && (*it)->realDeviceName() == medium->deviceNode() ) \
) + if ((*it)->mountedFrom() == medium->deviceNode() || ( \
!medium->deviceNode().isEmpty() && reald == medium->deviceNode() ) ) {
QStringList opts = (*it)->mountOptions();
if (opts.contains("user") || opts.contains("users"))
@@ -1021,14 +1087,25 @@
QString mountPoint = isInFstab(medium);
if (!mountPoint.isNull())
{
+ struct mount_job_data data;
+ data.completed = false;
+ data.medium = medium;
+
kdDebug() << "triggering user mount " << medium->deviceNode() << " " << \
mountPoint << " " << medium->id() << endl;
KIO::Job *job = KIO::mount( false, 0, medium->deviceNode(), mountPoint );
connect(job, SIGNAL( result (KIO::Job *)),
SLOT( slotResult( KIO::Job *)));
- mount_jobs[job] = medium->id();
- return QString(); // we won't report an error here
+ mount_jobs[job] = &data;
+ // The caller expects the device to be mounted when the function
+ // completes. Thus block until the job completes.
+ while (!data.completed) {
+ kapp->eventLoop()->enterLoop();
+ }
+ // Return the error message (if any) to the caller
+ return (data.error) ? data.errorMessage : QString::null;
+
} else if (medium->id().startsWith("/org/kde/") )
- return i18n("Permissions denied");
+ return i18n("Permissions denied");
QStringList soptions;
@@ -1130,12 +1207,22 @@
QString mountPoint = isInFstab(medium);
if (!mountPoint.isNull())
{
+ struct mount_job_data data;
+ data.completed = false;
+ data.medium = medium;
+
kdDebug() << "triggering user unmount " << medium->deviceNode() << " " << \
mountPoint << endl; KIO::Job *job = KIO::unmount( medium->mountPoint(), false );
connect(job, SIGNAL( result (KIO::Job *)),
SLOT( slotResult( KIO::Job *)));
- mount_jobs[job] = medium->id();
- return QString(); // we won't report an error here
+ mount_jobs[job] = &data;
+ // The caller expects the device to be unmounted when the function
+ // completes. Thus block until the job completes.
+ while (!data.completed) {
+ kapp->eventLoop()->enterLoop();
+ }
+ // Return the error message (if any) to the caller
+ return (data.error) ? data.errorMessage : QString::null;
}
DBusMessage *dmesg, *reply;
@@ -1174,37 +1261,33 @@
dbus_error_init (&error);
if (!(reply = dbus_connection_send_with_reply_and_block (dbus_connection, dmesg, \
-1, &error))) {
+ QString qerror, reason;
+
kdDebug() << "unmount failed for " << udi << ": " << error.name << " " << \
error.message << endl;
- QString qerror = error.message;
+ qerror = "<qt>";
+ qerror += i18n("Unfortunately, the device <b>%1</b> (%2) named <b>'%3'</b> \
and " + "currently mounted at <b>%4</b> could not be unmounted. \
").arg( + "system:/media/" + medium->name(),
+ medium->deviceNode(),
+ medium->prettyLabel(),
+ medium->prettyBaseURL().pathOrURL());
+ qerror += i18n("Unmounting failed due to the following error:");
if (!strcmp(error.name, "org.freedesktop.Hal.Device.Volume.Busy")) {
- qerror = QString("<qt>") + i18n("Device is Busy:");
- QString cmdline = QString("/usr/bin/env fuser -vm %1 \
2>&1").arg(KProcess::quote(medium->mountPoint()));
- FILE *fuser = popen(cmdline.latin1(), "r");
- uint counter = 0;
- if (fuser) {
- qerror += "<pre>";
- QTextIStream is(fuser);
- QString tmp;
- while (!is.atEnd()) {
- tmp = is.readLine();
- tmp = QStyleSheet::escape(tmp) + "\n";
-
- qerror += tmp;
- if (counter++ > 20)
- {
- qerror += "...";
- break;
- }
- }
- qerror += "</pre>";
- (void)pclose( fuser );
- }
- qerror += "</qt>";
+ reason = i18n("Device is Busy:");
} else if (!strcmp(error.name, \
"org.freedesktop.Hal.Device.Volume.NotMounted")) {
// this is faking. The error is that the device wasn't mounted by hal \
(but by the system)
- qerror = i18n("Permissions denied");
+ reason = i18n("Permissions denied");
+ } else {
+ reason = error.message;
}
+ qerror += "<p><b>" + reason + "</b></p>";
+ // Include list of processes (if any) using the device in the error message
+ reason = listUsingProcesses(medium);
+ if (!reason.isEmpty()) {
+ qerror += reason;
+ }
+
dbus_message_unref (dmesg);
dbus_error_free (&error);
return qerror;
--- branches/KDE/3.5/kdebase/kioslave/media/mediamanager/halbackend.h #601027:601028
@@ -151,6 +151,7 @@
void setCameraProperties(Medium* medium);
QString generateName(const QString &devNode);
static QString isInFstab(const Medium *medium);
+ static QString listUsingProcesses(const Medium *medium);
private slots:
void slotResult(KIO::Job *job);
@@ -216,7 +217,21 @@
DBusConnection *dbus_connection;
- QMap<KIO::Job *, QString> mount_jobs;
+ /**
+ * Data structure for fstab mount/unmount jobs
+ */
+ struct mount_job_data {
+ // [in] Medium, which is being mounted/unmounted by the job
+ const Medium* medium;
+ // [in,out] Should be set to true when the job completes
+ bool completed;
+ // [out] KIO::Error if an error occured during operation. Otherwise, 0
+ int error;
+ // [out] Error message to be displayed to the user
+ QString errorMessage;
+ };
+
+ QMap<KIO::Job *, struct mount_job_data*> mount_jobs;
};
#endif /* _HALBACKEND_H_ */
--- branches/KDE/3.5/kdebase/kioslave/media/mounthelper/kio_media_mounthelper.cpp \
#601027:601028 @@ -91,6 +91,23 @@
}
else if (args->isSet("s") || args->isSet("e"))
{
+ /*
+ * We want to call mediamanager unmount before invoking eject. That's
+ * because unmount would provide an informative error message in case of
+ * failure. However, there are cases when unmount would fail
+ * (supermount, slackware, see bug#116209) but eject would succeed.
+ * Thus if unmount fails, save unmount error message and invokeEject()
+ * anyway. Only if both unmount and eject fail, notify the user by
+ * displaying the saved error message (see ejectFinished()).
+ */
+ if (medium.isMounted())
+ {
+ DCOPRef mediamanager("kded", "mediamanager");
+ DCOPReply reply = mediamanager.call( "unmount", medium.id());
+ if (reply.isValid())
+ reply.get(m_errorStr);
+ m_device = device;
+ }
invokeEject(device, true);
}
else
@@ -115,72 +132,35 @@
*proc << "-q";
}
*proc << device;
- proc->start();
connect( proc, SIGNAL(processExited(KProcess *)),
- this, SLOT( finished() ) );
+ this, SLOT( ejectFinished(KProcess *) ) );
+ proc->start();
}
-void MountHelper::slotResultSafe(KIO::Job* job)
+void MountHelper::ejectFinished(KProcess* proc)
{
- if (job->error())
- {
- m_errorStr = job->errorText();
-
- if (m_isCdrom)
- {
- m_errorStr+= i18n("\nPlease check that the disk is"
- " entered correctly.");
+ /*
+ * If eject failed, report the error stored in m_errorStr
+ */
+ if (proc->normalExit() && proc->exitStatus() == 0) {
+ ::exit(0);
+ } else {
+ if (m_errorStr.isEmpty()) {
+ if (m_isCdrom)
+ m_errorStr = i18n("The device was successfully unmounted, but the tray could not \
be opened"); + else
+ m_errorStr = i18n("The device was successfully unmounted, but could not be \
ejected"); }
- else
- {
- m_errorStr+= i18n("\nPlease check that the device is"
- " plugged correctly.");
- }
-
- QTimer::singleShot(0, this, SLOT(error()) );
+ QTimer::singleShot(0, this, SLOT(error()));
}
- else
- {
- invokeEject(m_device, true);
- }
}
-void MountHelper::slotResult(KIO::Job* job)
-{
- if (job->error())
- {
- m_errorStr = job->errorText();
-
- if (m_isCdrom)
- {
- m_errorStr+= i18n("\nPlease check that the disk is"
- " entered correctly.");
- }
- else
- {
- m_errorStr+= i18n("\nPlease check that the device is"
- " plugged correctly.");
- }
-
- QTimer::singleShot(0, this, SLOT(error()) );
- }
- else
- {
- QTimer::singleShot(0, this, SLOT(finished()) );
- }
-}
-
void MountHelper::error()
{
KMessageBox::error(0, m_errorStr);
::exit(1);
}
-void MountHelper::finished()
-{
- ::exit(0);
-}
-
static KCmdLineOptions options[] =
{
{ "u", I18N_NOOP("Unmount given URL"), 0 },
--- branches/KDE/3.5/kdebase/kioslave/media/mounthelper/kio_media_mounthelper.h \
#601027:601028 @@ -42,9 +42,7 @@
bool m_isCdrom;
private slots:
- void slotResult(KIO::Job* job);
- void slotResultSafe(KIO::Job* job);
- void finished();
+ void ejectFinished(KProcess* proc);
void error();
};
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic