Ubuntu
mountall package

Merge lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer into lp://qastaging/ubuntu/natty/mountall

  1. Natty (11.04)
  2. mountall-stop-timer
  3. Merge into natty
Proposed by Surbhi Palande
Status: Needs review
Proposed branch: lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer
Merge into: lp://qastaging/ubuntu/natty/mountall
Diff against target: 3568 lines (+3066/-60)
25 files modified
Makefile.am (+1/-1)
Makefile.in (+1/-1)
configure (+5/-2)
configure.ac (+4/-2)
dbus/Makefile.am (+4/-0)
dbus/Makefile.in (+62/-11)
dbus/Mountall.Server.conf (+35/-0)
dbus/com.ubuntu.Mountall.Server.xml (+38/-0)
dbus/mountall.h (+49/-0)
debian/changelog (+26/-0)
debian/initramfs/Makefile.am (+4/-0)
debian/initramfs/Makefile.in (+499/-0)
debian/initramfs/mountall (+69/-0)
debian/initramfs/upstart-jobs/Makefile.am (+4/-0)
debian/initramfs/upstart-jobs/Makefile.in (+486/-0)
debian/initramfs/upstart-jobs/mountall.conf (+27/-0)
src/Makefile.am (+25/-6)
src/Makefile.in (+27/-7)
src/control.c (+258/-0)
src/control.h (+36/-0)
src/mountall.c (+270/-30)
src/mountall.h (+24/-0)
util/Makefile.am (+56/-0)
util/Makefile.in (+669/-0)
util/mntctl.c (+387/-0)
To merge this branch: bzr merge lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Fixing
Review via email: mp+57451@code.qastaging.launchpad.net

Description of the change

mountall provides support to wait for some time for devices to get ready when indicated through fstab with a "timeout" option. In some cases where human interaction is needed for configuring the device, the time that mountall waits may not indicate the time that the system takes in configuring the device. While the human interaction is happening, we may want to stop the timer and then restart it when the human interaction is through.

This patch adds support to start a private mountall server and exposes interfaces to stop and restart a timer if the device mentioned is indeed tagged with a TAG_TIMEOUT. Its upto the client program to appropriately send the device name which matches with what is indicated in /etc/fstab (otherwise the timer will have no effect).

The timer is not restarted with the remaining time, but what was indicated when mountall was started. This is intentionally done because of the fact that the human interaction would anyway change the timeout really and so no client application can really wait on the exact timeout.

Also, right now there is no consideration on authentication between the client and mountall (as the system bus is not involved) However this might need to be done, especially for the case when we are not in the initramfs context.

To post a comment you must log in.
Revision history for this message
Surbhi Palande (csurbhi) wrote :

I guess, this should be a merge request for Ubuntu Oneiric now. Please do consider this for the same. Thanks!

Reply
lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer updated
358. By Surbhi Palande

Fixed two errors found while browsing the code: 1) find_mount("/") returns
NULL when / is not found, so cannot access root->mounted. 2) the
information message about the dev_wait_timeout should be printed after
dev_wait_timeout is initialized properly.

359. By Bryce Harrington

