Code review comment for ~goneri/cloud-init:freebsd_mount_sync

On Mon, Mar 25, 2019 at 05:09:02PM -0000, Gonéri Le Bouder wrote:
> I may misunderstand something. The goal here is to open a device, read
> some data and unmount the device. The device being a little disk or a
> CDROM. I don't think cloud-init do any modification, does it? So, it
> should just use the 'ro' parameter and ignores the 'sync' flag.

That's how mount_cb is used where you're modifying the call in
DataSourceConfigDrive, yes. However, the mount_cb function has a rw
parameter, which means you need to support that parameter correctly in
your changes to that function; your current change to mount_cb removes
sync from _all_ mount calls made on non-Linux platforms.

Furthermore, removing the sync flag in places where it is currently
acceptable to pass it is a change in behaviour, which could end up
causing a regression on platforms we don't (or can't) test. My feeling
is that "we probably don't need it" is not compelling enough
justification for such a change, without clear benefits.

(I would suggest that you target your changes at specifically the case
that you are seeing fail, rather than modifying default behaviour for
the silent majority of working cases.)