Bunch o' small changes in this CL
1. Fixes the makefile so it can deal with header autogeneration. Thanks Scott!
2. Fixes a crash udev-device.cc
3. Adds a simple test runner, no tests yet though.
4. Adds a signal to the d-bus API.
5. Marshals the disks type to the d-bus wire format.
6. Logs to the syslog and optionally stderr
Change-Id: Ifc3e9cfb22d5dbd3cb9406a94eb41d5b2c522e52
BUG=13698
TEST=Ran platform_CrosDisksDBus on a VM.
Review URL: http://codereview.chromium.org/6838011
diff --git a/.gitignore b/.gitignore
index cae6289..7548abe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,2 @@
build-opt/
+cros-disks-server.h
diff --git a/Makefile b/Makefile
index 8121151..47b91bd 100644
--- a/Makefile
+++ b/Makefile
@@ -12,26 +12,32 @@
PKG_CONFIG ?= pkg-config
DBUSXX_XML2CPP = dbusxx-xml2cpp
-INCLUDE_DIRS = -I.. -I$(OUT)include $(shell $(PKG_CONFIG) --cflags dbus-1 dbus-glib-1\
+INCLUDE_DIRS = -I.. -I$(OUT) $(shell $(PKG_CONFIG) --cflags dbus-1 dbus-glib-1\
dbus-c++-1 glib-2.0)
LIB_DIRS = $(shell $(PKG_CONFIG) --libs dbus-1 dbus-glib-1 dbus-c++-1 glib-2.0)
CFLAGS := -Iinclude $(CFLAGS)
CXXFLAGS := -Iinclude -I../ $(INCLUDE_DIRS) $(CXXFLAGS)
-LDFLAGS += -lbase -lgflags -lmetrics -ludev $(LIB_DIRS)
+LDFLAGS += -lbase -lchromeos -lgflags -lmetrics -ludev $(LIB_DIRS)
-$(OUT)include/cros-disks-server.h: cros-disks.xml
- mkdir -p $(OUT)include
+cros-disks-server.h: cros-disks.xml
$(DBUSXX_XML2CPP) cros-disks.xml --adaptor=$@
-RM_ON_CLEAN += $(OUT)include/cros-disks-server.h
+RM_ON_CLEAN += cros-disks-server.h
-$(OUT)disks: $(filter-out %_testrunner.o %_unittest.o,$(C_OBJECTS)) \
- $(OUT)include/cros-disks-server.h \
- $(CXX_OBJECTS)
+$(OUT)disks: $(filter-out %_testrunner.o %_unittest.o,$(CXX_OBJECTS)) \
+ cros-disks-server.h
$(call cxx_binary)
-all: $(OUT)disks
+RM_ON_CLEAN += $(OUT)disks
+
+$(OUT)disks_testrunner: $(filter-out %main.o,$(CXX_OBJECTS)) \
+ cros-disks-server.h
+ $(call cxx_binary, -lgtest -lgmock)
RM_ON_CLEAN += $(OUT)disks
# Some shortcuts
-disks: $(OUT)disks
-dbus-headers: $(OUT)include/cros-disks-server.h
+all: disks
+disks: dbus-headers
+ make $(OUT)disks
+dbus-headers: cros-disks-server.h
+tests: dbus-headers
+ make $(OUT)disks_testrunner
diff --git a/cros-disks.conf b/cros-disks.conf
index a04a6df..53b06c1 100644
--- a/cros-disks.conf
+++ b/cros-disks.conf
@@ -4,20 +4,14 @@
# cros-disks upstart job
-env CROS_DISKS_LOG_DIR=/var/log/cros-disks
env CROS_DISKS_UID=213
env CROS_DISKS_GID=213
-start on started dbus
-stop on starting halt or starting reboot
+start on starting system-services and started dbus
+stop on stopping system-services
respawn
expect fork
-pre-start script
- mkdir -p -m 0755 "${CROS_DISKS_LOG_DIR}"
- chown -R cros-disks:cros-disks "${CROS_DISKS_LOG_DIR}"
-end script
-
exec /sbin/minijail --uid="${CROS_DISKS_UID}" --gid="${CROS_DISKS_GID}" -- /opt/google/cros-disks/disks
diff --git a/cros-disks.xml b/cros-disks.xml
index 9c6eb29..4dee880 100644
--- a/cros-disks.xml
+++ b/cros-disks.xml
@@ -29,6 +29,10 @@
<arg name="disks" type="aa{sv}" direction="out" tp:type="DiskDevices">
</arg>
</method>
+ <signal name="DeviceUpdates">
+ <arg name="disks" type="aa{sv}" direction="out" tp:type="DiskDevices">
+ </arg>
+ </signal>
<method name="IsAlive">
<tp:docstring>
Test method to verify that Cashew service is working.
diff --git a/disk.cc b/disk.cc
index fe0bf8c..0300be6 100644
--- a/disk.cc
+++ b/disk.cc
@@ -17,12 +17,13 @@
const char kDeviceFile[] = "DeviceFile";
const char kLabel[] = "IdLabel";
const char kDriveModel[] = "DriveModel";
-const char kPartitionSlave[] = "PartitionSlave";
const char kDriveIsRotational[] = "DriveIsRotational";
const char kDeviceIsOpticalDisc[] = "DeviceIsOpticalDisc";
const char kDeviceSize[] = "DeviceSize";
const char kReadOnly[] = "DeviceIsReadOnly";
+// TODO(rtc): Figure out what this field is and include it in the response.
+const char kPartitionSlave[] = "PartitionSlave";
// TODO(rtc): The constructor should set some defaults, but I'm still iterating
// on the data model.
@@ -32,4 +33,22 @@
Disk::~Disk() {
}
+DBusDisk Disk::ToDBusFormat() const {
+ DBusDisk disk;
+ disk[kDeviceIsDrive].writer().append_bool(is_drive());
+ disk[kDevicePresentationHide].writer().append_bool(is_hidden());
+ disk[kDeviceIsMounted].writer().append_bool(is_mounted());
+ disk[kDeviceMountPaths].writer().append_string(mount_path().c_str());
+ disk[kDeviceIsMediaAvailable].writer().append_bool(is_media_available());
+ disk[kNativePath].writer().append_string(native_path().c_str());
+ disk[kDeviceFile].writer().append_string(device_file().c_str());
+ disk[kLabel].writer().append_string(label().c_str());
+ disk[kDriveModel].writer().append_string(drive_model().c_str());
+ disk[kDriveIsRotational].writer().append_bool(is_rotational());
+ disk[kDeviceIsOpticalDisc].writer().append_bool(is_optical_disk());
+ disk[kDeviceSize].writer().append_int64(device_capacity());
+ disk[kReadOnly].writer().append_bool(is_read_only());
+ return disk;
+}
+
} // namespace cros_disks
diff --git a/disk.h b/disk.h
index 14cc059..2341db9 100644
--- a/disk.h
+++ b/disk.h
@@ -25,6 +25,7 @@
Disk();
virtual ~Disk();
+ DBusDisk ToDBusFormat() const;
bool is_drive() const { return is_drive_; }
void set_is_drive(bool is_drive) { is_drive_ = is_drive; }
diff --git a/disks_testrunner.cc b/disks_testrunner.cc
new file mode 100644
index 0000000..a375ee8
--- /dev/null
+++ b/disks_testrunner.cc
@@ -0,0 +1,12 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+int main(int argc, char **argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ ::testing::GTEST_FLAG(throw_on_failure) = true;
+ ::testing::InitGoogleMock(&argc, argv);
+ return RUN_ALL_TESTS();
+}
diff --git a/main.cc b/main.cc
index 56bb11f..4864f2a 100644
--- a/main.cc
+++ b/main.cc
@@ -6,10 +6,12 @@
#include "cros-disks-server-impl.h"
#include "disk-manager.h"
+
#include <base/basictypes.h>
+#include <base/command_line.h>
#include <base/file_util.h>
-#include <base/logging.h>
#include <base/string_util.h>
+#include <chromeos/syslog_logging.h>
#include <dbus-c++/glib-integration.h>
#include <dbus-c++/util.h>
#include <gflags/gflags.h>
@@ -25,16 +27,15 @@
DEFINE_bool(foreground, false,
"Don't daemon()ize; run in foreground.");
-// TODO(rtc): gflags string defines require the use of
-// -fno-strict-aliasing for some reason. Verify that disabling this check
-// is sane.
-DEFINE_string(log_dir, "/var/log/cros-disks", "log directory");
-
+// Always logs to the syslog and logs to stderr if
+// we are running in the foreground.
void SetupLogging() {
- logging::InitLogging(FLAGS_log_dir.c_str(),
- logging::LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG,
- logging::DONT_LOCK_LOG_FILE,
- logging::APPEND_TO_OLD_LOG_FILE);
+ int log_flags = 0;
+ log_flags |= chromeos::kLogToSyslog;
+ if (FLAGS_foreground) {
+ log_flags |= chromeos::kLogToStderr;
+ }
+ chromeos::InitLog(log_flags);
}
// This callback will be invoked once udev has data about
@@ -51,10 +52,13 @@
::g_type_init();
g_thread_init(NULL);
google::ParseCommandLineFlags(&argc, &argv, true);
+ CommandLine::Init(argc, argv);
+
+ SetupLogging();
if(!FLAGS_foreground) {
+ LOG(INFO) << "Daemonizing";
PLOG_IF(FATAL, daemon(0, 0) == 1) << "daemon() failed";
- // SetupLogging();
}
LOG(INFO) << "Creating a GMainLoop";
@@ -81,7 +85,6 @@
DiskManager manager;
manager.EnumerateDisks();
-
// Setup a monitor
g_io_add_watch_full(g_io_channel_unix_new(manager.udev_monitor_fd()),
G_PRIORITY_HIGH_IDLE,
diff --git a/udev-device.cc b/udev-device.cc
index e324706..bd62b34 100644
--- a/udev-device.cc
+++ b/udev-device.cc
@@ -109,6 +109,7 @@
if (fs.is_open()) {
return ParseMountedPaths(dev_file, fs);
}
+ LOG(INFO) << "unable to parse /proc/mounts";
return std::vector<std::string>();
}
@@ -145,7 +146,11 @@
std::vector<std::string> mounted_paths = GetMountedPaths();
disk.set_is_mounted(!mounted_paths.empty());
- disk.set_mount_path(mounted_paths[0]); // TODO(benchan): multiple paths
+
+ if (!mounted_paths.empty()) {
+ // TODO(benchan): support multiple paths
+ disk.set_mount_path(mounted_paths[0]);
+ }
uint64 total_size, remaining_size;
GetSizeInfo(&total_size, &remaining_size);