Correct grammar on user-visible strings
(LP: #572016)

Revision history for this message
Colin Watson (cjwatson) wrote :

+ <allow send_destination="com.ubuntu.Mountall.Server"
+ send_interface="com.ubuntu.Mountall0_1.Server"
+ send_type="method_call" send_member="StopTimer" />
+ <allow send_destination="com.ubuntu.Mountall.Server"
+ send_interface="com.ubuntu.Mountall0_1.Server"
+ send_type="method_call" send_member="RestartTimers" />
+
+ <allow send_destination="com.ubuntu.Mountall.Server"
+ send_interface="com.ubuntu.Mountall0_1.Server.Job"
+ send_type="method_call" send_member="ChangeMountDevice" />

These methods should be root-only; just removing them from <policy context="default"> should be good enough.

+ <allow send_destination="com.ubuntu.Mountall.Server"
+ send_interface="com.ubuntu.Mountall0_1.Server.Job"

Remove ".Job".

+ <method name="RestartTimer">
+ <arg name="devname" type="s" direction="in" />
+ </method>

This is called RestartTimers in the .conf file. Which is correct?

Also, the .conf file has an entry for GetMountDevices, but that is missing from the .xml file.

- --package=$(PACAKGE) \
+ --package=$(PACKAGE) \

Thanks for fixing this, but there's another (probably copied-and-pasted) occurrence of $(PACAKGE) below. Could you fix that too?

=== added file 'src/control.c'
--- src/control.c 1970-01-01 00:00:00 +0000
+++ src/control.c 2011-06-30 18:09:32 +0000
@@ -0,0 +1,253 @@
+/* mountall
+ *
+ * Copyright © 2010 Canonical Ltd.
+ * Author: Surbhi A. Palande <email address hidden>

This is substantially copied from Upstart. That's fine - they're both copyright Canonical with the same licence, but it'd be good form to acknowledge Scott's co-authorship.

Otherwise I'm happy with this on visual inspection.

review: Needs Fixing
Reply
lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer updated
360. By Surbhi Palande

Added com.ubuntu.Mountall.server interface. Implemented the mntctl command
with 4 commands: 1. StopTimer 2. RestartTimer 3. ChangeMountDevice 4.
Version.

Revision history for this message
Surbhi Palande (csurbhi) wrote :

Hi Colin,

Thanks a lot for the review. I have made the suggested changes and also added one more change:
a) for installing the dbus/Mountall.Server to the correct dir: /etc/dbus-1/system.d
in the dbus/Makefile.am

Please do let me know if this looks alright to you.

Warm Regards,
Surbhi.

Reply
Revision history for this message
Surbhi Palande (csurbhi) wrote :

Hi Colin,

And I forgot to mention that I have also remove the upstart job responsible for stopping and restarting the timer before and after cryptsetup job respectively for now. I will add that soon here!

Warm Regards,
Surbhi.

Reply
lp://qastaging/~csurbhi/ubuntu/natty/mountall/mountall-stop-timer updated
361. By Surbhi Palande

Fixed the coding style bugs in the "timeout" option related code. Also
removed a break from the is_device_ready () to get the "device-not-ready"
event for all the devices which are marked with the timeout option and
which are not ready by the time the timer expires.

362. By Surbhi Palande

Added debian/initramfs/Makefile.am to install mountall.initramfs-hook as
an initramfs hook that installs mntctl, mountall and other files needed by
an event driven initramfs. Also added
debian/initramfs/upstart-jobs/Makefile.am to install the upstart-jobs
related to mountall in the event driven initramfs. The current
mountall.conf execute as an upstart job in place of the "local" script in
the non-event driven initramfs

363. By Surbhi Palande

Changed the tag from maverick to oneiric

364. By Surbhi Palande

Added the missing debian/initramfs/upstart-jobs/Makefile.am for installing the
upstart jobs related to the functioning of the event driven initramfs

365. By Surbhi Palande

Fixed the installation of upstart jobs to event-driven dir in the
/usr/share/initramfs-tools/event-driven/

366. By Surbhi Palande

Changed the default mountall hook name to mountall.

367. By Surbhi Palande

Changed the DATA hook to SCRIPT hook to preserve the +x flag on the mountall
hook for initramfs.

368. By Surbhi Palande

Removed the previously added term_handler for the following reasons:
1) You need to disconnect from the dbus_server and unref it only once.
This must happen when mountall finishes mounting all the filesystems. If
mountall is terminated before that then this would be because of an
erroroneous condition on which you want to shut down and so restarting the
mountall Server would not be necessary.

369. By Surbhi Palande

Disconnect from the dbus server only if it was successfully connected
previously

370. By Surbhi Palande

Changed the interface to stop/restart timers to use a mountpoint rather than
the device name. This is useful in situations where the device mentioned in
/etc/fstab is not the device used for mounting because of the change mount
device interface. The mountpoint remains constant in all cases and is also
easier to use that specifying a device with uuids for eg.

371. By Surbhi Palande

Review fixes

372. By Surbhi Palande

Removed the destination for copy_exec and the unnecessary echoes

373. By Surbhi Palande

Added the shell execution exclamation mark to the mountall script

Unmerged revisions

373. By Surbhi Palande

Added the shell execution exclamation mark to the mountall script

372. By Surbhi Palande

Removed the destination for copy_exec and the unnecessary echoes

371. By Surbhi Palande

Review fixes

370. By Surbhi Palande

Changed the interface to stop/restart timers to use a mountpoint rather than
the device name. This is useful in situations where the device mentioned in
/etc/fstab is not the device used for mounting because of the change mount
device interface. The mountpoint remains constant in all cases and is also
easier to use that specifying a device with uuids for eg.

369. By Surbhi Palande

Disconnect from the dbus server only if it was successfully connected
previously

368. By Surbhi Palande

Removed the previously added term_handler for the following reasons:
1) You need to disconnect from the dbus_server and unref it only once.
This must happen when mountall finishes mounting all the filesystems. If
mountall is terminated before that then this would be because of an
erroroneous condition on which you want to shut down and so restarting the
mountall Server would not be necessary.

367. By Surbhi Palande

Changed the DATA hook to SCRIPT hook to preserve the +x flag on the mountall
hook for initramfs.

366. By Surbhi Palande

Changed the default mountall hook name to mountall.

365. By Surbhi Palande

Fixed the installation of upstart jobs to event-driven dir in the
/usr/share/initramfs-tools/event-driven/

364. By Surbhi Palande

Added the missing debian/initramfs/upstart-jobs/Makefile.am for installing the
upstart jobs related to the functioning of the event driven initramfs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches