Any reason to put that into imageloader when it can just be done as shell commands from imageloader.conf?

I think another goal is to consolidate this along with imageloader_wrapper into imageloader with '--init' option. 

But i'm fine with shell commands inside imageloader.conf.

That's a good point. imageloader_wrapper has no reason to exist anymore, it should all move into imageloader.conf and we can delete the wrapper.

relatively speaking, spawning a shell and running various programs is more expensive than doing it in C++.  it's also tends to be harder to do error checking (or no one bothers), harder to get logging when it fails (or no one bothers), and harder to unittest.

conversely, doing it all in C++ addresses all of those nicely.

Ok let's do it in C++ then. Either way, let's drop imageloader_wrapper and do it all in imageloader.conf with a --init flag.

I encountered an issue calling mount in imageloader_main.cc:

if (mount("imageloader", foo, "tmpfs", MS_SHARED, "") < 0) {
      LOG(ERROR)<<"Mount failed.";
}

mount returns '-1' in this case. Then when I changed the mounting parameter to be MS_RDONLY | MS_NOSUID | MS_NODEV (we use this to mount image in imageloader), it works properly. I think we encounter a similar issue when we tried to mount an ext4 image where kernel does not always support.

Should we switch to do this in imageloader.conf file? 

I need to think about the flags, but the first thing we should do is make that PLOG(ERROR) to see the actual errno message. Try again with PLOG(ERROR) there?

MS_SHARED is used to change the propagation type of an existing mount ... it isn't used to create a new mount

i'm not sure what |foo| represents here.  if it's /run/imageloader and you haven't first mounted it, you need to do that first.
  mount("imageloader", kLoadedMountsBase, "tmpfs", MS_RDONLY | MS_NOSUID | MS_NODEV, "mode=0755");
  mount(nullptr, kLoadedMountsBase, nullptr, MS_SHARED, nullptr);
(i haven't tested that though :P)

Thanks Mike and Greg. Yes I was doing in the wrong way...

Now I mount a tmpfs first and then use MS_SHARED as the sole parameter to mount again. This time it shows /run/imageloader mounted as 'shared' correctly. 

I also noticed that 'MS_SHARED' and 'MS_REMOUNT' could not be used the same time. Otherwise only MS_REMOUNT is executed (mount point being not shared).

Will try PLOG(ERROR) first next time!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/e1c1a2be30845637268a9cdfa89e2ba9bb12a55b

commit e1c1a2be30845637268a9cdfa89e2ba9bb12a55b
Author: Xiaochu Liu <xiaochu@chromium.org>
Date: Fri Feb 23 11:36:23 2018

platform_FilePerms: add entry for /run/imageloader

We mount tmpfs at /run/imageloader now. So add entries to check for its
permission.

BUG= chromium:813131 
TEST=test_that platform_FilePerms

Change-Id: I7111a54bef159c70f33b96b4c51e6a74c9620595
Reviewed-on: https://chromium-review.googlesource.com/933281
Commit-Ready: Xiaochu Liu <xiaochu@chromium.org>
Tested-by: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e1c1a2be30845637268a9cdfa89e2ba9bb12a55b/client/site_tests/platform_FilePerms/platform_FilePerms.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e2a6f810f48fb1d6b7c32371247f5495455f61a7

commit e2a6f810f48fb1d6b7c32371247f5495455f61a7
Author: Xiaochu Liu <xiaochu@chromium.org>
Date: Fri Feb 23 11:36:24 2018

imageloader: mount a tmpfs at /run/imageloader

This CL mounts a tmpfs at /run/imageloader when --init option is used.
Also imageloader_wrapper's content is moved to imageloader_main.cc. We
do not delete imageloader_wrapper for now otherwise install fails since
it looks for this file to copy.

BUG= chromium:813131 
TEST=test_that -b ${BOARD} 100.96.51.83 platform_ImageLoaderServer
CQ-DEPEND=CL:933281

Change-Id: I17f6c6b49349c9df3f60700ec0eea7a975d7082d
Reviewed-on: https://chromium-review.googlesource.com/929307
Commit-Ready: Xiaochu Liu <xiaochu@chromium.org>
Tested-by: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e2a6f810f48fb1d6b7c32371247f5495455f61a7/imageloader/imageloader_main.cc
[modify] https://crrev.com/e2a6f810f48fb1d6b7c32371247f5495455f61a7/imageloader/imageloader.conf
[modify] https://crrev.com/e2a6f810f48fb1d6b7c32371247f5495455f61a7/imageloader/dbus_service/org.chromium.ImageLoader.service

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/a0c0064456fe3a742856b40ddc96b9f606206c9b

commit a0c0064456fe3a742856b40ddc96b9f606206c9b
Author: Xiaochu Liu <xiaochu@chromium.org>
Date: Sat Feb 24 05:06:42 2018

chromeos-base/imageloader: not install imageloader_wrapper

imageloader_wrapper is no longer used and we do not need to install it
to device.

BUG= chromium:813131 
TEST=emerge imageloader && cros deploy IP imageloader
CQ-DEPEND=CL:929307

Change-Id: I277165e58794fb802024527e00487624fb12f8bd
Reviewed-on: https://chromium-review.googlesource.com/932017
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a0c0064456fe3a742856b40ddc96b9f606206c9b/chromeos-base/imageloader/imageloader-9999.ebuild

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e1f8538fa3256c74d49cf8702939599ca67927db

commit e1f8538fa3256c74d49cf8702939599ca67927db
Author: Xiaochu Liu <xiaochu@chromium.org>
Date: Sat Feb 24 05:06:43 2018

imageloader: remove imageloader_wrapper

imageloader_wrapper is no longer used or installed. So delete it from
repo.

BUG= chromium:813131 
TEST=emerge imageloader && cros deploy IP imageloader
CQ-DEPEND=CL:932017

Change-Id: I42caa968e386f8d6233f65113e978c585100cb78
Reviewed-on: https://chromium-review.googlesource.com/932341
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/e1f8538fa3256c74d49cf8702939599ca67927db/imageloader/imageloader_main.cc
[modify] https://crrev.com/e1f8538fa3256c74d49cf8702939599ca67927db/imageloader/README.md
[delete] https://crrev.com/6556fb37fc0937a2bb351a05c16118348c72a155/imageloader/imageloader_wrapper