On Tue, May 07, 2019 at 09:31:28AM +0200, Ladislav Michl wrote:
> On Mon, May 06, 2019 at 07:44:09PM +0200, Ladislav Michl wrote:
> > Hi there,
> > 
> > I'm using rauc with casync to update target system - it is symmetric 
> > rootfsA,
> > rootfsB, scenario, so slots are defined as:
> > [slot.rootfs.0]
> > device=/dev/ubi0_0
> > type=ubifs
> > bootname=system0
> > 
> > [slot.rootfs.1]
> > device=/dev/ubi0_1
> > type=ubifs
> > bootname=system1
> > 
> > Now system is booted with rootfs mounted read only which makes mounting seed
> > slot fail:
> > rauc[871]: Mounting /dev/ubi0_0 to use as seed
> > rauc[871]: launching subprocess: mount -t ubifs /dev/ubi0_0 
> > /run/rauc/rootfs.0
> > rauc[871]: posix_spawn avoided (fd close requested) (child_setup specified) 
> > rauc[871]: mount: /run/rauc/rootfs.0: /dev/ubi0_0 already mounted or mount 
> > point busy.
> > 
> > Here adding '-o ro' would quick fix it up (but break rw case, as options 
> > have to
> > match), so it seems better to use bind mount as comment casync_extract_image
> > function suggests, but do not see it code.
> > 
> > Am I missing anything?
> 
> So, whats still missing is a review of pull request #406 (Normalize device 
> names
> to find mounted slots) :)

It feels really strange talking to myself, so let's hope someone else joins as
well, hence Cc list.

Problem is similar to that one solved by #406 and #388, however it needs to be
extended to ubi<X> and possibly mtd<X> case. For now let's start with fixing
pull request #406 to:
- use C comments
- avoid unneded allocations
- also allow character devices (for example /dev/ubi0_0)

> Then determine_slot_states should properly fill slot's ext_mount_point and
> casync_extract_image would then use it to bind mount seed slot.
> 
> And here's the tricky part. For device-less mount, volume to mount is 
> specified
> for example using ubiX_Y or ubiX:NAME syntax, so fix in #406 will fail in this
> case.

This one was fixed by extending normalize_mountable_object from #406 and
adjusting casync_extract_image, but lets first get #406 revieved and merged :)
Trent, in case you want to merge following additional patch to your PR here's my
Signed-off-by: Ladislav Michl <la...@linux-mips.org>

diff --git a/src/config_file.c b/src/config_file.c
index 875e0ee..e592d15 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -494,27 +494,25 @@ free:
        return res;
 }
 
-// Something that is, or can be, mounted onto a mount point
+/* Something that is, or can be, mounted onto a mount point */
 typedef struct {
-       gboolean is_device;  // vs being a loop mounted file
-       dev_t dev;  // the device, or for a file, the device the file is on
-       ino_t inode;  // inode of file for a non-device
+       gboolean is_device;     /* vs being a loop mounted file */
+       dev_t dev;              /* the device, or for a file, the device the 
file is on */
+       ino_t inode;            /* inode of file for a non-device */
 } MountableObj;
 
-// Take a device (or file) path and normalize it
-static MountableObj *normalize_mountable_object(const gchar *devicepath)
+/* Take a device (or file) path or name and normalize it */
+static gboolean normalize_mountable_object(const gchar *devicename, 
MountableObj *obj)
 {
-       MountableObj *obj;
        GStatBuf st;
 
-       if (g_stat(devicepath, &st) == -1) {
+       if (g_stat(devicename, &st) == -1) {
                /* Virtual filesystems like devpts trigger case */
-               g_debug("Can't stat '%s', assuming unmountable: %s", 
devicepath, g_strerror(errno));
-               return NULL;
+               g_debug("Can't stat '%s', assuming unmountable: %s", 
devicename, g_strerror(errno));
+               return FALSE;
        }
 
-       obj = g_new0(MountableObj, 1);
-       if (S_ISBLK(st.st_mode)) {
+       if (S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode)) {
                obj->is_device = TRUE;
                obj->dev = st.st_rdev;
        } else if (S_ISREG(st.st_mode)) {
@@ -522,10 +520,10 @@ static MountableObj *normalize_mountable_object(const 
gchar *devicepath)
                obj->dev = st.st_dev;
                obj->inode = st.st_ino;
        } else {
-               g_debug("Device '%s' is not something which is mountable", 
devicepath);
-               g_clear_pointer(&obj, g_free);
+               g_debug("Device '%s' is not something which is mountable", 
devicename);
+               return FALSE;
        }
-       return obj;
+       return TRUE;
 }
 
 /* Compare two MountableObj for equality */
@@ -546,24 +544,24 @@ RaucSlot *find_slot_by_device(GHashTable *slots, const 
gchar *device)
 {
        GHashTableIter iter;
        RaucSlot *slot;
-       g_autofree MountableObj *obj = NULL;
+       MountableObj obj;
+       gboolean normalized;
 
        g_return_val_if_fail(slots, NULL);
        g_return_val_if_fail(device, NULL);
 
-       obj = normalize_mountable_object(device);
+       normalized = normalize_mountable_object(device, &obj);
 
        g_hash_table_iter_init(&iter, slots);
        while (g_hash_table_iter_next(&iter, NULL, (gpointer*) &slot)) {
                if (g_strcmp0(slot->device, device) == 0)
                        goto out;
 
-               // Path doesn't match, but maybe device is same?
-               if (obj) {
-                       g_autofree MountableObj *slot_obj =
-                               normalize_mountable_object(slot->device);
-
-                       if (slot_obj && is_same_mountable_object(obj, slot_obj))
+               /* Path doesn't match, but maybe device is the same? */
+               if (normalized) {
+                       MountableObj slot_obj;
+                       if (normalize_mountable_object(slot->device, &slot_obj) 
&&
+                           is_same_mountable_object(&obj, &slot_obj))
                                goto out;
                }
        }

_______________________________________________
RAUC mailing list

Reply via email to