mail archive of the rauc mailing list
 help / color / mirror / Atom feed
From: Trent Piepho <tpiepho@impinj.com>
To: "ladis@linux-mips.org" <ladis@linux-mips.org>,
	"rauc@pengutronix.de" <rauc@pengutronix.de>
Cc: "ejo@pengutronix.de" <ejo@pengutronix.de>,
	"jlu@pengutronix.de" <jlu@pengutronix.de>
Subject: Re: [RAUC] Failed to mount seed slot
Date: Tue, 7 May 2019 17:59:36 +0000	[thread overview]
Message-ID: <1557251975.31309.83.camel@impinj.com> (raw)
In-Reply-To: <20190507154902.GA30791@lenoch>

On Tue, 2019-05-07 at 17:49 +0200, Ladislav Michl wrote:
> 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:
> > > [slot.rootfs.0]
> > > device=/dev/ubi0_0
> > > type=ubifs
> > > bootname=system0
> > > 
> > > 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.

What does the kernel show in /proc/mounts in this case?  If it's
"/dev/ubi0_0", then it should match based on the simple path compare.

> 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:

Is there a way to mount a /dev/mtd<X> device directly, and not through
/dev/mtdblock<X>?  I was under the impression that yaffs1/2 and jffs2
used mtdblock, but I haven't used raw nand (what a PITA it is) in
while.

> - use C comments

Is this a requirement for rauc?  I see other // comments in rauc, they
are part of the C standard since C99, and rauc's extensive github
commit checking doesn't flag it.

> - avoid unneded allocations

Perhaps this is better, since none of the allocations need to live.

> - also allow character devices (for example /dev/ubi0_0)

Seems reasonable to fix that case.  This would work for /dev/mtd too,
if there is a way to mount those.

> > 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.

If the mount call uses ubiX:NAME, what does /proc/mounts report?  That
really the issue: matching the config file device string to the string
reported by the kernel when the device is mounted.

> 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 <ladis@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;

There would also need to be another bool to indicate if the device is a
 block dev or char dev, since the major:minor could match between
different device types.  That could be part of a union with inode since
the two are mutually exclusive.

>  
> -// 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;
>  		}
>  	}

  reply	other threads:[~2019-05-07 17:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06 17:44 Ladislav Michl
2019-05-07  7:31 ` Ladislav Michl
2019-05-07 15:49   ` Ladislav Michl
2019-05-07 17:59     ` Trent Piepho [this message]
2019-05-07 20:33       ` Ladislav Michl
2019-05-09 11:32         ` Ladislav Michl
2019-05-08  5:44       ` Ladislav Michl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1557251975.31309.83.camel@impinj.com \
    --to=tpiepho@impinj.com \
    --cc=ejo@pengutronix.de \
    --cc=jlu@pengutronix.de \
    --cc=ladis@linux-mips.org \
    --cc=rauc@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox